[libc] Remove workarounds for lack of functional NVPTX linker #96972
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Joseph Huber (jhuber6) ChangesSummary: Depends on #96561 Full diff: https://github.com/llvm/llvm-project/pull/96972.diff 3 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
index 1f80e7f4e57c1..e44e262b83a27 100644
--- a/libc/cmake/modules/LLVMLibCObjectRules.cmake
+++ b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -65,25 +65,6 @@ function(create_object_library fq_target_name)
target_include_directories(${fq_target_name} PRIVATE ${LIBC_SOURCE_DIR})
target_compile_options(${fq_target_name} PRIVATE ${compile_options})
- # The NVPTX target is installed as LLVM-IR but the internal testing toolchain
- # cannot handle it natively. Make a separate internal target for testing.
- if(LIBC_TARGET_ARCHITECTURE_IS_NVPTX AND NOT LIBC_GPU_TESTS_DISABLED)
- add_library(
- ${internal_target_name}
- EXCLUDE_FROM_ALL
- OBJECT
- ${ADD_OBJECT_SRCS}
- ${ADD_OBJECT_HDRS}
- )
- target_include_directories(${internal_target_name} SYSTEM PRIVATE ${LIBC_INCLUDE_DIR})
- target_include_directories(${internal_target_name} PRIVATE ${LIBC_SOURCE_DIR})
- target_compile_options(${internal_target_name} PRIVATE ${compile_options}
- -fno-lto -march=${LIBC_GPU_TARGET_ARCHITECTURE})
- set_target_properties(${internal_target_name}
- PROPERTIES
- CXX_STANDARD ${ADD_OBJECT_CXX_STANDARD})
- endif()
-
if(SHOW_INTERMEDIATE_OBJECTS)
message(STATUS "Adding object library ${fq_target_name}")
if(${SHOW_INTERMEDIATE_OBJECTS} STREQUAL "DEPS")
@@ -283,12 +264,6 @@ function(create_entrypoint_object fq_target_name)
add_dependencies(${internal_target_name} ${full_deps_list})
target_link_libraries(${internal_target_name} ${full_deps_list})
- # The NVPTX target cannot use LTO for the internal targets used for testing.
- if(LIBC_TARGET_ARCHITECTURE_IS_NVPTX)
- target_compile_options(${internal_target_name} PRIVATE
- -fno-lto -march=${LIBC_GPU_TARGET_ARCHITECTURE})
- endif()
-
add_library(
${fq_target_name}
# We want an object library as the objects will eventually get packaged into
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index fbeec32883b63..d17ebb1287b4b 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -453,8 +453,6 @@ function(add_integration_test test_name)
add_executable(
${fq_build_target_name}
EXCLUDE_FROM_ALL
- # The NVIDIA 'nvlink' linker does not currently support static libraries.
- $<$<BOOL:${LIBC_TARGET_ARCHITECTURE_IS_NVPTX}>:${link_object_files}>
${INTEGRATION_TEST_SRCS}
${INTEGRATION_TEST_HDRS}
)
@@ -473,8 +471,6 @@ function(add_integration_test test_name)
"-Wl,-mllvm,-amdgpu-lower-global-ctor-dtor=0" -nostdlib -static
"-Wl,-mllvm,-amdhsa-code-object-version=${LIBC_GPU_CODE_OBJECT_VERSION}")
elseif(LIBC_TARGET_ARCHITECTURE_IS_NVPTX)
- # 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} -Wno-multi-gpu
"-Wl,--suppress-stack-size-warning"
@@ -490,10 +486,9 @@ function(add_integration_test test_name)
endif()
target_link_libraries(
${fq_build_target_name}
- # The NVIDIA 'nvlink' linker does not currently support static libraries.
- $<$<NOT:$<BOOL:${LIBC_TARGET_ARCHITECTURE_IS_NVPTX}>>:${fq_target_name}.__libc__>
- libc.startup.${LIBC_TARGET_OS}.crt1${internal_suffix}
- libc.test.IntegrationTest.test${internal_suffix}
+ ${fq_target_name}.__libc__
+ libc.startup.${LIBC_TARGET_OS}.crt1
+ libc.test.IntegrationTest.test
)
add_dependencies(${fq_build_target_name}
libc.test.IntegrationTest.test
@@ -628,8 +623,6 @@ function(add_libc_hermetic test_name)
add_executable(
${fq_build_target_name}
EXCLUDE_FROM_ALL
- # The NVIDIA 'nvlink' linker does not currently support static libraries.
- $<$<BOOL:${LIBC_TARGET_ARCHITECTURE_IS_NVPTX}>:${link_object_files}>
${HERMETIC_TEST_SRCS}
${HERMETIC_TEST_HDRS}
)
@@ -656,13 +649,12 @@ function(add_libc_hermetic 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 -Wno-multi-gpu
+ ${LIBC_COMPILE_OPTIONS_DEFAULT} -Wno-multi-gpu
+ -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto
+ "-Wl,-asdfasdfasdf"
"-Wl,-mllvm,-amdgpu-lower-global-ctor-dtor=0" -nostdlib -static
"-Wl,-mllvm,-amdhsa-code-object-version=${LIBC_GPU_CODE_OBJECT_VERSION}")
elseif(LIBC_TARGET_ARCHITECTURE_IS_NVPTX)
- # 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} -Wno-multi-gpu
"-Wl,--suppress-stack-size-warning"
@@ -679,11 +671,10 @@ function(add_libc_hermetic test_name)
target_link_libraries(
${fq_build_target_name}
PRIVATE
- libc.startup.${LIBC_TARGET_OS}.crt1${internal_suffix}
+ libc.startup.${LIBC_TARGET_OS}.crt1
${link_libraries}
LibcHermeticTestSupport.hermetic
- # The NVIDIA 'nvlink' linker does not currently support static libraries.
- $<$<NOT:$<BOOL:${LIBC_TARGET_ARCHITECTURE_IS_NVPTX}>>:${fq_target_name}.__libc__>)
+ ${fq_target_name}.__libc__)
add_dependencies(${fq_build_target_name}
LibcTest.hermetic
libc.test.UnitTest.ErrnoSetterMatcher
diff --git a/libc/test/UnitTest/CMakeLists.txt b/libc/test/UnitTest/CMakeLists.txt
index 4adc2f5c725f7..6427b86158077 100644
--- a/libc/test/UnitTest/CMakeLists.txt
+++ b/libc/test/UnitTest/CMakeLists.txt
@@ -11,17 +11,10 @@ function(add_unittest_framework_library name)
"header only libraries, use 'add_header_library'")
endif()
- # The Nvidia 'nvlink' linker does not support static libraries.
- if(LIBC_TARGET_ARCHITECTURE_IS_NVPTX)
- set(library_type OBJECT)
- else()
- set(library_type STATIC)
- endif()
-
foreach(lib IN ITEMS ${name}.unit ${name}.hermetic)
add_library(
${lib}
- ${library_type}
+ STATIC
EXCLUDE_FROM_ALL
${TEST_LIB_SRCS}
${TEST_LIB_HDRS}
|
michaelrj-google
approved these changes
Jun 27, 2024
Summary: Currently we have several hacks to work around the fact that the NVPTX linker, 'nvlink', does not support static libraries or LTO linking. The patch in llvm#96561 introduces a wrapper in the toolchain that allows us to use a standard `ld.lld` like interface. This means all the divergence with this target can be removed. Depends on llvm#96561
| -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto -Wno-multi-gpu | ||
| ${LIBC_COMPILE_OPTIONS_DEFAULT} -Wno-multi-gpu | ||
| -mcpu=${LIBC_GPU_TARGET_ARCHITECTURE} -flto | ||
| "-Wl,-asdfasdfasdf" |
There was a problem hiding this comment.
Is asdfasdfasdf something that's specific to LLVM or AMDGPU? If so, what's the story behind the name?
There was a problem hiding this comment.
Okay nevermind, I never noticed this broke the AMDGPU bot because it broke for another reason around the same time.
There was a problem hiding this comment.
That made it into the LLVM 19 fork, how embarrassing.
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
Summary: Currently we have several hacks to work around the fact that the NVPTX linker, 'nvlink', does not support static libraries or LTO linking. The patch in #96561 introduces a wrapper in the toolchain that allows us to use a standard `ld.lld` like interface. This means all the divergence with this target can be removed. Depends on #96561
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Currently we have several hacks to work around the fact that the NVPTX
linker, 'nvlink', does not support static libraries or LTO linking.
The patch in #96561 introduces
a wrapper in the toolchain that allows us to use a standard
ld.lldlike interface. This means all the divergence with this target can be
removed.
Depends on #96561