Skip to content

[libc] Fix standard cross build targeting the GPU #82724

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

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Feb 23, 2024

Summary:
The GPU target has recently been changed to support standard libc
build rules. This means we should be able to build for it both in
LLVM_ENABLE_PROJECTS mode, or targeting the runtimes directory
directly as in the LLVM libc documentation. Previously this failed
because the version check on the compiler was too strict and the
--target= options were not being set on the link jobs unless in CMake
cross compiliation mode. This patch fixes those so the following config
should work now to build the GPU target directly if using NVPTX.

cmake ../runtimes -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang \
  -DLLVM_ENABLE_RUNTIMES=libc -DLLVM_RUNTIMES_TARGET=nvptx64-nvidia-cuda \
  -DLLVM_DEFAULT_TARGET_TRIPLE=nvptx64-nvidia-cuda \
  -DLIBC_HDRGEN_EXE=/path/to/hdrgen/libc-hdrgen \
  -DLLVM_LIBC_FULL_BUILD=ON -GNinja

Summary:
The GPU target has recently been changed to support standard `libc`
build rules. This means we should be able to build for it both in
`LLVM_ENABLE_PROJECTS` mode, or targeting the runtimes directory
directly as in the LLVM `libc` documentation. Previously this failed
because the version check on the compiler was too strict and the
`--target=` options were not being set on the link jobs unless in CMake
cross compiliation mode. This patch fixes those so the following config
should work now to build the GPU target directly if using NVPTX.

```
cmake ../runtimes -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang \
  -DLLVM_ENABLE_RUNTIMES=libc -DLLVM_RUNTIMES_TARGET=nvptx64-nvidia-cuda \
  -DLLVM_DEFAULT_TARGET_TRIPLE=nvptx64-nvidia-cuda \
  -DLIBC_HDRGEN_EXE=/path/to/hdrgen/libc-hdrgen \
  -DLLVM_LIBC_FULL_BUILD=ON -GNinja
```
@llvmbot
Copy link
Member

llvmbot commented Feb 23, 2024

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
The GPU target has recently been changed to support standard libc
build rules. This means we should be able to build for it both in
LLVM_ENABLE_PROJECTS mode, or targeting the runtimes directory
directly as in the LLVM libc documentation. Previously this failed
because the version check on the compiler was too strict and the
--target= options were not being set on the link jobs unless in CMake
cross compiliation mode. This patch fixes those so the following config
should work now to build the GPU target directly if using NVPTX.

cmake ../runtimes -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang \
  -DLLVM_ENABLE_RUNTIMES=libc -DLLVM_RUNTIMES_TARGET=nvptx64-nvidia-cuda \
  -DLLVM_DEFAULT_TARGET_TRIPLE=nvptx64-nvidia-cuda \
  -DLIBC_HDRGEN_EXE=/path/to/hdrgen/libc-hdrgen \
  -DLLVM_LIBC_FULL_BUILD=ON -GNinja

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

2 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+4)
  • (modified) libc/cmake/modules/prepare_libc_gpu_build.cmake (+2-2)
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 1166c26e4d8a56..9d1e426a5b7b31 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -463,6 +463,7 @@ function(add_integration_test test_name)
 
   if(LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
     target_link_options(${fq_build_target_name} PRIVATE 
+      ${LIBC_COMPILE_OPTIONS_DEFAULT}
       -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto
       "-Wl,-mllvm,-amdgpu-lower-global-ctor-dtor=0" -nostdlib -static
       "-Wl,-mllvm,-amdhsa-code-object-version=${LIBC_GPU_CODE_OBJECT_VERSION}")
@@ -470,6 +471,7 @@ function(add_integration_test test_name)
     # We need to use the internal object versions for NVPTX.
     set(internal_suffix ".__internal__")
     target_link_options(${fq_build_target_name} PRIVATE 
+      ${LIBC_COMPILE_OPTIONS_DEFAULT}
       "-Wl,--suppress-stack-size-warning"
       -march=${LIBC_GPU_TARGET_ARCHITECTURE} -nostdlib -static
       "--cuda-path=${LIBC_CUDA_ROOT}")
@@ -644,6 +646,7 @@ function(add_libc_hermetic_test test_name)
 
   if(LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
     target_link_options(${fq_build_target_name} PRIVATE 
+      ${LIBC_COMPILE_OPTIONS_DEFAULT}
       -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto
       "-Wl,-mllvm,-amdgpu-lower-global-ctor-dtor=0" -nostdlib -static
       "-Wl,-mllvm,-amdhsa-code-object-version=${LIBC_GPU_CODE_OBJECT_VERSION}")
@@ -651,6 +654,7 @@ function(add_libc_hermetic_test test_name)
     # We need to use the internal object versions for NVPTX.
     set(internal_suffix ".__internal__")
     target_link_options(${fq_build_target_name} PRIVATE 
+      ${LIBC_COMPILE_OPTIONS_DEFAULT}
       "-Wl,--suppress-stack-size-warning"
       -march=${LIBC_GPU_TARGET_ARCHITECTURE} -nostdlib -static
       "--cuda-path=${LIBC_CUDA_ROOT}")
diff --git a/libc/cmake/modules/prepare_libc_gpu_build.cmake b/libc/cmake/modules/prepare_libc_gpu_build.cmake
index 75beef86760c8d..8e5d1bb93d77cf 100644
--- a/libc/cmake/modules/prepare_libc_gpu_build.cmake
+++ b/libc/cmake/modules/prepare_libc_gpu_build.cmake
@@ -5,8 +5,8 @@ endif()
 
 # Ensure the compiler is a valid clang when building the GPU target.
 set(req_ver "${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")
-if(NOT (CMAKE_CXX_COMPILER_ID MATCHES "[Cc]lang" AND
-        ${CMAKE_CXX_COMPILER_VERSION} VERSION_EQUAL "${req_ver}"))
+if(LLVM_VERSION_MAJOR AND NOT (CMAKE_CXX_COMPILER_ID MATCHES "[Cc]lang" AND
+   ${CMAKE_CXX_COMPILER_VERSION} VERSION_EQUAL "${req_ver}"))
   message(FATAL_ERROR "Cannot build libc for GPU. CMake compiler "
                       "'${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}' "
                       " is not 'Clang ${req_ver}'.")

@jhuber6 jhuber6 merged commit 640ba3f into llvm:main Feb 23, 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.

3 participants