[Darwin][CompilerRT] Set compiler ID to 'Clang' for Compiler RT Tests for Apple Clang #121858
Conversation
This isn't the place impacted tho, right? We'd need to change all of those cases to check for AppleClang as well? This seems not optimal. I'm just trying to get the previous behavior back so our tests can run again. I'm not entirely sure the PR to not detect as GCC needed to change the previous behavior of setting clang strings to clang.. |
The point was to try to reduce the special case paths. If there is a specific reason that we cannot treat Doing that in two phases where we first restore the behaviour and then follow up with a change to make it follow the same behaviour is fine too. But the idea is that we should try to bring the configuration into the same path if possible or document why it needs to be different. |
But this change "i.e. pass through the identity unmodified" is very new behavior added a month ago, and there are various places that don't assume that. Historically we matched any clang regex and set the compiler ID to Clang? This patch simply returns to a long standing assumption to avoid breaking tests or changing what flags are passed. Not changing this would require a special path in lit config (no big deal) but also various CMake files where we check that |
There was a problem hiding this comment.
Ah I missed the config.available_features.add(compiler_id). I agree we should normalize this to clang either in cmake or the top-level lit config file.
I'd prefer to do the canonicalization at the end of this if-else chain for future proofing, i.e
if ("${COMPILER_RT_TEST_COMPILER_ID}" STREQUAL "AppleClang")
# Compiler-rt lit tests expect compiler ID 'Clang' for AppleClang.
set(COMPILER_RT_TEST_COMPILER_ID Clang)
endif()
but I guess the current one is fine too.
|
Sorry about the breakage, I had forgotten at the time that cmake uses a different compiler id for apple change. |
I'm fine with this. Thanks very much! |
This patch restores previous behavior we relied on. Even when the Compiler ID is set to AppleClang, we expect Compiler RT Tests to use Clang as the compiler ID. This impacts various make and lit commands. Caused by: https://github.com/llvm/llvm-project/pull/117812/files rdar://141548700
thetruestblue
force-pushed
the
truesblue/compiler_rt_appleclang_compilerid
branch
from
2107264 to
a97a8c8
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/4291 Here is the relevant piece of the build log for the reference |
This patch restores previous behavior. Even when the Compiler ID is set to AppleClang, we expect Compiler RT Tests to use Clang as the compiler ID. This impacts various make and lit commands.
Caused by: https://github.com/llvm/llvm-project/pull/117812/files
rdar://141548700