Skip to content

Conversation

@michaelrj-google
Copy link
Contributor

@michaelrj-google michaelrj-google commented Jun 5, 2025

The string_utils.h file previously included both memcpy and bzero. There
were no uses of bzero, and only one use of memcpy which was replaced
with __builtin_memcpy.

Also fix strsep which was broken by this change, fix a useless assert of
"sizeof(char) == sizeof(cpp::byte)", and update the bazel.

The string_utils.h file previously included both memcpy and bzero. There
were no uses of bzero, and only one use of memcpy which was replaced
with __builtin_memcpy.
@llvmbot llvmbot added libc bazel "Peripheral" support tier build system: utils/bazel labels Jun 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2025

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

The string_utils.h file previously included both memcpy and bzero. There
were no uses of bzero, and only one use of memcpy which was replaced
with __builtin_memcpy.


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

3 Files Affected:

  • (modified) libc/src/string/CMakeLists.txt (-3)
  • (modified) libc/src/string/string_utils.h (+1-3)
  • (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (-1)
diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index 2c607bf8ea895..deb74d979a3f7 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -15,10 +15,7 @@ add_header_library(
   HDRS
     string_utils.h
   DEPENDS
-    .memory_utils.inline_bzero
-    .memory_utils.inline_memcpy
     libc.hdr.types.size_t
-    libc.include.stdlib
     libc.src.__support.CPP.bitset
     libc.src.__support.CPP.type_traits
     libc.src.__support.common
diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
index e4659f65c93e2..e666636632184 100644
--- a/libc/src/string/string_utils.h
+++ b/libc/src/string/string_utils.h
@@ -19,8 +19,6 @@
 #include "src/__support/CPP/type_traits.h" // cpp::is_same_v
 #include "src/__support/macros/config.h"
 #include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
-#include "src/string/memory_utils/inline_bzero.h"
-#include "src/string/memory_utils/inline_memcpy.h"
 
 namespace LIBC_NAMESPACE_DECL {
 namespace internal {
@@ -220,7 +218,7 @@ LIBC_INLINE size_t strlcpy(char *__restrict dst, const char *__restrict src,
   if (!size)
     return len;
   size_t n = len < size - 1 ? len : size - 1;
-  inline_memcpy(dst, src, n);
+  __builtin_memcpy(dst, src, n);
   dst[n] = '\0';
   return len;
 }
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index aa7f0a9389405..65e6a947eaed6 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -4190,7 +4190,6 @@ libc_support_library(
         ":__support_cpp_bitset",
         ":__support_macros_optimization",
         ":llvm_libc_types_size_t",
-        ":string_memory_utils",
         ":types_size_t",
     ],
 )

@michaelrj-google
Copy link
Contributor Author

thanks for the review, going to merge once the presubmits pass

@michaelrj-google michaelrj-google merged commit 59f88a8 into llvm:main Jun 6, 2025
12 of 13 checks passed
@michaelrj-google michaelrj-google deleted the libcStringUtilsCleanup branch June 6, 2025 18:17
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jun 6, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime-2 running on rocm-worker-hw-02 while building libc,utils at step 5 "compile-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/6844

Here is the relevant piece of the build log for the reference
Step 5 (compile-openmp) failure: build (failure)
...
0.320 [273/99/315] Generating header math.h from /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/runtimes/../libc/include/math.yaml
0.326 [273/98/316] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.bsearch.dir/bsearch.cpp.o
0.328 [273/97/317] Generating /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa/llvm-libc-decls/math.h
0.341 [239/130/318] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.atoi.dir/atoi.cpp.o
0.341 [238/130/319] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.atol.dir/atol.cpp.o
0.342 [237/130/320] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.atoll.dir/atoll.cpp.o
0.342 [236/130/321] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.strtol.dir/strtol.cpp.o
0.343 [235/130/322] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.strtoull.dir/strtoull.cpp.o
0.343 [234/130/323] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.strtoll_l.dir/strtoll_l.cpp.o
0.344 [233/130/324] Building CXX object libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.ferror.dir/ferror.cpp.o
FAILED: libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.ferror.dir/ferror.cpp.o 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/./bin/clang++ --target=amdgcn-amd-amdhsa -DLIBC_NAMESPACE=__llvm_libc_21_0_0_git -D__LIBC_USE_FLOAT16_CONVERSION -I/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc -isystem /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa -O3 -DNDEBUG --target=amdgcn-amd-amdhsa -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS "-DLIBC_MATH=(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES | LIBC_MATH_NO_ERRNO | LIBC_MATH_NO_EXCEPT)" -fpie -DLIBC_FULL_BUILD -nostdlibinc -ffixed-point -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -nogpulib -fvisibility=hidden -fconvergent-functions -flto -Wno-multi-gpu -Xclang -mcode-object-version=none -DLIBC_COPT_PUBLIC_PACKAGING -UNDEBUG -MD -MT libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.ferror.dir/ferror.cpp.o -MF libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.ferror.dir/ferror.cpp.o.d -o libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.ferror.dir/ferror.cpp.o -c /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/ferror.cpp
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/ferror.cpp:17:25: error: unknown type name 'ferror'
   17 | LLVM_LIBC_FUNCTION(int, ferror, (::FILE * stream)) {
      |                         ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/ferror.cpp:17:34: error: a type specifier is required for all declarations
   17 | LLVM_LIBC_FUNCTION(int, ferror, (::FILE * stream)) {
      |                                  ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/ferror.cpp:17:1: error: a type specifier is required for all declarations
   17 | LLVM_LIBC_FUNCTION(int, ferror, (::FILE * stream)) {
      | ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/ferror.cpp:22:45: error: use of undeclared identifier 'stream'
   22 |         buffer->data[0] = file::from_stream(stream);
      |                                             ^~~~~~
4 errors generated.
0.344 [233/129/325] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.div.dir/div.cpp.o
0.344 [233/128/326] Building CXX object libc/src/stdlib/CMakeFiles/libc.src.stdlib.ldiv.dir/ldiv.cpp.o
0.344 [233/127/327] Building CXX object libc/src/stdlib/gpu/CMakeFiles/libc.src.stdlib.gpu.aligned_alloc.dir/aligned_alloc.cpp.o
0.344 [233/126/328] Building CXX object libc/src/locale/CMakeFiles/libc.src.locale.locale.dir/locale.cpp.o
0.345 [233/125/329] Building CXX object libc/src/locale/CMakeFiles/libc.src.locale.duplocale.dir/duplocale.cpp.o
0.345 [233/124/330] Building CXX object libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.fseek.dir/fseek.cpp.o
FAILED: libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.fseek.dir/fseek.cpp.o 
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/./bin/clang++ --target=amdgcn-amd-amdhsa -DLIBC_NAMESPACE=__llvm_libc_21_0_0_git -D__LIBC_USE_FLOAT16_CONVERSION -I/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc -isystem /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa -O3 -DNDEBUG --target=amdgcn-amd-amdhsa -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS "-DLIBC_MATH=(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES | LIBC_MATH_NO_ERRNO | LIBC_MATH_NO_EXCEPT)" -fpie -DLIBC_FULL_BUILD -nostdlibinc -ffixed-point -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -nogpulib -fvisibility=hidden -fconvergent-functions -flto -Wno-multi-gpu -Xclang -mcode-object-version=none -DLIBC_COPT_PUBLIC_PACKAGING -UNDEBUG -MD -MT libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.fseek.dir/fseek.cpp.o -MF libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.fseek.dir/fseek.cpp.o.d -o libc/src/stdio/gpu/CMakeFiles/libc.src.stdio.gpu.fseek.dir/fseek.cpp.o -c /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/fseek.cpp
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/fseek.cpp:17:25: error: unknown type name 'fseek'
   17 | LLVM_LIBC_FUNCTION(int, fseek, (::FILE * stream, long offset, int whence)) {
      |                         ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/fseek.cpp:17:33: error: a type specifier is required for all declarations
   17 | LLVM_LIBC_FUNCTION(int, fseek, (::FILE * stream, long offset, int whence)) {
      |                                 ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/fseek.cpp:17:1: error: a type specifier is required for all declarations
   17 | LLVM_LIBC_FUNCTION(int, fseek, (::FILE * stream, long offset, int whence)) {
      | ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/fseek.cpp:22:45: error: use of undeclared identifier 'stream'
   22 |         buffer->data[0] = file::from_stream(stream);
      |                                             ^~~~~~
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/fseek.cpp:23:49: error: use of undeclared identifier 'offset'
   23 |         buffer->data[1] = static_cast<uint64_t>(offset);
      |                                                 ^~~~~~
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/stdio/gpu/fseek.cpp:24:49: error: use of undeclared identifier 'whence'

@Kewen12
Copy link
Contributor

Kewen12 commented Jun 6, 2025

Hi there, this PR had build failure that breaks our buildbots. Could you please take a look. Thanks!

@michaelrj-google
Copy link
Contributor Author

@Kewen12 are you talking about the GPU build? If so it should be fixed by 5823e92

@Kewen12
Copy link
Contributor

Kewen12 commented Jun 6, 2025

Hi @michaelrj-google, it seems the fix build successfully but failed a check. Appreciated if you could take another look.

/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/test/integration/startup/gpu/rpc_stream_test.cpp:84:3: error: use of undeclared identifier 'inline_memset'
84 | inline_memset(buffer, 0, offset);
| ^~~~~~~~~~~~~
1 error generated.

buildbot: https://lab.llvm.org/buildbot/#/builders/10/builds/6850

Kewen12 added a commit to Kewen12/llvm-project that referenced this pull request Jun 6, 2025
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
The string_utils.h file previously included both memcpy and bzero. There
were no uses of bzero, and only one use of memcpy which was replaced
with __builtin_memcpy.

Also fix strsep which was broken by this change, fix a useless assert of
"sizeof(char) == sizeof(cpp::byte)", and update the bazel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel libc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants