Skip to content
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.

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

[LLVM] Add 'LLVM_EXTRA_RUNTIME_TARGETS' to append to default list #82513

Closed
wants to merge 1 commit into from

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Feb 21, 2024

Summary:
One problem with using the standard LLVM_RUNTIME_TARGETS is that it
requires setting 'default' manually. This is very good for use-cases
where the user wants to completely override their build, but it is bad
for builds where we want to augment the normal list. This is because it
requires changing things other than the projects that require it. This
patch introduces LLVM_EXTRA_RUNTIME_TARGETS which is only expected to
append to the default list. This patch also fixes the use-case of
libc using this new potentially different list.

This unblocks #81921 so
hopefully this is less contentious and can be merged soon.

Summary:
One problem with using the standard `LLVM_RUNTIME_TARGETS` is that it
requires setting 'default' manually. This is very good for use-cases
where the user wants to completely override their build, but it is bad
for builds where we want to augment the normal list. This is because it
requires changing things other than the projects that require it. This
patch introduces `LLVM_EXTRA_RUNTIME_TARGETS` which is only expected to
append to the default list. This patch also fixes the use-case of
`libc` using this new potentially different list.

This unblocks llvm#81921 so
hopefully this is less contentious and can be merged soon.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 21, 2024

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
One problem with using the standard LLVM_RUNTIME_TARGETS is that it
requires setting 'default' manually. This is very good for use-cases
where the user wants to completely override their build, but it is bad
for builds where we want to augment the normal list. This is because it
requires changing things other than the projects that require it. This
patch introduces LLVM_EXTRA_RUNTIME_TARGETS which is only expected to
append to the default list. This patch also fixes the use-case of
libc using this new potentially different list.

This unblocks #81921 so
hopefully this is less contentious and can be merged soon.


Full diff: https://github.com/llvm/llvm-project/pull/82513.diff

3 Files Affected:

  • (modified) libc/CMakeLists.txt (+7-2)
  • (modified) llvm/CMakeLists.txt (+10-8)
  • (modified) llvm/runtimes/CMakeLists.txt (+3-3)
diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 3d775736616745..bafc111dd31046 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -57,9 +57,14 @@ if(LLVM_LIBC_FULL_BUILD OR LIBC_GPU_BUILD OR LIBC_GPU_ARCHITECTURES)
   endif()
 endif()
 
+foreach(name default ${LLVM_RUNTIME_TARGETS} ${LLVM_EXTRA_RUNTIME_TARGETS})
+  if("libc" IN_LIST LLVM_ENABLE_RUNTIMES OR "libc" IN_LIST RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES)
+    set(LIBC_RUNTIMES_BUILD ON)
+  endif()
+endforeach()
+
 option(LIBC_HDRGEN_ONLY "Only build the 'libc-hdrgen' executable" OFF)
-if(("libc" IN_LIST LLVM_ENABLE_RUNTIMES AND NOT LLVM_RUNTIMES_BUILD) OR 
-   LIBC_HDRGEN_ONLY)
+if((LIBC_RUNTIMES_BUILD AND NOT LLVM_RUNTIMES_BUILD) OR LIBC_HDRGEN_ONLY)
   # When libc is build as part of the runtimes/bootstrap build's CMake run, we
   # only need to build the host tools to build the libc. So, we just do enough
   # to build libc-hdrgen and return.
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 98cef005f07e09..f57228eb0f2c84 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -168,15 +168,17 @@ foreach(proj IN LISTS LLVM_ENABLE_RUNTIMES)
   endif()
 endforeach()
 
-if ("libc" IN_LIST LLVM_ENABLE_RUNTIMES)
-  # To build the libc runtime, we need to be able to build few libc build
-  # tools from the "libc" project. So, we add it to the list of enabled
-  # projects.
-  if (NOT "libc" IN_LIST LLVM_ENABLE_PROJECTS)
-    message(STATUS "Enabling libc project to build libc build tools")
-    list(APPEND LLVM_ENABLE_PROJECTS "libc")
+foreach(name default ${LLVM_RUNTIME_TARGETS} ${LLVM_EXTRA_RUNTIME_TARGETS})
+  if("libc" IN_LIST LLVM_ENABLE_RUNTIMES OR "libc" IN_LIST RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES)
+    # To build the libc runtime, we need to be able to build few libc build
+    # tools from the "libc" project. So, we add it to the list of enabled
+    # projects.
+    if(NOT "libc" IN_LIST LLVM_ENABLE_PROJECTS)
+      message(STATUS "Enabling libc project to build libc build tools")
+      list(APPEND LLVM_ENABLE_PROJECTS "libc")
+    endif()
   endif()
-endif()
+endforeach()
 
 # LLVM_ENABLE_PROJECTS_USED is `ON` if the user has ever used the
 # `LLVM_ENABLE_PROJECTS` CMake cache variable.  This exists for
diff --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index 8c48d85a4346f4..035849020f9904 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -449,14 +449,14 @@ if(runtimes)
       endforeach()
     endif()
   endif()
-  if(NOT LLVM_RUNTIME_TARGETS)
+  if(NOT LLVM_RUNTIME_TARGETS AND NOT LLVM_EXTRA_RUNTIME_TARGETS)
     runtime_default_target(
       DEPENDS ${builtins_dep} ${extra_deps}
       CMAKE_ARGS ${libc_cmake_args}
       PREFIXES ${prefixes})
     set(test_targets check-runtimes)
   else()
-    if("default" IN_LIST LLVM_RUNTIME_TARGETS)
+    if("default" IN_LIST LLVM_RUNTIME_TARGETS OR LLVM_EXTRA_RUNTIME_TARGETS)
       runtime_default_target(
         DEPENDS ${builtins_dep} ${extra_deps}
         CMAKE_ARGS ${libc_cmake_args}
@@ -481,7 +481,7 @@ if(runtimes)
       endif()
     endif()
 
-    foreach(name ${LLVM_RUNTIME_TARGETS})
+    foreach(name ${LLVM_RUNTIME_TARGETS} ${LLVM_EXTRA_RUNTIME_TARGETS})
       if(builtins_dep)
         if (LLVM_BUILTIN_TARGETS)
           set(builtins_dep_name "${builtins_dep}-${name}")

@jhuber6
Copy link
Contributor Author

jhuber6 commented Feb 21, 2024

If this lands the expected way to enable the GPU libc would become the following.

    -DRUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES=libc   
    -DRUNTIMES_amdgcn-amd-amdhsa_LLVM_ENABLE_RUNTIMES=libc 
    -DLLVM_EXTRA_RUNTIME_TARGETS=amdgcn-amd-amdhsa;nvptx64-nvidia-cuda

@jplehr
Copy link
Contributor

jplehr commented Feb 26, 2024

This removes the need to have default specified as target, right?

@petrhosek
Copy link
Member

I'm working on a larger refactor that eliminates the default as a special target altogether without a need to introduce additional CMake option.

@jhuber6 jhuber6 closed this Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants