Skip to content

Conversation

@jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Oct 5, 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.

This patch more or less gives up an just copies the definitions as
expected by glibc if they are provided that way, otherwise we fall
back to the accepted way. This is the alternative solution to an
existing PR which instead disable's GCC's handling.

@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 Oct 5, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 5, 2023

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

@llvm/pr-subscribers-libc

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.

This patch more or less gives up an just copies the definitions as
expected by glibc if they are provided that way, otherwise we fall
back to the accepted way. This is the alternative solution to an
existing PR which instead disable's GCC's handling.


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

3 Files Affected:

  • (modified) clang/lib/Headers/llvm_libc_wrappers/string.h (+70-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..49c8234d7bed492 100644 --- a/clang/lib/Headers/llvm_libc_wrappers/string.h +++ b/clang/lib/Headers/llvm_libc_wrappers/string.h @@ -13,9 +13,6 @@ #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. #include_next <string.h> #if __has_include(<llvm-libc-decls/string.h>) @@ -26,8 +23,78 @@ #pragma omp begin declare target +// The GNU headers provide C++ standard compliant headers when in C++ mode and +// the LLVM libc does not. We need to manually provide the definitions using the +// same prototypes. +#if defined(__cplusplus) && defined(__GLIBC__) && \ + defined(__CORRECT_ISO_CPP_STRING_H_PROTO) + +#ifndef __LIBC_ATTRS +#define __LIBC_ATTRS +#endif + +extern "C" { +void *memccpy(void *__restrict, const void *__restrict, int, + size_t) __LIBC_ATTRS; +extern "C++" void *memchr(void *__s, int __c, size_t __n) noexcept __LIBC_ATTRS; +extern "C++" const void *memchr(const void *__s, int __c, + size_t __n) noexcept __LIBC_ATTRS; +int memcmp(const void *, const void *, size_t) __LIBC_ATTRS; +void *memcpy(void *__restrict, const void *__restrict, size_t) __LIBC_ATTRS; +void *memmem(const void *, size_t, const void *, size_t) __LIBC_ATTRS; +void *memmove(void *, const void *, size_t) __LIBC_ATTRS; +void *mempcpy(void *__restrict, const void *__restrict, size_t) __LIBC_ATTRS; +extern "C++" void *memrchr(void *__s, int __c, + size_t __n) noexcept __LIBC_ATTRS; +extern "C++" const void *memrchr(const void *__s, int __c, + size_t __n) noexcept __LIBC_ATTRS; +void *memset(void *, int, size_t) __LIBC_ATTRS; +char *stpcpy(char *__restrict, const char *__restrict) __LIBC_ATTRS; +char *stpncpy(char *__restrict, const char *__restrict, size_t) __LIBC_ATTRS; +extern "C++" char *strcasestr(char *__haystack, + const char *__needle) noexcept __LIBC_ATTRS; +extern "C++" const char *strcasestr(const char *__haystack, + const char *__needle) noexcept __LIBC_ATTRS; +char *strcat(char *__restrict, const char *__restrict) __LIBC_ATTRS; +extern "C++" char *strchr(char *__s, int __c) noexcept __LIBC_ATTRS; +extern "C++" const char *strchr(const char *__s, int __c) noexcept __LIBC_ATTRS; +extern "C++" char *strchrnul(char *__s, int __c) noexcept __LIBC_ATTRS; +extern "C++" const char *strchrnul(const char *__s, + int __c) noexcept __LIBC_ATTRS; +int strcmp(const char *, const char *) __LIBC_ATTRS; +int strcoll(const char *, const char *) __LIBC_ATTRS; +char *strcpy(char *__restrict, const char *__restrict) __LIBC_ATTRS; +size_t strcspn(const char *, const char *) __LIBC_ATTRS; +char *strdup(const char *) __LIBC_ATTRS; +size_t strlen(const char *) __LIBC_ATTRS; +char *strncat(char *, const char *, size_t) __LIBC_ATTRS; +int strncmp(const char *, const char *, size_t) __LIBC_ATTRS; +char *strncpy(char *__restrict, const char *__restrict, size_t) __LIBC_ATTRS; +char *strndup(const char *, size_t) __LIBC_ATTRS; +size_t strnlen(const char *, size_t) __LIBC_ATTRS; +extern "C++" char *strpbrk(char *__s, + const char *__accept) noexcept __LIBC_ATTRS; +extern "C++" const char *strpbrk(const char *__s, + const char *__accept) noexcept __LIBC_ATTRS; +extern "C++" char *strrchr(char *__s, int __c) noexcept __LIBC_ATTRS; +extern "C++" const char *strrchr(const char *__s, + int __c) noexcept __LIBC_ATTRS; +size_t strspn(const char *, const char *) __LIBC_ATTRS; +extern "C++" char *strstr(char *__haystack, + const char *__needle) noexcept __LIBC_ATTRS; +extern "C++" const char *strstr(const char *__haystack, + const char *__needle) noexcept __LIBC_ATTRS; +char *strtok(char *__restrict, const char *__restrict) __LIBC_ATTRS; +char *strtok_r(char *__restrict, const char *__restrict, + char **__restrict) __LIBC_ATTRS; +size_t strxfrm(char *__restrict, const char *__restrict, size_t) __LIBC_ATTRS; +} + +#else #include <llvm-libc-decls/string.h> +#endif + #pragma omp end declare target #undef __LIBC_ATTRS 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 
Copy link
Member

@jdoerfert jdoerfert left a comment

Choose a reason for hiding this comment

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

Seems a reasonable solution, given that nothing in this space is really nice and clean.

Nobody complained for a while, LG.

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. This patch more or less gives up an just copies the definitions as expected by `glibc` if they are provided that way, otherwise we fall back to the accepted way. This is the alternative solution to an existing PR which instead disable's GCC's handling.
@jhuber6 jhuber6 merged commit 25bf1ae into llvm:main Oct 23, 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

3 participants