[libc-commits] [libc] [libc] Fix GPU tests not running after recent patches (PR #77248)

Joseph Huber via libc-commits libc-commits at lists.llvm.org
Sun Jan 7 11:25:32 PST 2024


https://github.com/jhuber6 updated https://github.com/llvm/llvm-project/pull/77248

>From 370821677d33dd907adb9e09390fa6a7edb5557a Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Sun, 7 Jan 2024 12:59:55 -0600
Subject: [PATCH 1/3] [libc] Fix GPU tests not running after recent patches

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.
---
 libc/cmake/modules/LLVMLibCTestRules.cmake   | 21 ++++++----
 libc/include/CMakeLists.txt                  | 41 +++++++++++---------
 libc/src/__support/StringUtil/CMakeLists.txt | 34 ++++++++--------
 3 files changed, 55 insertions(+), 41 deletions(-)

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()

>From 7272ec08def3f41e869480d5fc1031f763dd0c82 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Sun, 7 Jan 2024 13:15:49 -0600
Subject: [PATCH 2/3] Fix path

---
 libc/include/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 596787a377a653..24d27888e5a441 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -185,7 +185,7 @@ add_gen_header(
 )
 
 
-if(EXISTS ../config/${LIBC_TARGET_OS}/signal.h.in)
+if(EXISTS "../config/${LIBC_TARGET_OS}/signal.h.in")
   add_gen_header(
     signal
     DEF_FILE signal.h.def

>From 8b078b01ac7542b8ca98f40402a95d738ca4a2cc Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Sun, 7 Jan 2024 13:25:18 -0600
Subject: [PATCH 3/3] Fix on Linux

---
 libc/include/CMakeLists.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 24d27888e5a441..7067d004c53380 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -185,15 +185,15 @@ add_gen_header(
 )
 
 
-if(EXISTS "../config/${LIBC_TARGET_OS}/signal.h.in")
+if(EXISTS "${LIBC_SOURCE_DIR}/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
+    platform_signal=${LIBC_SOURCE_DIR}/config/${LIBC_TARGET_OS}/signal.h.in
     GEN_HDR signal.h
     DATA_FILES
-      ../config/${LIBC_TARGET_OS}/signal.h.in
+      ${LIBC_SOURCE_DIR}/config/${LIBC_TARGET_OS}/signal.h.in
     DEPENDS
       .llvm-libc-macros.signal_macros
       .llvm-libc-types.sig_atomic_t



More information about the libc-commits mailing list