[compiler-rt] Produce the right arch suffix for arm libraries

[mstorsjo]

Summary

If producing libraries with an arch suffix (i.e. if
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR isn't set), we append the
architecture name. However, for arm, clang doesn't look for libraries
with the full architecture name, but only looks for "arm" and "armhf".

Try to deduce what the full target triple might have been, and use
that for deciding between "arm" and "armhf".

This tries to reapply this bit from D98173, that had to be reverted
in 7b153b43d3a14d76975039408c4b922beb576735 due to affecting how
the builtins themselves are compiled, not only affecting the output
file name.

[mstorsjo]

Ping @phosek @MaskRay - hopefully the dust has settled after the patches last week, so we could try out this one, if it looks good to you.

[phosek]

LGTM

[manojgupta]

Sorry for replying since this patch has been there for a while. But this has broken the installs for the baremetal targets eg armv7m-none-eabi.

Clang expects builtins at a different location:
$ clang -target armv7m-none-eabi -print-libgcc-file-name -rtlib=compiler-rt
/usr/lib64/clang/13.0.0/lib/baremetal/libclang_rt.builtins-armv7m.a. <this is the expected location>

Previous location before the patch was correct:

/usr/lib64/clang/13.0.0/lib/baremetal/libclang_rt.builtins-armv7m.a

New location after this patch:
/usr/lib64/clang/13.0.0/lib/baremetal/libclang_rt.builtins-arm.a

See that we have lost the architecture differentiability. We can no longer have armv6m and armv7m installed in same location. This also does not match the "-print-libgcc-file-name" location.

Setting LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does not help either. It changes installation location to /usr/lib64/clang/14.0.0/lib/armv7m-none-eabi/libclang_rt.builtins.a which is better for installing multiple ABIS.
But "-print-libgcc-file-name" does not detect it, it still prints /usr/lib64/clang/13.0.0/lib/baremetal/libclang_rt.builtins-armv7m.a. .

[mstorsjo]

In D98452#3113198, @manojgupta wrote:

Sorry for replying since this patch has been there for a while. But this has broken the installs for the baremetal targets eg armv7m-none-eabi.

Sorry for the breakage. Now I see the source for the issue.

Have a look at getArchNameForCompilerRTLib in clang/lib/Driver/ToolChain.cpp:

static StringRef getArchNameForCompilerRTLib(const ToolChain &TC,
                                             const ArgList &Args) {
  const llvm::Triple &Triple = TC.getTriple();
  bool IsWindows = Triple.isOSWindows();

  if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb)
    return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows)
               ? "armhf"
               : "arm";
[...]

This gets called from ToolChain::buildCompilerRTBasename. Thus, when using this implementation, Clang only ever looks for libraries with the name libclang_rt.builtins-arm.a (and -armhf) . It does indeed lack differentiability between architecture versions - but I changed compiler-rt to produce the names that Clang expects.

The unexpected issue is that the BareMetal toolchain overrides buildCompilerRTBasename, like this:

std::string BareMetal::buildCompilerRTBasename(const llvm::opt::ArgList &,
                                               StringRef, FileType,
                                               bool) const {
  return ("libclang_rt.builtins-" + getTriple().getArchName() + ".a").str();
}

Thus here the exact architecture name gets used.

Can we detect whether we are targeting the baremetal OS in the compiler-rt cmake codepath that I touched (I would expect that we can, as it installs things into a /baremetal/ subdirectory), and avoid the architecture name normalization in that case?

Setting LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does not help either. It changes installation location to /usr/lib64/clang/14.0.0/lib/armv7m-none-eabi/libclang_rt.builtins.a which is better for installing multiple ABIS.

But "-print-libgcc-file-name" does not detect it, it still prints /usr/lib64/clang/13.0.0/lib/baremetal/libclang_rt.builtins-armv7m.a. .

That sounds like a separate issue that woul be great to fix, independently of this issue. The per-target runtime directory setup probably is better overall, but we should still keep both setups working.

[manojgupta]

Yes, you can check for COMPILER_RT_BAREMETAL_BUILD in cmake to keep the older naming for baremetal. The baremetal targets have very different driver searching mechanisms for libs, not sure why.

[mstorsjo]

In D98452#3113745, @manojgupta wrote:

Yes, you can check for COMPILER_RT_BAREMETAL_BUILD in cmake to keep the older naming for baremetal. The baremetal targets have very different driver searching mechanisms for libs, not sure why.

Ah, nice. Are you able to try to make a patch for it? I could give it a try, but I'd be writing it mostly without really trying it out...

[manojgupta]

Sure, see D113357

compiler-rt/cmake/Modules/AddCompilerRT.cmake

 endfunction()
 
 macro(set_output_name output name arch)
   if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
     set(${output} ${name})
   else()
     if(ANDROID AND ${arch} STREQUAL "i386")
       set(${output} "${name}-i686${COMPILER_RT_OS_SUFFIX}")
+    elseif("${arch}" MATCHES "^arm")
+      if(COMPILER_RT_DEFAULT_TARGET_ONLY)
+        set(triple "${COMPILER_RT_DEFAULT_TARGET_TRIPLE}")
+      else()
+        set(triple "${TARGET_TRIPLE}")
+      endif()
+      # When using arch-suffixed runtime library names, clang only looks for
+      # libraries named "arm" or "armhf", see getArchNameForCompilerRTLib in
+      # clang. Therefore, try to inspect both the arch name and the triple
+      # if it seems like we're building an armhf target.
+      if ("${arch}" MATCHES "hf$" OR "${triple}" MATCHES "hf$")
+        set(${output} "${name}-armhf${COMPILER_RT_OS_SUFFIX}")
+      else()
+        set(${output} "${name}-arm${COMPILER_RT_OS_SUFFIX}")
+      endif()
     else()
       set(${output} "${name}-${arch}${COMPILER_RT_OS_SUFFIX}")
     endif()
   endif()
 endmacro()
 
 # Adds static or shared runtime for a list of architectures and operating
 # systems and puts it in the proper directory in the build and install trees.