Skip to content

[libc] Remove workarounds for lack of functional NVPTX linker #96972

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
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions libc/cmake/modules/LLVMLibCObjectRules.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
25 changes: 8 additions & 17 deletions libc/cmake/modules/LLVMLibCTestRules.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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}
)
Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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}
)
Expand All @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is asdfasdfasdf something that's specific to LLVM or AMDGPU? If so, what's the story behind the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops.

Copy link
Contributor Author

@jhuber6 jhuber6 Jul 24, 2024

Choose a reason for hiding this comment

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

Okay nevermind, I never noticed this broke the AMDGPU bot because it broke for another reason around the same time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, interesting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That made it into the LLVM 19 fork, how embarrassing.

"-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"
Expand All @@ -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
Expand Down
15 changes: 11 additions & 4 deletions libc/config/gpu/entrypoints.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
if(LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
set(extra_entrypoints
# stdio.h entrypoints
libc.src.stdio.snprintf
libc.src.stdio.sprintf
libc.src.stdio.vsnprintf
libc.src.stdio.vsprintf
)
endif()

set(TARGET_LIBC_ENTRYPOINTS
# assert.h entrypoints
libc.src.assert.__assert_fail
Expand Down Expand Up @@ -176,16 +186,13 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.errno.errno

# stdio.h entrypoints
${extra_entrypoints}
libc.src.stdio.clearerr
libc.src.stdio.fclose
libc.src.stdio.printf
libc.src.stdio.vprintf
libc.src.stdio.fprintf
libc.src.stdio.vfprintf
libc.src.stdio.sprintf
libc.src.stdio.snprintf
libc.src.stdio.vsprintf
libc.src.stdio.vsnprintf
libc.src.stdio.feof
libc.src.stdio.ferror
libc.src.stdio.fflush
Expand Down
9 changes: 1 addition & 8 deletions libc/test/UnitTest/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Loading