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

[libc] Fix libc-hdrgen crosscompiling #78227

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

petrhosek
Copy link
Member

The support introduced in 675702f356b0c3a540fa2e8af4192f7d658b2988 is not working correctly in all scenarios. Instead of setup_host_tool function, we can use the existing targets introduced by add_tablegen macro.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 16, 2024

@llvm/pr-subscribers-libc

Author: Petr Hosek (petrhosek)

Changes

The support introduced in 675702f356b0c3a540fa2e8af4192f7d658b2988 is not working correctly in all scenarios. Instead of setup_host_tool function, we can use the existing targets introduced by add_tablegen macro.


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

3 Files Affected:

  • (modified) libc/CMakeLists.txt (+4)
  • (modified) libc/utils/HdrGen/CMakeLists.txt (-2)
  • (modified) llvm/runtimes/CMakeLists.txt (+14-6)
diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 9c82db754b5f73..fd1f945455d8e2 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -34,6 +34,10 @@ if(LLVM_LIBC_FULL_BUILD OR LIBC_GPU_BUILD OR LIBC_GPU_ARCHITECTURES)
     # We need to set up hdrgen first since other targets depend on it.
     add_subdirectory(utils/LibcTableGenUtil)
     add_subdirectory(utils/HdrGen)
+    # Calling add_tablegen sets variables like LIBC_TABLEGEN_EXE in
+    # PARENT_SCOPE which get lost until saved in the cache.
+    set(LIBC_TABLEGEN_EXE "${LIBC_TABLEGEN_EXE}" CACHE INTERNAL "")
+    set(LIBC_TABLEGEN_TARGET "${LIBC_TABLEGEN_TARGET}" CACHE INTERNAL "")
   else()
     message(STATUS "Will use ${LIBC_HDRGEN_EXE} for libc header generation.")
   endif()
diff --git a/libc/utils/HdrGen/CMakeLists.txt b/libc/utils/HdrGen/CMakeLists.txt
index cfadd040df41e9..0ec1cba542d400 100644
--- a/libc/utils/HdrGen/CMakeLists.txt
+++ b/libc/utils/HdrGen/CMakeLists.txt
@@ -18,5 +18,3 @@ target_include_directories(libc-hdrgen PRIVATE ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_I
 target_link_libraries(libc-hdrgen PRIVATE LibcTableGenUtil)
 
 add_subdirectory(PrototypeTestGen)
-
-setup_host_tool(libc-hdrgen LIBC_HDRGEN libc_hdrgen_exe libc_hdrgen_target)
diff --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index a41945eb113b2f..8c48d85a4346f4 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -425,14 +425,22 @@ if(runtimes)
   endif()
   if("libc" IN_LIST LLVM_ENABLE_PROJECTS AND
       (LLVM_LIBC_FULL_BUILD OR LIBC_GPU_BUILD OR LIBC_GPU_ARCHITECTURES))
-    get_host_tool_path(libc-hdrgen LIBC_HDRGEN libc_hdrgen_exe libc_hdrgen_target)
-    if(NOT libc_hdrgen_exe)
+    if(LIBC_HDRGEN_EXE)
+      set(hdrgen_exe ${LIBC_HDRGEN_EXE})
+    else()
+      if(TARGET ${LIBC_TABLEGEN_EXE})
+        set(hdrgen_exe $<TARGET_FILE:${LIBC_TABLEGEN_EXE}>)
+      else()
+        set(hdrgen_exe ${LIBC_TABLEGEN_EXE})
+      endif()
+      set(hdrgen_deps ${LIBC_TABLEGEN_TARGET})
+    endif()
+    if(NOT hdrgen_exe)
       message(FATAL_ERROR "libc-hdrgen executable missing")
     endif()
-    set(libc_cmake_args "-DLIBC_HDRGEN_EXE=${libc_hdrgen_exe}"
+    set(libc_cmake_args "-DLIBC_HDRGEN_EXE=${hdrgen_exe}"
                         "-DLLVM_LIBC_FULL_BUILD=ON")
-    set(libc_tools libc_hdrgen_target)
-    list(APPEND extra_deps ${libc_hdrgen_target})
+    list(APPEND extra_deps ${hdrgen_deps})
     if(LIBC_GPU_BUILD OR LIBC_GPU_ARCHITECTURES)
       foreach(dep clang-offload-packager nvptx-arch amdgpu-arch)
         if(TARGET ${dep})
@@ -485,7 +493,7 @@ if(runtimes)
       check_apple_target(${name} runtime)
 
       runtime_register_target(${name}
-        DEPENDS ${builtins_dep_name} ${libc_tools}
+        DEPENDS ${builtins_dep_name} ${hdrgen_deps}
         CMAKE_ARGS -DLLVM_DEFAULT_TARGET_TRIPLE=${name} ${libc_cmake_args}
         EXTRA_ARGS TARGET_TRIPLE ${name})
     endforeach()

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 16, 2024

Fails with this locally.

[0/5] Building native libc-hdrgen...
ninja: error: unknown target 'libc-hdrgen'
FAILED: NATIVE/bin/libc-hdrgen /home/jhuber/Documents/llvm/llvm-project/build/NATIVE/bin/libc-hdrgen 
cd /home/jhuber/Documents/llvm/llvm-project/build/NATIVE && /usr/bin/cmake --build /home/jhuber/Documents/llvm/llvm-project/build/NATIVE --target libc-hdrgen --config Release
ninja: build stopped: subcommand failed.

My CMake is pretty close to the following, you could probably remove a lot of stuff.

cmake ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra;compiler-rt;lld" -DLLVM_ENABLE_RUNTIMES="openmp;libc;libunwind" -DLLVM_ENABLE_ASSERTIONS=ON -DLIBOMPTARGET_ENABLE_DEBUG=ON -DLIBOMPTARGET_DEVICE_ARCHITECTURES="gfx1030;sm_60;sm_89" -DLLVM_USE_LINKER=lld -DCLANG_DEFAULT_LINKER=lld -DLIBC_GPU_ARCHITECTURES="sm_89;gfx1030" -DLIBC_GPU_TEST_JOBS=1 -DLLVM_OPTIMIZED_TABLEGEN=ON -DBUILD_SHARED_LIBS=ON -DLLVM_CCACHE_BUILD=ON -DLLVM_APPEND_VC_REV=OFF -G Ninja && ninja

@petrhosek
Copy link
Member Author

I suspect it's related to -DLLVM_OPTIMIZED_TABLEGEN=ON but I cannot reproduce this locally using your CMake options, was this with clean or incremental build?

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 16, 2024

I suspect it's related to -DLLVM_OPTIMIZED_TABLEGEN=ON but I cannot reproduce this locally using your CMake options, was this with clean or incremental build?

I did a clean build and still got

[1475/6997] Building native libc-hdrgen...
ninja: error: unknown target 'libc-hdrgen'
[1494/6997] Creating library symlink lib/libLLVMCodeGenTypes.so
FAILED: NATIVE/bin/libc-hdrgen /home/jhuber/Documents/llvm/llvm-project/build/NATIVE/bin/libc-hdrgen 
cd /home/jhuber/Documents/llvm/llvm-project/build/NATIVE && /usr/bin/cmake --build /home/jhuber/Documents/llvm/llvm-project/build/NATIVE --target libc-hdrgen --config Release

Curious that you can't reproduce it, here's a script that I used originally https://gist.github.com/jhuber6/c439286bf0da51905649b334efac02ee.

@petrhosek
Copy link
Member Author

Never mind, I can reproduce it now, I was too eager at dropping flags...

The support introduced in 675702f356b0c3a540fa2e8af4192f7d658b2988 is
not working correctly in all scenarios. Instead of setup_host_tool
function, we can use the existing targets introduced by add_tablegen
macro.
@petrhosek petrhosek force-pushed the libc-hdrgen-crosscompiling-fix branch from 2d87260 to b26dabe Compare January 16, 2024 06:30
@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Jan 16, 2024
@petrhosek
Copy link
Member Author

The issue you ran into should be addressed in the latest revision.

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

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

Thanks, fixed my issues.

@jhuber6 jhuber6 merged commit c20811b into llvm:main Jan 16, 2024
5 checks passed
@petrhosek petrhosek deleted the libc-hdrgen-crosscompiling-fix branch January 16, 2024 18:38
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
The support introduced in 675702f356b0c3a540fa2e8af4192f7d658b2988 is
not working correctly in all scenarios. Instead of setup_host_tool
function, we can use the existing targets introduced by add_tablegen
macro.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular libc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants