[flang-rt] replace the triple dir to 'aix' for flang-rt to be consistent with clang on AIX. #130875

DanielCChen · 2025-03-12T01:36:21Z

This change is to put libflang_rt.runtime.a into build/lib/clang/21/lib/aix/libflang_rt.runtime.a, which is consistent with clang on AIX.

Meinersbur · 2025-03-12T15:42:02Z

The snippet was copied from

llvm-project/compiler-rt/cmake/Modules/CompilerRTUtils.cmake

Lines 463 to 508 in 665299e

    
             if(COMPILER_RT_DEFAULT_TARGET_ONLY) 
        
               # Use exact spelling when building only for the target specified to CMake. 
        
               set(target "${COMPILER_RT_DEFAULT_TARGET_TRIPLE}") 
        
             elseif(ANDROID AND ${arch} STREQUAL "i386") 
        
               set(target "i686${triple_suffix}") 
        
             elseif(${arch} STREQUAL "amd64") 
        
               set(target "x86_64${triple_suffix}") 
        
             elseif(${arch} STREQUAL "sparc64") 
        
               set(target "sparcv9${triple_suffix}") 
        
             elseif("${arch}" MATCHES "mips64|mips64el") 
        
               string(REGEX REPLACE "-gnu.*" "-gnuabi64" triple_suffix_gnu "${triple_suffix}") 
        
               string(REGEX REPLACE "mipsisa32" "mipsisa64" triple_cpu_mips "${triple_cpu}") 
        
               string(REGEX REPLACE "^mips$" "mips64" triple_cpu_mips "${triple_cpu_mips}") 
        
               string(REGEX REPLACE "^mipsel$" "mips64el" triple_cpu_mips "${triple_cpu_mips}") 
        
               set(target "${triple_cpu_mips}${triple_suffix_gnu}") 
        
             elseif("${arch}" MATCHES "mips|mipsel") 
        
               string(REGEX REPLACE "-gnuabi.*" "-gnu" triple_suffix_gnu "${triple_suffix}") 
        
               string(REGEX REPLACE "mipsisa64" "mipsisa32" triple_cpu_mips "${triple_cpu}") 
        
               string(REGEX REPLACE "mips64" "mips" triple_cpu_mips "${triple_cpu_mips}") 
        
               set(target "${triple_cpu_mips}${triple_suffix_gnu}") 
        
             elseif("${arch}" MATCHES "^arm") 
        
               # Arch is arm, armhf, armv6m (anything else would come from using 
        
               # COMPILER_RT_DEFAULT_TARGET_ONLY, which is checked above). 
        
               if (${arch} STREQUAL "armhf") 
        
                 # If we are building for hard float but our ABI is soft float. 
        
                 if ("${triple_suffix}" MATCHES ".*eabi$") 
        
                   # Change "eabi" -> "eabihf" 
        
                   set(triple_suffix "${triple_suffix}hf") 
        
                 endif() 
        
                 # ABI is already set in the triple, don't repeat it in the architecture. 
        
                 set(arch "arm") 
        
               else () 
        
                 # If we are building for soft float, but the triple's ABI is hard float. 
        
                 if ("${triple_suffix}" MATCHES ".*eabihf$") 
        
                   # Change "eabihf" -> "eabi" 
        
                   string(REGEX REPLACE "hf$" "" triple_suffix "${triple_suffix}") 
        
                 endif() 
        
               endif() 
        
               set(target "${arch}${triple_suffix}") 
        
             elseif("${arch}" MATCHES "^amdgcn") 
        
               set(target "amdgcn-amd-amdhsa") 
        
             elseif("${arch}" MATCHES "^nvptx") 
        
               set(target "nvptx64-nvidia-cuda") 
        
             else() 
        
               set(target "${arch}${triple_suffix}") 
        
             endif()

Do you intend to update it there as well? Or maybe I missed that compiler-rt is lower-casing the triple somewhere before calling get_compiler_rt_target?

DanielCChen · 2025-03-12T18:39:41Z

The snippet was copied from

llvm-project/compiler-rt/cmake/Modules/CompilerRTUtils.cmake

Lines 463 to 508 in 665299e

if(COMPILER_RT_DEFAULT_TARGET_ONLY)

# Use exact spelling when building only for the target specified to CMake.

set(target "${COMPILER_RT_DEFAULT_TARGET_TRIPLE}")

elseif(ANDROID AND ${arch} STREQUAL "i386")

set(target "i686${triple_suffix}")

elseif(${arch} STREQUAL "amd64")

set(target "x86_64${triple_suffix}")

elseif(${arch} STREQUAL "sparc64")

set(target "sparcv9${triple_suffix}")

elseif("${arch}" MATCHES "mips64|mips64el")

string(REGEX REPLACE "-gnu.*" "-gnuabi64" triple_suffix_gnu "${triple_suffix}")

string(REGEX REPLACE "mipsisa32" "mipsisa64" triple_cpu_mips "${triple_cpu}")

string(REGEX REPLACE "^mips$" "mips64" triple_cpu_mips "${triple_cpu_mips}")

string(REGEX REPLACE "^mipsel$" "mips64el" triple_cpu_mips "${triple_cpu_mips}")

set(target "${triple_cpu_mips}${triple_suffix_gnu}")

elseif("${arch}" MATCHES "mips|mipsel")

string(REGEX REPLACE "-gnuabi.*" "-gnu" triple_suffix_gnu "${triple_suffix}")

string(REGEX REPLACE "mipsisa64" "mipsisa32" triple_cpu_mips "${triple_cpu}")

string(REGEX REPLACE "mips64" "mips" triple_cpu_mips "${triple_cpu_mips}")

set(target "${triple_cpu_mips}${triple_suffix_gnu}")

elseif("${arch}" MATCHES "^arm")

# Arch is arm, armhf, armv6m (anything else would come from using

# COMPILER_RT_DEFAULT_TARGET_ONLY, which is checked above).

if (${arch} STREQUAL "armhf")

# If we are building for hard float but our ABI is soft float.

if ("${triple_suffix}" MATCHES ".*eabi$")

# Change "eabi" -> "eabihf"

set(triple_suffix "${triple_suffix}hf")

endif()

# ABI is already set in the triple, don't repeat it in the architecture.

set(arch "arm")

else ()

# If we are building for soft float, but the triple's ABI is hard float.

if ("${triple_suffix}" MATCHES ".*eabihf$")

# Change "eabihf" -> "eabi"

string(REGEX REPLACE "hf$" "" triple_suffix "${triple_suffix}")

endif()

endif()

set(target "${arch}${triple_suffix}")

elseif("${arch}" MATCHES "^amdgcn")

set(target "amdgcn-amd-amdhsa")

elseif("${arch}" MATCHES "^nvptx")

set(target "nvptx64-nvidia-cuda")

else()

set(target "${arch}${triple_suffix}")

endif()

Do you intend to update it there as well? Or maybe I missed that compiler-rt is lower-casing the triple somewhere before calling get_compiler_rt_target?

Actually, compiler-rt build on AIX doesn't call get_compiler_rt_target, so it shouldn't be affected.

Meinersbur · 2025-03-13T01:03:11Z

Actually, compiler-rt build on AIX doesn't call get_compiler_rt_target, so it shouldn't be affected.

How so? What is stopping someone to set LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON on AIX? Is it just not a supported configuration?

DanielCChen · 2025-03-13T05:21:06Z

Actually, compiler-rt build on AIX doesn't call get_compiler_rt_target, so it shouldn't be affected.

How so? What is stopping someone to set LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON on AIX? Is it just not an unsupported configuration?

You are right. We do call get_compiler_rt_target when LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON.

I think the comment in flang-rt/cmake/modules/GetToolchainDirs.cmake may be incorrectly flipped. It has

# * LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=1: lib/${oslibname}/libclang_rt.builtins-${arch}.a
# * LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=0: lib/${triple}/libclang_rt.builtins.a

On AIX,

When it is ON, we have lib/clang/21/lib/powerpc-ibm-aix/libclang_rt.builtins.a
When it is OFF (which is default for compiler-rt), we have lib/clang/21/lib/aix/libclang_rt.builtins-powerpc64.a

which is exactly the opposite of what the flang-rt has.

If the behavior on AIX is correct, flang-rt is actually have LLVM_ENABLE_PER_TARGET_RUNTIME_DIR default ON where compiler-rt has it default OFF.

@DanielCChen

The cases of LLVM_ENABLE_PER_TARGET_RUNTIME_DIR were swapped. Noticed by @DanielCChen in #130875. Thanks!

Meinersbur

You are correct, fixed in 13261e8.

As the comment mentioned, flang-rt only uses the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON scheme and ignores the setting. See #110217 (comment) for the decision.

I am LGTM-ing this low-impact PR in order to get it working on AIX even though I think there is something more profound going on. It can be cleaned up some other time.

kkwli · 2025-03-13T13:25:25Z

flang-rt/cmake/modules/GetToolchainDirs.cmake

@@ -118,6 +118,9 @@ function (get_toolchain_arch_dirname outvar)
    set(target "amdgcn-amd-amdhsa")
  elseif("${arch}" MATCHES "^nvptx")
    set(target "nvptx64-nvidia-cuda")
+  elseif(UNIX AND CMAKE_SYSTEM_NAME MATCHES "AIX")
+    # Put at lib/aix to be consistent with clang on AIX.


Nit: "Put at" → "Use"

Thanks. Will fix.

DanielCChen · 2025-03-14T12:05:21Z

You are correct, fixed in 13261e8.

As the comment mentioned, flang-rt only uses the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON scheme and ignores the setting. See #110217 (comment) for the decision.

I am LGTM-ing this low-impact PR in order to get it working on AIX even though I think there is something more profound going on. It can be cleaned up some other time.

Thanks for the review!
We are not planning to support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON on AIX for both flang and clang, so this PR is setting the library path for flang-rt as if the option is OFF.
I am planning to also change the library name to include ${arch} to complete the reverse.
I am not sure if you plan to change the default of the option in the future, but we can deal with that if it is the case.

Meinersbur · 2025-03-14T14:16:09Z

Not planning to implement support for honoring LLVM_ENABLE_PER_TARGET_RUNTIME_DIR myself. I think the idea of LLVM move away from LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF as a configuration option, it just adds complexity/differences. The resource directory is also private to Clang, its organization should not be a concern of the host platform. The driver never even supported it. However, I would not object to someone implementing it.

Consider that there already an exception for Apple:

llvm-project/flang-rt/cmake/modules/GetToolchainDirs.cmake

Line 37 in bbd1bb4

if (APPLE)

Consider using os_dirname instead of modifying get_toolchain_arch_dirname.

…ent with clang on AIX.

…tead of changing get_toolchain_arch_dirname.

DanielCChen · 2025-03-14T17:41:36Z

Good Idea! Will change to it.

@DanielCChen

The cases of LLVM_ENABLE_PER_TARGET_RUNTIME_DIR were swapped. Noticed by @DanielCChen in llvm#130875. Thanks!

DanielCChen requested review from Meinersbur, kkwli and madanial0 March 12, 2025 01:36

DanielCChen self-assigned this Mar 12, 2025

DanielCChen requested a review from daltenty March 13, 2025 03:34

Meinersbur added a commit that referenced this pull request Mar 13, 2025

[Flang-RT][NFC] Fix comment

13261e8

The cases of LLVM_ENABLE_PER_TARGET_RUNTIME_DIR were swapped. Noticed by @DanielCChen in #130875. Thanks!

Meinersbur approved these changes Mar 13, 2025

View reviewed changes

kkwli reviewed Mar 13, 2025

View reviewed changes

kkwli approved these changes Mar 13, 2025

View reviewed changes

[flang-rt] replace the triple dir to 'aix' for flang-rt to be consist…

0644e33

…ent with clang on AIX.

DanielCChen force-pushed the daniel_aixlibpath branch from 558a64c to 0644e33 Compare March 14, 2025 17:38

[flang-rt] Using get_toolchain_os_dirname for AIX to get lib path ins…

960d207

…tead of changing get_toolchain_arch_dirname.

DanielCChen merged commit a4510aa into llvm:main Mar 17, 2025
10 checks passed

DanielCChen deleted the daniel_aixlibpath branch March 17, 2025 15:30

frederik-h pushed a commit to frederik-h/llvm-project that referenced this pull request Mar 18, 2025

[Flang-RT][NFC] Fix comment

6bf3a00

The cases of LLVM_ENABLE_PER_TARGET_RUNTIME_DIR were swapped. Noticed by @DanielCChen in llvm#130875. Thanks!

DanielCChen mentioned this pull request Mar 18, 2025

[flang-rt] Pass the whole path of libflang_rt.runtime.a to linker on AIX and LoP #131041

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flang-rt] replace the triple dir to 'aix' for flang-rt to be consistent with clang on AIX. #130875

[flang-rt] replace the triple dir to 'aix' for flang-rt to be consistent with clang on AIX. #130875

DanielCChen commented Mar 12, 2025

Meinersbur commented Mar 12, 2025 •

edited

Loading

DanielCChen commented Mar 12, 2025

Meinersbur commented Mar 13, 2025 •

edited

Loading

DanielCChen commented Mar 13, 2025 •

edited

Loading

Meinersbur left a comment

kkwli Mar 13, 2025

DanielCChen Mar 14, 2025

DanielCChen commented Mar 14, 2025

Meinersbur commented Mar 14, 2025 •

edited

Loading

DanielCChen commented Mar 14, 2025

[flang-rt] replace the triple dir to 'aix' for flang-rt to be consistent with clang on AIX. #130875

[flang-rt] replace the triple dir to 'aix' for flang-rt to be consistent with clang on AIX. #130875

Conversation

DanielCChen commented Mar 12, 2025

Meinersbur commented Mar 12, 2025 • edited Loading

DanielCChen commented Mar 12, 2025

Meinersbur commented Mar 13, 2025 • edited Loading

DanielCChen commented Mar 13, 2025 • edited Loading

Meinersbur left a comment

Choose a reason for hiding this comment

kkwli Mar 13, 2025

Choose a reason for hiding this comment

DanielCChen Mar 14, 2025

Choose a reason for hiding this comment

DanielCChen commented Mar 14, 2025

Meinersbur commented Mar 14, 2025 • edited Loading

DanielCChen commented Mar 14, 2025

Meinersbur commented Mar 12, 2025 •

edited

Loading

Meinersbur commented Mar 13, 2025 •

edited

Loading

DanielCChen commented Mar 13, 2025 •

edited

Loading

Meinersbur commented Mar 14, 2025 •

edited

Loading