Skip to content

[LLVM][runtimes] Prepopulate LLVM_BUILTIN_TARGETS with runtimes values #95554

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 1 commit into from
Jul 12, 2024
Merged

[LLVM][runtimes] Prepopulate LLVM_BUILTIN_TARGETS with runtimes values #95554

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 38 additions & 26 deletions llvm/runtimes/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ foreach(proj ${LLVM_ENABLE_RUNTIMES})
endforeach()

function(get_compiler_rt_path path)
foreach(entry ${runtimes})
set(all_runtimes ${runtimes})
foreach(name ${LLVM_RUNTIME_TARGETS})
list(APPEND all_runtimes ${RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES})
endforeach()
list(REMOVE_DUPLICATES all_runtimes)
foreach(entry ${all_runtimes})
get_filename_component(projName ${entry} NAME)
if("${projName}" MATCHES "compiler-rt")
set(${path} ${entry} PARENT_SCOPE)
Expand Down Expand Up @@ -138,37 +143,44 @@ endfunction()
# before the just-built compiler can pass the configuration tests.
get_compiler_rt_path(compiler_rt_path)
if(compiler_rt_path)
if(NOT LLVM_BUILTIN_TARGETS)
# If the user did not specify the targets infer them from the runtimes.
set(builtin_targets ${LLVM_BUILTIN_TARGETS})
if(NOT builtin_targets)
if("compiler-rt" IN_LIST LLVM_ENABLE_RUNTIMES)
list(APPEND builtin_targets "default")
endif()
Comment on lines +149 to +151
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that applying DRY here is worth it. I'd rather just duplicate the builtin_default_target invocation and remove the string handling for "default".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the following case to work,

   -DLLVM_ENABLE_RUNTIMES=compiler-rt \
   -DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=compiler-rt               \
   -DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=compiler-rt                 \
   -DLLVM_RUNTIME_TARGETS="default;amdgcn-amd-amdhsa;nvptx64-nvidia-cuda"

Honestly I think we should get rid of the distinction between the "default" target, but that's a much larger refactoring I think @petrhosek wanted to do.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The "default" target is also important for how the compiler-rt parts of this are built on Darwin, which is a mess.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that applying DRY here is worth it. I'd rather just duplicate the builtin_default_target invocation and remove the string handling for "default".

I have a WIP change that removes the "default" altogether since that part is a relic, but it needs some more testing to make sure I don't break any of existing configurations.

foreach(name ${LLVM_RUNTIME_TARGETS})
if("compiler-rt" IN_LIST RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES)
list(APPEND builtin_targets ${name})
endif()
endforeach()
endif()
if("default" IN_LIST builtin_targets)
builtin_default_target(${compiler_rt_path}
DEPENDS clang-resource-headers)
list(REMOVE_ITEM builtin_targets "default")
else()
if("default" IN_LIST LLVM_BUILTIN_TARGETS)
builtin_default_target(${compiler_rt_path}
DEPENDS clang-resource-headers)
list(REMOVE_ITEM LLVM_BUILTIN_TARGETS "default")
else()
add_custom_target(builtins)
add_custom_target(install-builtins)
add_custom_target(install-builtins-stripped)
set_target_properties(
builtins install-builtins install-builtins-stripped
PROPERTIES FOLDER "Compiler-RT"
)
endif()
add_custom_target(builtins)
add_custom_target(install-builtins)
add_custom_target(install-builtins-stripped)
set_target_properties(
builtins install-builtins install-builtins-stripped
PROPERTIES FOLDER "Compiler-RT"
)
endif()

foreach(target ${LLVM_BUILTIN_TARGETS})
check_apple_target(${target} builtin)
foreach(target ${builtin_targets})
check_apple_target(${target} builtin)

builtin_register_target(${compiler_rt_path} ${target}
DEPENDS clang-resource-headers
CMAKE_ARGS -DLLVM_DEFAULT_TARGET_TRIPLE=${target}
EXTRA_ARGS TARGET_TRIPLE ${target})
builtin_register_target(${compiler_rt_path} ${target}
DEPENDS clang-resource-headers
CMAKE_ARGS -DLLVM_DEFAULT_TARGET_TRIPLE=${target}
EXTRA_ARGS TARGET_TRIPLE ${target})

add_dependencies(builtins builtins-${target})
add_dependencies(install-builtins install-builtins-${target})
add_dependencies(install-builtins-stripped install-builtins-${target}-stripped)
endforeach()
endif()
add_dependencies(builtins builtins-${target})
add_dependencies(install-builtins install-builtins-${target})
add_dependencies(install-builtins-stripped install-builtins-${target}-stripped)
endforeach()
set(builtins_dep builtins)
# We don't need to depend on the builtins if we're building instrumented
# because the next stage will use the same compiler used to build this stage.
Expand Down
Loading