[libc] Remove special full build handling for GPU

[jhuber6]

Summary:
Currently we default to non-fullbuild for all targets, but realistically
we should do this depending on the target OS. Some OS's like the GPU or
upcoming UEFI have no existing hosted system, so they cannot be built
with an overlay build. These are already errors so there's no reason to
complicate things and require passing it in through the runtimes build.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
Currently we default to non-fullbuild for all targets, but realistically
we should do this depending on the target OS. Some OS's like the GPU or
upcoming UEFI have no existing hosted system, so they cannot be built
with an overlay build. These are already errors so there's no reason to
complicate things and require passing it in through the runtimes build.

---

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

2 Files Affected:

• (modified) libc/CMakeLists.txt (+7-1)
• (modified) llvm/runtimes/CMakeLists.txt (-2)

diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 47708c2267818..a4fbba92c2727 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -128,7 +128,13 @@ else()
   endif()
 endif()
 
-option(LLVM_LIBC_FULL_BUILD "Build and test LLVM libc as if it is the full libc" OFF)
+# Some targets can only support the full build.
+set(default_to_full_build OFF)
+if(LIBC_TARGET_OS_IS_GPU)
+  set(default_to_full_build ON)
+endif()
+
+option(LLVM_LIBC_FULL_BUILD "Build and test LLVM libc as if it is the full libc" ${default_to_full_build})
 option(LLVM_LIBC_IMPLEMENTATION_DEFINED_TEST_BEHAVIOR "Build LLVM libc tests assuming our implementation-defined behavior" ON)
 option(LLVM_LIBC_ENABLE_LINTING "Enables linting of libc source files" OFF)
 
diff --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index 2370b41fb7f0b..77a82ed196cd9 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -535,7 +535,6 @@ if(build_runtimes)
              "-DRUNTIMES_amdgcn-amd-amdhsa_LIBC_GPU_LOADER_EXECUTABLE=$<TARGET_FILE:amdhsa-loader>")
         list(APPEND extra_deps amdhsa-loader)
       endif()
-      list(APPEND extra_cmake_args "-DRUNTIMES_amdgcn-amd-amdhsa_LLVM_LIBC_FULL_BUILD=ON")
     endif()
     if("libc" IN_LIST RUNTIMES_nvptx64-nvidia-cuda_LLVM_ENABLE_RUNTIMES)
       if(TARGET nvptx-loader)
@@ -543,7 +542,6 @@ if(build_runtimes)
              "-DRUNTIMES_nvptx64-nvidia-cuda_LIBC_GPU_LOADER_EXECUTABLE=$<TARGET_FILE:nvptx-loader>")
         list(APPEND extra_deps nvptx-loader)
       endif()
-      list(APPEND extra_cmake_args "-DRUNTIMES_nvptx64-nvidia-cuda_LLVM_LIBC_FULL_BUILD=ON")
     endif()
     if(TARGET clang-offload-packager)
       list(APPEND extra_deps clang-offload-packager)

[JonChesterfield]

I like it. Cleaner and means the spirv work in progress has a decent chance of doing the right thing instead of refusing to build.

[lntue]

hmm, why don't you just overwrite the LLVM_LIBC_FULL_BUILD directly after defining option?

[jhuber6]

Couldn't decide if that was better since this allows the user the ability to manually override it and see a nice error that it doesn't work.

[lntue]

I'd prefer to just message that default to full build mode with LIBC_TARGET_OS_IS_GPU and then overwrite it, so that we don't have to introduce extra variable.

[jhuber6]

I could just make it ${LIBC_OS_IS_GPU} for now, that's a one liner.

[lntue]

then it will not be so clean for other targets that want to default to full build also. Something like:

option(LLVM_LIBC_FULL_BUILD "Build and test LLVM libc as if it is the full libc" OFF)
if(LIBC_TARGET_OS_IS_GPU)
  message(STATUS "Default to full build mode.")
  set(LLVM_LIBC_FULL_BUILD ON)
endif()

should be clearer to users.

[jhuber6]

I would've preferred allowing the user to override the default behavior, but I guess I could make this change and then also remove the static check elsewhere.

[jhuber6]

Also because resetting a cache variable is a pain because you need to force it. Whereas this is just an extra normal variable. Do I really need to do it this way?

[lntue]

nah, that's ok.

[jplehr]

This broke the libc for GPU builder:

https://lab.llvm.org/buildbot/#/builders/73/builds/13524