Skip to content

[CMake] Use Clang to infer the target triple #89425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 6, 2024
Merged

[CMake] Use Clang to infer the target triple #89425

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb24598
[CMake] Use Clang to infer the target triple
petrhosek Feb 16, 2022
2c89c50
Merge remote-tracking branch 'origin/main' into runtimes-clang-resour…
petrhosek Jun 25, 2024
302fa32
Update the patch to incorporate changes from #89234
petrhosek Jun 25, 2024
5044cde
Add a TODO comment
petrhosek Jul 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/cmake/caches/Fuchsia-stage2.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ if(WIN32 OR LLVM_WINSYSROOT)
set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS ${WINDOWS_LINK_FLAGS} CACHE STRING "")
endif()

foreach(target aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unknown-linux-gnu;riscv64-unknown-linux-gnu;x86_64-unknown-linux-gnu)
foreach(target aarch64-linux-gnu;armv7-linux-gnueabihf;i386-linux-gnu;riscv64-linux-gnu;x86_64-linux-gnu)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems offtopic of commit msg?
Since the normalize format of aarch64-linux-gnu is just aarch64-unknown-linux-gnu, why do we need to modify them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I included this as a regression test, without this change the build of our toolchain would break, but I'm fine omitting this if you prefer.

if(LINUX_${target}_SYSROOT)
# Set the per-target builtins options.
list(APPEND BUILTIN_TARGETS "${target}")
Expand Down
16 changes: 12 additions & 4 deletions compiler-rt/cmake/Modules/CompilerRTUtils.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,14 +368,22 @@ macro(construct_compiler_rt_default_triple)
"Default triple for which compiler-rt runtimes will be built.")
endif()

if ("${CMAKE_C_COMPILER_ID}" MATCHES "Clang")
if(CMAKE_C_COMPILER_ID MATCHES "Clang")
set(option_prefix "")
if (CMAKE_C_SIMULATE_ID MATCHES "MSVC")
set(option_prefix "/clang:")
endif()
execute_process(COMMAND ${CMAKE_C_COMPILER} ${option_prefix}--target=${COMPILER_RT_DEFAULT_TARGET_TRIPLE} ${option_prefix}-print-target-triple
OUTPUT_VARIABLE COMPILER_RT_DEFAULT_TARGET_TRIPLE
OUTPUT_STRIP_TRAILING_WHITESPACE)
set(print_target_triple ${CMAKE_C_COMPILER} ${option_prefix}--target=${COMPILER_RT_DEFAULT_TARGET_TRIPLE} ${option_prefix}-print-target-triple)
execute_process(COMMAND ${print_target_triple}
RESULT_VARIABLE result
OUTPUT_VARIABLE output
OUTPUT_STRIP_TRAILING_WHITESPACE)
if(result EQUAL 0)
set(COMPILER_RT_DEFAULT_TARGET_TRIPLE ${output})
else()
string(REPLACE ";" " " print_target_triple "${print_target_triple}")
message(WARNING "Failed to execute `${print_target_triple}` to normalize target triple.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it should be a fetal error instead of just a warning?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we ignore any errors so my preference would be to start with a warning and change it to an error in a follow up change after we verify there are no issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you are right. I tested with some quite bad --target values, it will output the original values.
So, I think that it will be safe. Anyway, a warning is OK for me as I think that it won't be used at all.

endif()
endif()

string(REPLACE "-" ";" LLVM_TARGET_TRIPLE_LIST ${COMPILER_RT_DEFAULT_TARGET_TRIPLE})
Expand Down
18 changes: 18 additions & 0 deletions runtimes/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,24 @@ message(STATUS "LLVM default target triple: ${LLVM_DEFAULT_TARGET_TRIPLE}")

set(LLVM_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}")

if(CMAKE_C_COMPILER_ID MATCHES "Clang")
set(option_prefix "")
if (CMAKE_C_SIMULATE_ID MATCHES "MSVC")
set(option_prefix "/clang:")
endif()
set(print_target_triple ${CMAKE_C_COMPILER} ${option_prefix}--target=${LLVM_DEFAULT_TARGET_TRIPLE} ${option_prefix}-print-target-triple)
execute_process(COMMAND ${print_target_triple}
RESULT_VARIABLE result
OUTPUT_VARIABLE output
OUTPUT_STRIP_TRAILING_WHITESPACE)
if(result EQUAL 0)
set(LLVM_DEFAULT_TARGET_TRIPLE ${output})
else()
string(REPLACE ";" " " print_target_triple "${print_target_triple}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto. I guess that we need to test on more target. As some target doesn't use the new filesystem struct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since most of the CIs monitor the commit only. I have no idea how to trigger them before commit.

message(WARNING "Failed to execute `${print_target_triple}` to normalize target triple.")
endif()
endif()

option(LLVM_INCLUDE_TESTS "Generate build targets for the runtimes unit tests." ON)
option(LLVM_INCLUDE_DOCS "Generate build targets for the runtimes documentation." ON)
option(LLVM_ENABLE_SPHINX "Use Sphinx to generate the runtimes documentation." OFF)
Expand Down
Loading