[libc] Fix GPU tests not running after recent patches #77248
Merged
Conversation
This file contains 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: Another patch, because the build-bot was down, snuck in that prevented Full diff: https://github.com/llvm/llvm-project/pull/77248.diff 3 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index b69839afebf8a1..24f543f6e4c132 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -498,10 +498,14 @@ function(add_integration_test test_name)
libc.src.string.memcpy
libc.src.string.memmove
libc.src.string.memset
- # __stack_chk_fail should always be included to allow building libc with
- # stack protector.
- libc.src.compiler.__stack_chk_fail
)
+
+ if(libc.src.compiler.__stack_chk_fail IN_LIST TARGET_LLVMLIBC_ENTRYPOINTS)
+ # __stack_chk_fail should always be included if supported to allow building
+ # libc with the stack protector enabled.
+ list(APPEND fq_deps_list libc.src.compiler.__stack_chk_fail)
+ endif()
+
list(REMOVE_DUPLICATES fq_deps_list)
# TODO: Instead of gathering internal object files from entrypoints,
@@ -668,12 +672,15 @@ function(add_libc_hermetic_test test_name)
libc.src.string.memmove
libc.src.string.memset
libc.src.__support.StringUtil.error_to_string
- # __stack_chk_fail should always be included to allow building libc with
- # stack protector.
- libc.src.compiler.__stack_chk_fail
)
- if(TARGET libc.src.time.clock)
+ if(libc.src.compiler.__stack_chk_fail IN_LIST TARGET_LLVMLIBC_ENTRYPOINTS)
+ # __stack_chk_fail should always be included if supported to allow building
+ # libc with the stack protector enabled.
+ list(APPEND fq_deps_list libc.src.compiler.__stack_chk_fail)
+ endif()
+
+ if(libc.src.time.clock IN_LIST TARGET_LLVMLIBC_ENTRYPOINTS)
# We will link in the 'clock' implementation if it exists for test timing.
list(APPEND fq_deps_list libc.src.time.clock)
endif()
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 59c6c4a9bb4200..596787a377a653 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -184,24 +184,29 @@ add_gen_header(
.llvm-libc-macros.generic_error_number_macros
)
-add_gen_header(
- signal
- DEF_FILE signal.h.def
- PARAMS
- platform_signal=../config/${LIBC_TARGET_OS}/signal.h.in
- GEN_HDR signal.h
- DATA_FILES
- ../config/${LIBC_TARGET_OS}/signal.h.in
- DEPENDS
- .llvm-libc-macros.signal_macros
- .llvm-libc-types.sig_atomic_t
- .llvm-libc-types.sigset_t
- .llvm-libc-types.struct_sigaction
- .llvm-libc-types.union_sigval
- .llvm-libc-types.siginfo_t
- .llvm-libc-types.stack_t
- .llvm-libc-types.pid_t
-)
+
+if(EXISTS ../config/${LIBC_TARGET_OS}/signal.h.in)
+ add_gen_header(
+ signal
+ DEF_FILE signal.h.def
+ PARAMS
+ platform_signal=../config/${LIBC_TARGET_OS}/signal.h.in
+ GEN_HDR signal.h
+ DATA_FILES
+ ../config/${LIBC_TARGET_OS}/signal.h.in
+ DEPENDS
+ .llvm-libc-macros.signal_macros
+ .llvm-libc-types.sig_atomic_t
+ .llvm-libc-types.sigset_t
+ .llvm-libc-types.struct_sigaction
+ .llvm-libc-types.union_sigval
+ .llvm-libc-types.siginfo_t
+ .llvm-libc-types.stack_t
+ .llvm-libc-types.pid_t
+ )
+else()
+ message(STATUS "Skipping header signal.h as the target config is missing")
+endif()
add_gen_header(
stdio
diff --git a/libc/src/__support/StringUtil/CMakeLists.txt b/libc/src/__support/StringUtil/CMakeLists.txt
index c053966d54181b..41b20dc2cb1171 100644
--- a/libc/src/__support/StringUtil/CMakeLists.txt
+++ b/libc/src/__support/StringUtil/CMakeLists.txt
@@ -51,19 +51,21 @@ add_object_library(
libc.src.__support.integer_to_string
)
-add_object_library(
- signal_to_string
- HDRS
- signal_to_string.h
- SRCS
- signal_to_string.cpp
- DEPENDS
- .message_mapper
- .platform_signals
- libc.include.signal
- libc.src.__support.common
- libc.src.__support.CPP.span
- libc.src.__support.CPP.string_view
- libc.src.__support.CPP.stringstream
- libc.src.__support.integer_to_string
-)
+if(TARGET libc.include.signal)
+ add_object_library(
+ signal_to_string
+ HDRS
+ signal_to_string.h
+ SRCS
+ signal_to_string.cpp
+ DEPENDS
+ .message_mapper
+ .platform_signals
+ libc.include.signal
+ libc.src.__support.common
+ libc.src.__support.CPP.span
+ libc.src.__support.CPP.string_view
+ libc.src.__support.CPP.stringstream
+ libc.src.__support.integer_to_string
+ )
+endif()
|
Summary: A previous patch added a dependency on the stack protectors, this was not built on the GPU targets so every test was disabled. It turns out that disabled tests still get targets so we need to specifically check if the it is in the target's set of entrypoints before we can use it. Another patch, because the build-bot was down, snuck in that prevented the new math tests from being run. The problem is that the `signal.h` header requires target specific definitions but was being used unconditionally. I have made changes that disable building this header if the file is not defined in the config. This required disbaling the signal_to_string utility, so that will simply be missing from targets that don't define it. Fix path Fix on Linux
lntue
approved these changes
Jan 7, 2024
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
Summary: A previous patch added a dependency on the stack protectors, this was not built on the GPU targets so every test was disabled. It turns out that disabled tests still get targets so we need to specifically check if the it is in the target's set of entrypoints before we can use it. Another patch, because the build-bot was down, snuck in that prevented the new math tests from being run. The problem is that the `signal.h` header requires target specific definitions but was being used unconditionally. I have made changes that disable building this header if the file is not defined in the config. This required disbaling the signal_to_string utility, so that will simply be missing from targets that don't define it.
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:
A previous patch added a dependency on the stack protectors, this was
not built on the GPU targets so every test was disabled. It turns out
that disabled tests still get targets so we need to specifically check
if the it is in the target's set of entrypoints before we can use it.
Another patch, because the build-bot was down, snuck in that prevented
the new math tests from being run. The problem is that the
signal.hheader requires target specific definitions but was being used
unconditionally. I have made changes that disable building this header
if the file is not defined in the config. This required disbaling the
signal_to_string utility, so that will simply be missing from targets
that don't define it.