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] Attempt to enable missing string functions for offloading #67869

Closed
wants to merge 1 commit into from

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Sep 30, 2023

Summary:
We previously had to disable these string functions because they were
not compatible with the definitions coming from the GNU / host
environment. The GPU, when exporting its declarations, has a very
difficult requirement that it be compatible with the host environment as
both sides of the compilation need to agree on definitions and what's
present.

For string.h, some functions have different defintions for C and C++.
GNU would optionally provide the C++ definitions directly in the
string.h file. This patch attempts to hack around this by tricking the
GNU headers into thinking that the clang used it too old to support
their special definition handling. The blast radius from this hack
should be somewhat minimal, only changing C++ behaviour for a few
functions. The only noticable change is that we lose const error
checking for a small handful of functions if the user includes
string.h directly in C++ mode instead of .

Summary:
We previously had to disable these string functions because they were
not compatible with the definitions coming from the GNU / host
environment. The GPU, when exporting its declarations, has a very
difficult requirement that it be compatible with the host environment as
both sides of the compilation need to agree on definitions and what's
present.

For `string.h`, some functions have different defintions for C and C++.
GNU would optionally provide the C++ definitions directly in the
`string.h` file. This patch attempts to hack around this by tricking the
GNU headers into thinking that the `clang` used it too old to support
their special definition handling. The blast radius from this hack
should be somewhat minimal, only changing C++ behaviour for a few
functions. The only noticable change is that we lose const error
checking for a small handful of functions if the user includes
`string.h` directly in C++ mode instead of <cstring>.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics libc labels Sep 30, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 30, 2023

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-libc

@llvm/pr-subscribers-clang

Changes

Summary:
We previously had to disable these string functions because they were
not compatible with the definitions coming from the GNU / host
environment. The GPU, when exporting its declarations, has a very
difficult requirement that it be compatible with the host environment as
both sides of the compilation need to agree on definitions and what's
present.

For string.h, some functions have different defintions for C and C++.
GNU would optionally provide the C++ definitions directly in the
string.h file. This patch attempts to hack around this by tricking the
GNU headers into thinking that the clang used it too old to support
their special definition handling. The blast radius from this hack
should be somewhat minimal, only changing C++ behaviour for a few
functions. The only noticable change is that we lose const error
checking for a small handful of functions if the user includes
string.h directly in C++ mode instead of <cstring>.


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

3 Files Affected:

  • (modified) clang/lib/Headers/llvm_libc_wrappers/string.h (+24-3)
  • (modified) libc/config/gpu/entrypoints.txt (+16)
  • (modified) libc/docs/gpu/support.rst (+19-8)
diff --git a/clang/lib/Headers/llvm_libc_wrappers/string.h b/clang/lib/Headers/llvm_libc_wrappers/string.h
index 027c415c1d0f8f3..0fa6d81fda36b61 100644
--- a/clang/lib/Headers/llvm_libc_wrappers/string.h
+++ b/clang/lib/Headers/llvm_libc_wrappers/string.h
@@ -13,11 +13,32 @@
 #error "This file is for GPU offloading compilation only"
 #endif
 
-// FIXME: The GNU headers provide C++ standard compliant headers when in C++
-// mode and the LLVM libc does not. We cannot enable memchr, strchr, strchrnul,
-// strpbrk, strrchr, strstr, or strcasestr until this is addressed.
+// The GNU headers provide C++ standard compliant headers when in C++ mode and
+// the LLVM libc does not. We need to perform a pretty nasty hack to trick the
+// GNU headers into emitting the C compatible definitions so we can use them.
+#if defined(__cplusplus) && defined(__GLIBC__)
+
+// We need to make sure that the GNU C library has done its setup before we mess
+// with the expected macro values.
+#if !defined(__GLIBC_INTERNAL_STARTING_HEADER_IMPLEMENTATION) &&               \
+    __has_include(<bits/libc-header-start.h>)
+#define __GLIBC_INTERNAL_STARTING_HEADER_IMPLEMENTATION
+#include <bits/libc-header-start.h>
+#endif
+
+// Trick the GNU headers into thinking that this clang is too old for the C++
+// definitions.
+#pragma push_macro("__clang_major__")
+#define __clang_major__ 3
+#endif
+
 #include_next <string.h>
 
+// Resore the original macros if they were changed.
+#if defined(__cplusplus) && defined(__GLIBC__)
+#pragma pop_macro("__clang_major__")
+#endif
+
 #if __has_include(<llvm-libc-decls/string.h>)
 
 #if defined(__HIP__) || defined(__CUDA__)
diff --git a/libc/config/gpu/entrypoints.txt b/libc/config/gpu/entrypoints.txt
index ad68216a76b9429..dcaca15b500cbe9 100644
--- a/libc/config/gpu/entrypoints.txt
+++ b/libc/config/gpu/entrypoints.txt
@@ -22,21 +22,31 @@ set(TARGET_LIBC_ENTRYPOINTS
 
     # string.h entrypoints
     libc.src.string.bcmp
+    libc.src.string.bcopy
     libc.src.string.bzero
+    libc.src.string.index
     libc.src.string.memccpy
+    libc.src.string.memchr
     libc.src.string.memcmp
     libc.src.string.memcpy
     libc.src.string.memmem
     libc.src.string.memmove
     libc.src.string.mempcpy
+    libc.src.string.memrchr
     libc.src.string.memset
+    libc.src.string.rindex
     libc.src.string.stpcpy
     libc.src.string.stpncpy
     libc.src.string.strcasecmp
+    libc.src.string.strcasestr
     libc.src.string.strcat
+    libc.src.string.strchr
+    libc.src.string.strchrnul
     libc.src.string.strcmp
+    libc.src.string.strcoll
     libc.src.string.strcpy
     libc.src.string.strcspn
+    libc.src.string.strdup
     libc.src.string.strlcat
     libc.src.string.strlcpy
     libc.src.string.strlen
@@ -44,10 +54,16 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.string.strncat
     libc.src.string.strncmp
     libc.src.string.strncpy
+    libc.src.string.strndup
     libc.src.string.strnlen
+    libc.src.string.strpbrk
+    libc.src.string.strrchr
+    libc.src.string.strsep
     libc.src.string.strspn
+    libc.src.string.strstr
     libc.src.string.strtok
     libc.src.string.strtok_r
+    libc.src.string.strxfrm
 
     # stdlib.h entrypoints
     libc.src.stdlib.abs
diff --git a/libc/docs/gpu/support.rst b/libc/docs/gpu/support.rst
index fd27273ed562e49..beca7b587a05215 100644
--- a/libc/docs/gpu/support.rst
+++ b/libc/docs/gpu/support.rst
@@ -45,37 +45,48 @@ string.h
 Function Name  Available  RPC Required
 =============  =========  ============
 bcmp           |check|
+bcopy          |check|
 bzero          |check|
+index          |check|
 memccpy        |check|
-memchr         
+memchr         |check|
 memcmp         |check|
 memcpy         |check|
+memmem         |check|
 memmove        |check|
 mempcpy        |check|
-memrchr
+memrchr        |check|
 memset         |check|
+rindex         |check|
 stpcpy         |check|
 stpncpy        |check|
+strcasecmp     |check|
+strcasestr     |check|
 strcat         |check|
-strchr         
+strchr         |check|
+strchrnul      |check|
 strcmp         |check|
+strcoll        |check|
 strcpy         |check|
 strcspn        |check|
+strdup         |check|
 strlcat        |check|
 strlcpy        |check|
 strlen         |check|
+strncasecmp    |check|
 strncat        |check|
 strncmp        |check|
 strncpy        |check|
+strndup        |check|
 strnlen        |check|
-strpbrk        
-strrchr        
+strpbrk        |check|
+strrchr        |check|
+strsep         |check|
 strspn         |check|
-strstr         
+strstr         |check|
 strtok         |check|
 strtok_r       |check|
-strdup
-strndup
+strxfrm        |check|
 =============  =========  ============
 
 stdlib.h

@jhuber6
Copy link
Contributor Author

jhuber6 commented Oct 25, 2023

Invalidated by a merged PR.

@jhuber6 jhuber6 closed this Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category libc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants