Skip to content

[libc] Correct 'memrchr' definition and re-enable on GPU #67850

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
Sep 29, 2023

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Sep 29, 2023

Summary:
This was disabled on the GPU because it conflicted with the definition
in glibc. According to information online and in the glibc
implementation, the first argument should be a const void *. Fixing
this resolves the problem when exporting this to offloading languages.

@llvmbot llvmbot added the libc label Sep 29, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2023

@llvm/pr-subscribers-libc

Changes

Summary:
This was disabled on the GPU because it conflicted with the definition
in glibc. According to information online and in the glibc
implementation, the first argument should be a const void *. Fixing
this resolves the problem when exporting this to offloading languages.


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

2 Files Affected:

  • (modified) libc/config/gpu/entrypoints.txt (+1)
  • (modified) libc/spec/gnu_ext.td (+1-1)
diff --git a/libc/config/gpu/entrypoints.txt b/libc/config/gpu/entrypoints.txt
index ad68216a76b9429..839213f82a845eb 100644
--- a/libc/config/gpu/entrypoints.txt
+++ b/libc/config/gpu/entrypoints.txt
@@ -45,6 +45,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.string.strncmp
     libc.src.string.strncpy
     libc.src.string.strnlen
+    libc.src.string.memrchr
     libc.src.string.strspn
     libc.src.string.strtok
     libc.src.string.strtok_r
diff --git a/libc/spec/gnu_ext.td b/libc/spec/gnu_ext.td
index 362add7283d6e11..dfb12419d14005b 100644
--- a/libc/spec/gnu_ext.td
+++ b/libc/spec/gnu_ext.td
@@ -69,7 +69,7 @@ def GnuExtensions : StandardSpec<"GNUExtensions"> {
         FunctionSpec<
             "memrchr",
             RetValSpec<VoidPtr>,
-            [ArgSpec<VoidPtr>, ArgSpec<IntType>, ArgSpec<SizeTType>]
+            [ArgSpec<ConstVoidPtr>, ArgSpec<IntType>, ArgSpec<SizeTType>]
         >,
         FunctionSpec<
             "strerror_r",

@jhuber6 jhuber6 requested review from a team, gchatelet, lntue and michaelrj-google September 29, 2023 20:37
Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

LGTM

Summary:
This was disabled on the GPU because it conflicted with the definition
in `glibc`. According to information online and in the `glibc`
implementation, the first argument should be a `const void *`. Fixing
this resolves the problem when exporting this to offloading languages.
@jhuber6 jhuber6 merged commit f88f090 into llvm:main Sep 29, 2023
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Oct 12, 2023
Local branch amd-gfx 4e95612 Merged main:d89d3a6a0eb3 into amd-gfx:6b554b98f849
Remote branch main f88f090 [libc] Correct memrchr definition and re-enable on GPU (llvm#67850)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants