CompilerRT: Normalize COMPILER_RT_DEFAULT_TARGET_TRIPLE #88835
Conversation
If LLVM is configured with -DLLVM_DEFAULT_TARGET_TRIPLE, or compiler_rt is configured with -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE, while the argument is not normalized, such as Debian-style vendor-less triple, clang will try to find libclang_rt in lib/<normalized_triple>, while libclang_rt is placed into lib/<triple_arg>. Let's also place libclang_rt into lib/<normalized_triple>.
|
The previous PR introduced 2 test fails: https://lab.llvm.org/buildbot/#/builders/77/builds/36372 |
…)" This reverts commit 16f1887.
|
As this is a reland of an old patch, it would be good to link explicitly to the previously relanded commit or PR, or even the commit where the previous one was reverted - as things stand right now, it's not easy to go from this PR review to the previous one. Anyway, this patch did break my build... I agree with the idea of this patch, but When building compiler-rt builtins for the We can patch around that by with the following patch: diff --git a/compiler-rt/cmake/builtin-config-ix.cmake b/compiler-rt/cmake/builtin-config-ix.cmake
index 33c97b1ac28a..cf02f47e4c1e 100644
--- a/compiler-rt/cmake/builtin-config-ix.cmake
+++ b/compiler-rt/cmake/builtin-config-ix.cmake
@@ -53,7 +53,7 @@ else()
endif()
set(ARM64 aarch64)
-set(ARM32 arm armhf armv4t armv5te armv6 armv6m armv7m armv7em armv7 armv7s armv7k armv8m.base armv8m.main armv8.1m.main)
+set(ARM32 arm armhf armv4t armv5te armv6 armv6m armv7m armv7em armv7 armv7s armv7k armv8m.base armv8m.main armv8.1m.main thumbv7)
set(AVR avr)
set(HEXAGON hexagon)
set(X86 i386)
diff --git a/compiler-rt/lib/builtins/CMakeLists.txt b/compiler-rt/lib/builtins/CMakeLists.txt
index f9611574a562..e94912103755 100644
--- a/compiler-rt/lib/builtins/CMakeLists.txt
+++ b/compiler-rt/lib/builtins/CMakeLists.txt
@@ -615,6 +615,7 @@ set(armhf_SOURCES ${arm_SOURCES})
set(armv7_SOURCES ${arm_SOURCES})
set(armv7s_SOURCES ${arm_SOURCES})
set(armv7k_SOURCES ${arm_SOURCES})
+set(thumbv7_SOURCES ${arm_SOURCES})
set(arm64_SOURCES ${aarch64_SOURCES})
set(arm64e_SOURCES ${aarch64_SOURCES})
set(arm64_32_SOURCES ${aarch64_SOURCES})However, for the purposes of this patch, getting the right normalized triple for the output of the build so clang will find it, this target also behaves weirdly: $ clang --target=armv7-w64-windows-gnu -print-effective-triple
thumbv7-w64-windows-gnu
$ clang --target=armv7-w64-windows-gnu -print-runtime-dir
<prefix>/lib/clang/19/lib/armv7-w64-windows-gnuSo for this target, the output of So I'd go ahead and revert this for now, so we can unbreak my continous builds at e.g. https://github.com/mstorsjo/llvm-mingw/actions/runs/8730621159 that were broken by this. |
|
@MaskRay Can you comment on the core of this issue, i.e. this: $ clang --target=armv7-w64-windows-gnu -print-effective-triple
thumbv7-w64-windows-gnu
$ clang --target=armv7-w64-windows-gnu -print-runtime-dir
<prefix>/lib/clang/19/lib/armv7-w64-windows-gnuWe could work around this by applying a replacement back from |
|
Oh, and also for the non-per-target runtime directory config, even if we patch the builtins build to recognize |
|
Thanks for digging Martin, I was pinged about this too today. This also broke libcxx Picolib checks at the install step. This may be because this build does not use the per target runtime dir at all, being an embedded build, not sure yet. https://buildkite.com/llvm-project/libcxx-ci/builds/34897#018eec06-612c-47f1-9931-d3bd88bf7ced To reproduce this you'll need qemu-system-arm 8.1.3 or newer installed, set CC/CXX to clang 17/18 (latest is ideal) and then from the llvm-project dir run The triple ends up converted to thumbv7: And I don't see any sign of either triple in any folder names in the |
…)" This reverts commit 16f1887. This broke Libcxx Picolib testing at the install step, and builds for Windows builtins. Revert while we figure out the cause.
|
Thanks @DavidSpickett! It's good to see that this issue isn't specific for the windows target - I guess it goes for any |
|
Sorry for this trouble. I misunderstanding the usage of |
|
@mstorsjo @DavidSpickett can you have a try to use |
That does seem to do the right thing: |
|
There still some problem for
Is it correct? Should we fix the C++ code to normalize triples?
|
|
Which is more normal, however I still get: That subdir doesn't exist but the one clang printed does: Maybe something in the install step also needs to be updated? Also it is a bit weird that this is adding a PER_TARGET_RUNTIME_DIR style folder path, in a build that doesn't enable that setting. But I don't have the full context so I'm not sure that's wrong necessarily. |
Yes. It is hardcoded in
See #87866 |
You're right, this isn't part of the compiler-rt cmake at all, and it does this too: Which explains the layout.
I'd rather be explicit and specify the path. One less layer to unpick when it goes wrong. The following fixes the libcxx build: So does -print-target-triple:
|
Yes. The previous is wrong due to I misunderstood
I will file a new PR. And add Martin as reviewer. |
|
If you add the libcxx build script changes to that PR it should be run by the libcxx CI so we can confirm that that works too. |
|
Yup, or you could also just |
If LLVM is configured with -DLLVM_DEFAULT_TARGET_TRIPLE, or compiler_rt is configured with -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE, while the argument is not normalized, such as Debian-style vendor-less triple, clang will try to find libclang_rt in lib/<normalized_triple>, while libclang_rt is placed into lib/<triple_arg>. Let's also place libclang_rt into lib/<normalized_triple>. `libcxx/utils/ci/run-buildbot` is also updated to use `armv7m-none-unknown-eabi` as normalized triple instead of current `armv7m-none-eabi`. ChangeLog of this PR: This patch has been applied and revert twice: 1. The first try is #88407, and then it is found that it causes some CI failures. https://lab.llvm.org/buildbot/#/builders/98/builds/36366 It is then resolved by another commit: 1693009 https://lab.llvm.org/buildbot/#/builders/77/builds/36372 It is caused that `COMPILER_RT_DEFAULT_TARGET_TRIPLE` is overwrite without taking care about `CACHE`. 2. The second try #88835, resolves https://lab.llvm.org/buildbot/#/builders/77/builds/36372 and in fact only one `execute_process` is needed. Then we find some other CI failures. https://github.com/mstorsjo/llvm-mingw/actions/runs/8730621159 https://buildkite.com/llvm-project/libcxx-ci/builds/34897#018eec06-612c-47f1-9931-d3bd88bf7ced It is due to misunderstanding `-print-effective-triple`: which will output `thumbv7-w64-windows-gnu` for `armv7-w64-windows-gnu` or some other thumb-enabled arm triples. In fact we should use `-print-target-triple`. For armv7m-picolibc, `armv7m-none-eabi` is hardcoded in libcxx/utils/ci/run-buildbot, while in fact `armv7m-none-unknown-eabi` is the real normalized triple.
If LLVM is configured with -DLLVM_DEFAULT_TARGET_TRIPLE, or compiler_rt is configured with -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE, while the argument is not normalized, such as Debian-style vendor-less triple, clang will try to find libclang_rt in lib/<normalized_triple>, while libclang_rt is placed into lib/<triple_arg>.
Let's also place libclang_rt into lib/<normalized_triple>.