This is an archive of the discontinued LLVM Phabricator instance.

[flang] Make real type of kind 10 target dependent
ClosedPublic

Authored by peixin on Sep 16 2022, 12:42 AM.

Details

Summary

The real(10) is supported on x86_64. On aarch64, the value of
selected_real_kind(16) should be 16 rather than 10 since real(10)
is not supported on x86_64. Previously, the real type support check
is not target dependent. Support it now through the target triple
information.

Diff Detail

Event Timeline

peixin created this revision.Sep 16 2022, 12:42 AM
peixin requested review of this revision.Sep 16 2022, 12:42 AM
peixin updated this revision to Diff 460969.EditedSep 16 2022, 11:12 PM

Add "-triple aarch64-unknown-linux-gnu" to fix the CI fail. Do not know why the test case is still checked on Debian X86 even with ! REQUIRES: aarch64-registered-target.

Add "-triple aarch64-unknown-linux-gnu" to fix the CI fail. Do not know why the test case is still checked on Debian X86 even with ! REQUIRES: aarch64-registered-target.

! REQUIRES: <target> refers to the targets enabled when building LLVM rather than to the host machine on which the compiler is built. In the case of pre-commit CI, all available targets are enabled (i.e. the default value for LLVM_TARGETS_TO_BUILD is used):

-- Targeting AArch64
-- Targeting AMDGPU
-- Targeting ARM
-- Targeting AVR
-- Targeting BPF
-- Targeting Hexagon
-- Targeting Lanai
-- Targeting Mips
-- Targeting MSP430
-- Targeting NVPTX
-- Targeting PowerPC
-- Targeting RISCV
-- Targeting Sparc
-- Targeting SystemZ
-- Targeting VE
-- Targeting WebAssembly
-- Targeting X86
-- Targeting XCore
clementval added a comment.EditedSep 17 2022, 11:17 AM

I think you should just call targetCharacteristics.DisableType(Fortran::common::TypeCategory::Real, 10); from the driver when the target is set and the rest is already in place.

peixin planned changes to this revision.Sep 18 2022, 11:57 PM

Add "-triple aarch64-unknown-linux-gnu" to fix the CI fail. Do not know why the test case is still checked on Debian X86 even with ! REQUIRES: aarch64-registered-target.

! REQUIRES: <target> refers to the targets enabled when building LLVM rather than to the host machine on which the compiler is built. In the case of pre-commit CI, all available targets are enabled (i.e. the default value for LLVM_TARGETS_TO_BUILD is used):

-- Targeting AArch64
-- Targeting AMDGPU
-- Targeting ARM
-- Targeting AVR
-- Targeting BPF
-- Targeting Hexagon
-- Targeting Lanai
-- Targeting Mips
-- Targeting MSP430
-- Targeting NVPTX
-- Targeting PowerPC
-- Targeting RISCV
-- Targeting Sparc
-- Targeting SystemZ
-- Targeting VE
-- Targeting WebAssembly
-- Targeting X86
-- Targeting XCore

Got it. Thanks.

I think you should just call targetCharacteristics.DisableType(Fortran::common::TypeCategory::Real, 10); from the driver when the target is set and the rest is already in place.

Good point. This will avoid adding new element to SemanticContext. Will fix the patch.

peixin updated this revision to Diff 461193.Sep 19 2022, 6:03 AM

Reimplement this as @clementval suggested.

clementval added inline comments.Sep 26 2022, 6:29 AM
flang/module/__fortran_ieee_exceptions.f90
46

I'm not sure this is the right approach. It will not work when we do cross compilation (target is different than the host machine) since the target might have or not have support for real(10).

peixin added inline comments.Sep 26 2022, 7:18 AM
flang/module/__fortran_ieee_exceptions.f90
46

You are right. I also have this question when I follow the previous implementation in flang/module/iso_c_binding.f90. I think this is only the temporary and fast solution to support quad on aarch64. The final solution for the cross-compilation might need fix the build scripts (CMakeLists.txt) for flang.

klausler added inline comments.Sep 26 2022, 8:57 AM
flang/module/__fortran_ieee_exceptions.f90
46

Such a cross-compilation would properly define x86_64 or not, and that's the only architecture with x87 80-bit extended precision numbers.

This revision is now accepted and ready to land.Sep 27 2022, 12:35 AM
ekieri added a subscriber: ekieri.Sep 28 2022, 12:01 PM
This revision was automatically updated to reflect the committed changes.