[libc] Add memset and bzero implementations

[gchatelet]

Summary

This patch adds general purpose memset and bzero implementations.

[gchatelet]

libc/src/string/memory_utils/memset_utils.h

21

Theoretically the compiler is allowed to call memsethere and end up with a recursive call, practically it doesn't happen, however this should be replaced with a __builtin_memset_inline once it's available in clang.

[gchatelet]

• Remove dead code.

[gchatelet]

rebase

[abrachet]

libc/src/string/memory_utils/memset_utils.h

10

Should this have STRING in between SRC and MEMORY? I think clang-tidy might complain.

21

Of course I can't find it anymore, but we had for memcpy a CMake rule to see if the file contained any references to memcpy I think. Is that being done here?

28

Strange const

96

Why choose 32 and not 64? If 32 is the best choice here then why does the count <= 128 condition exist?

libc/src/string/memset.cpp

17–18

I think the cast is explanatory enough without the comment.

libc/test/src/string/bzero_test.cpp

1

//===-- Unittests for bzero --...

39

There was recently a patch which changed all uses of functional style casts. I don't know if its in the style guide to not use them or not. But maybe it's better to use (char) 0 or '\0'?

libc/test/src/string/memcpy_test.cpp

38

Maybe reinterpret_cast to const void *const. Obviously it isn't harmful here because ASSERT_EQ takes a const T but there is a sneaky const_cast in this C-style cast

[gchatelet]

libc/src/string/memory_utils/memset_utils.h

21

Yes I removed them because it was brittle, also we now have tests that cover all implementations so the stack overflow will be caught.

96

This may look surprising indeed. We want to balance two things here:

• The number of redundant writes,
• The number of conditionals for sizes <=128 : 91% of memset calls are for such sizes.

For the range 64-128:

• SetBlockOverlap<64> uses no conditionals but always writes 128 Bytes this is wasteful for 65 but efficient for 128.
• SetAlignedBlocks<32> would consume between 3 and 4 conditionals and write 96 or 128 Bytes.
• Another approach could be to use an hybrid approach Copy<64>+Overlap<32> for 65-96 and Copy<96>+Overlap<32> for 97-128

Benchmarks showed that redundant writes were cheap (for Intel X86) but conditional were expensive, even on processor that do not support writing 64B at a time (pre-AVX512F). We also want to favor short functions that allow more hot code to fit in the iL1 cache.

Above 128 we have to use conditionals since we don't know the upper bound in advance. SetAlignedBlocks<64> may waste up to 63 Bytes, SetAlignedBlocks<32> may waste up to 31 Bytes. Benchmarks showed that SetAlignedBlocks<64> was not superior for sizes that mattered.

To be honest this part of the implementation is subject to change as we benchmark more processors. We may also want to specialize it for processors with specialized instructions that performs better (rep stosb).

libc/test/src/string/bzero_test.cpp

39

This is not a functional cast but a constructor of type char with value 0.

[gchatelet]

• Adress comments

[sivachandra]

LGTM overall.

May be a tad incomplete in the sense that bzero will not show up in the public string.h. But, I don't think this patch should block on that. bzero is essentially an LLVM libc extension at this point. It probably makes sense to add a new standard spec called llvm-libc-ext and add all llvm-libc specific functions there and then expose them via public headers.

libc/src/string/memory_utils/memset_utils.h

21

Can you add your note as a comment here?

96

Can you add your explanation as a comment here?

[gchatelet]

• Address comments

[gchatelet]

I'll submit this version and we can iterate from here. Thx for the review !

[gchatelet]

• rebase

libc/lib/CMakeLists.txt

DEPENDS
     libc.src.signal.sigfillset
     libc.src.signal.signal
 
     # stdlib.h entrypoints
     libc.src.stdlib._Exit
     libc.src.stdlib.abort
 
     # string.h entrypoints
+    libc.src.string.bzero
     libc.src.string.memcpy
-    libc.src.string.strcpy
+    libc.src.string.memset
     libc.src.string.strcat
+    libc.src.string.strcpy
     libc.src.string.strlen
 
     # sys/mman.h entrypoints
     libc.src.sys.mman.mmap
     libc.src.sys.mman.munmap
 
     # threads.h entrypoints
     libc.src.threads.mtx_init

libc/src/string/CMakeLists.txt

add_entrypoint_object(
   SRCS
     strlen.cpp
   HDRS
     strlen.h
   DEPENDS
     libc.include.string
 )
 
+# Helper to define a function with multiple implementations
+# - Computes flags to satisfy required/rejected features and arch,
+# - Declares an entry point,
+# - Attach the REQUIRE_CPU_FEATURES property to the target,
+# - Add the fully qualified target to `${name}_implementations` global property for tests.
+function(add_implementation name impl_name)
+  cmake_parse_arguments(
+    "ADD_IMPL"
+    "" # Optional arguments
+    "MARCH" # Single value arguments
+    "REQUIRE;REJECT;SRCS;HDRS;DEPENDS;COMPILE_OPTIONS" # Multi value arguments
+    ${ARGN})
+  compute_flags(flags
+    MARCH ${ADD_IMPL_MARCH}
+    REQUIRE ${ADD_IMPL_REQUIRE}
+    REJECT ${ADD_IMPL_REJECT}
+  )
+  add_entrypoint_object(${impl_name}
+    SRCS ${ADD_IMPL_SRCS}
+    HDRS ${ADD_IMPL_HDRS}
+    DEPENDS ${ADD_IMPL_DEPENDS}
+    COMPILE_OPTIONS ${ADD_IMPL_COMPILE_OPTIONS} ${flags}
+  )
+  get_fq_target_name(${impl_name} fq_target_name)
+  set_target_properties(${fq_target_name} PROPERTIES REQUIRE_CPU_FEATURES "${ADD_IMPL_REQUIRE}")
+  set_property(GLOBAL APPEND PROPERTY "${name}_implementations" "${fq_target_name}")
+endfunction()
+
 # ------------------------------------------------------------------------------
 # memcpy
 # ------------------------------------------------------------------------------
 
 # include the relevant architecture specific implementations
 if(${LIBC_TARGET_MACHINE} STREQUAL "x86_64")
-  set(LIBC_MEMCPY_IMPL_FOLDER "x86")
+  set(LIBC_STRING_TARGET_FOLDER "x86")
 else()
-  set(LIBC_MEMCPY_IMPL_FOLDER ${LIBC_TARGET_MACHINE})
+  set(LIBC_STRING_TARGET_FOLDER ${LIBC_TARGET_MACHINE})
 endif()
 
 add_gen_header(
   memcpy_arch_specific
   DEF_FILE
     memcpy_arch_specific.h.def
   GEN_HDR
     memcpy_arch_specific.h
   PARAMS
-    memcpy_arch_specific=${LIBC_MEMCPY_IMPL_FOLDER}/memcpy_arch_specific.h.inc
+    memcpy_arch_specific=${LIBC_STRING_TARGET_FOLDER}/memcpy_arch_specific.h.inc
   DATA_FILES
-    ${LIBC_MEMCPY_IMPL_FOLDER}/memcpy_arch_specific.h.inc
+    ${LIBC_STRING_TARGET_FOLDER}/memcpy_arch_specific.h.inc
 )
 
-# Helper to define an implementation of memcpy.
-# - Computes flags to satisfy required/rejected features and arch,
-# - Declares an entry point,
-# - Attach the REQUIRE_CPU_FEATURES property to the target,
-# - Add the target to `memcpy_implementations` global property for tests.
 function(add_memcpy memcpy_name)
-  cmake_parse_arguments(
-    "ADD_MEMCPY"
-    "" # Optional arguments
-    "MARCH" # Single value arguments
-    "REQUIRE;REJECT" # Multi value arguments
-    ${ARGN})
-  compute_flags(flags
-    MARCH ${ADD_MEMCPY_MARCH}
-    REQUIRE ${ADD_MEMCPY_REQUIRE}
-    REJECT ${ADD_MEMCPY_REJECT}
-  )
-  add_entrypoint_object(
-    ${memcpy_name}
+  add_implementation(memcpy ${memcpy_name}
     SRCS ${LIBC_SOURCE_DIR}/src/string/memcpy.cpp
     HDRS ${LIBC_SOURCE_DIR}/src/string/memcpy.h
     DEPENDS
       .memory_utils.memory_utils
       .memcpy_arch_specific
       libc.include.string
     COMPILE_OPTIONS
       -fno-builtin-memcpy
-      ${flags}
+    ${ARGN}
   )
-  get_fq_target_name(${memcpy_name} fq_target_name)
-  set_target_properties(${fq_target_name} PROPERTIES REQUIRE_CPU_FEATURES "${ADD_MEMCPY_REQUIRE}")
-  get_property(all GLOBAL PROPERTY memcpy_implementations)
-  list(APPEND all ${memcpy_name})
-  set_property(GLOBAL PROPERTY memcpy_implementations "${all}")
 endfunction()
 
-include(${LIBC_MEMCPY_IMPL_FOLDER}/CMakeLists.txt)
 add_memcpy(memcpy MARCH native)
+
+# ------------------------------------------------------------------------------
+# memset
+# ------------------------------------------------------------------------------
+
+function(add_memset memset_name)
+  add_implementation(memset ${memset_name}
+    SRCS ${LIBC_SOURCE_DIR}/src/string/memset.cpp
+    HDRS ${LIBC_SOURCE_DIR}/src/string/memset.h
+    DEPENDS
+      .memory_utils.memory_utils
+      libc.include.string
+    COMPILE_OPTIONS
+      -fno-builtin-memset
+    ${ARGN}
+  )
+endfunction()
+
+add_memset(memset MARCH native)
+
+# ------------------------------------------------------------------------------
+# bzero
+# ------------------------------------------------------------------------------
+
+function(add_bzero bzero_name)
+  add_implementation(bzero ${bzero_name}
+    SRCS ${LIBC_SOURCE_DIR}/src/string/bzero.cpp
+    HDRS ${LIBC_SOURCE_DIR}/src/string/bzero.h
+    DEPENDS
+      .memory_utils.memory_utils
+      libc.include.string
+    COMPILE_OPTIONS
+      -fno-builtin-memset
+      -fno-builtin-bzero
+    ${ARGN}
+  )
+endfunction()
+
+add_bzero(bzero MARCH native)
+
+# ------------------------------------------------------------------------------
+# Add all other relevant implementations for the native target.
+# ------------------------------------------------------------------------------
+
+include(${LIBC_STRING_TARGET_FOLDER}/CMakeLists.txt)

libc/src/string/bzero.h

+//===-- Implementation header for bzero -------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STRING_BZERO_H
+#define LLVM_LIBC_SRC_STRING_BZERO_H
+
+#include "include/string.h"
+
+namespace __llvm_libc {
+
+void bzero(void *ptr, size_t count);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_STRING_BZERO_H

libc/src/string/bzero.cpp

+//===-- Implementation of bzero -------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/string/bzero.h"
+#include "src/__support/common.h"
+#include "src/string/memory_utils/memset_utils.h"
+
+namespace __llvm_libc {
+
+void LLVM_LIBC_ENTRYPOINT(bzero)(void *ptr, size_t count) {
+  GeneralPurposeMemset(reinterpret_cast<char *>(ptr), 0, count);
+}
+
+} // namespace __llvm_libc

libc/src/string/memory_utils/CMakeLists.txt

DATA_FILES
     cacheline_size_${LIBC_TARGET_MACHINE}.h.inc
 )
 
 add_header_library(
   memory_utils
   HDRS
     utils.h
     memcpy_utils.h
+    memset_utils.h
   DEPENDS
     .cacheline_size
 )

libc/src/string/memory_utils/memset_utils.h

+//===-- Memset utils --------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MEMORY_UTILS_MEMSET_UTILS_H
+#define LLVM_LIBC_SRC_MEMORY_UTILS_MEMSET_UTILS_H
+
+#include "src/string/memory_utils/utils.h"
+
+#include <stddef.h> // size_t
+
+namespace __llvm_libc {
+
+// Sets `kBlockSize` bytes starting from `src` to `value`.
+template <size_t kBlockSize> static void SetBlock(char *dst, unsigned value) {
+  __builtin_memset(dst, value, kBlockSize);
+}
+
+// Sets `kBlockSize` bytes from `src + count - kBlockSize` to `value`.
+// Precondition: `count >= kBlockSize`.
+template <size_t kBlockSize>
+static void SetLastBlock(char *dst, unsigned value, size_t count) {
+  const size_t offset = count - kBlockSize;
+  SetBlock<kBlockSize>(dst + offset, value);
+}
+
+// Sets `kBlockSize` bytes twice with an overlap between the two.
+//
+// [1234567812345678123]
+// [__XXXXXXXXXXXXXX___]
+// [__XXXXXXXX_________]
+// [________XXXXXXXX___]
+//
+// Precondition: `count >= kBlockSize && count <= kBlockSize`.
+template <size_t kBlockSize>
+static void SetBlockOverlap(char *dst, unsigned value, size_t count) {
+  SetBlock<kBlockSize>(dst, value);
+  SetLastBlock<kBlockSize>(dst, value, count);
+}
+
+// Sets `count` bytes by blocks of `kBlockSize` bytes.
+// Sets at the start and end of the buffer are unaligned.
+// Sets in the middle of the buffer are aligned to `kBlockSize`.
+//
+// e.g. with
+// [12345678123456781234567812345678]
+// [__XXXXXXXXXXXXXXXXXXXXXXXXXXX___]
+// [__XXXXXXXX______________________]
+// [________XXXXXXXX________________]
+// [________________XXXXXXXX________]
+// [_____________________XXXXXXXX___]
+//
+// Precondition: `count > 2 * kBlockSize` for efficiency.
+//               `count >= kBlockSize` for correctness.
+template <size_t kBlockSize>
+static void SetAlignedBlocks(char *dst, unsigned value, size_t count) {
+  SetBlock<kBlockSize>(dst, value); // Set first block
+
+  // Set aligned blocks
+  size_t offset = kBlockSize - offset_from_last_aligned<kBlockSize>(dst);
+  for (; offset + kBlockSize < count; offset += kBlockSize)
+    SetBlock<kBlockSize>(dst + offset, value);
+
+  SetLastBlock<kBlockSize>(dst, value, count); // Set last block
+}
+
+// A general purpose implementation assuming cheap unaligned writes for sizes:
+// 1, 2, 4, 8, 16, 32 and 64 Bytes.
+inline static void GeneralPurposeMemset(char *dst, unsigned char value,
+                                        size_t count) {
+  if (count == 0)
+    return;
+  if (count == 1)
+    return SetBlock<1>(dst, value);
+  if (count == 2)
+    return SetBlock<2>(dst, value);
+  if (count == 3)
+    return SetBlock<3>(dst, value);
+  if (count == 4)
+    return SetBlock<4>(dst, value);
+  if (count <= 8)
+    return SetBlockOverlap<4>(dst, value, count);
+  if (count <= 16)
+    return SetBlockOverlap<8>(dst, value, count);
+  if (count <= 32)
+    return SetBlockOverlap<16>(dst, value, count);
+  if (count <= 64)
+    return SetBlockOverlap<32>(dst, value, count);
+  if (count <= 128)
+    return SetBlockOverlap<64>(dst, value, count);
+  return SetAlignedBlocks<32>(dst, value, count);
+}
+
+} // namespace __llvm_libc
+
+#endif //  LLVM_LIBC_SRC_MEMORY_UTILS_MEMSET_UTILS_H

libc/src/string/memset.h

+//===-- Implementation header for memset ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STRING_MEMSET_H
+#define LLVM_LIBC_SRC_STRING_MEMSET_H
+
+#include "include/string.h"
+
+namespace __llvm_libc {
+
+void *memset(void *ptr, int value, size_t count);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_STRING_MEMSET_H

libc/src/string/memset.cpp

+//===-- Implementation of memset ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/string/memset.h"
+#include "src/__support/common.h"
+#include "src/string/memory_utils/memset_utils.h"
+
+namespace __llvm_libc {
+
+void *LLVM_LIBC_ENTRYPOINT(memset)(void *dst, int value, size_t count) {
+  // The value is passed as an int, but the function fills the block of memory
+  // using the `unsigned char` conversion of this value.
+  GeneralPurposeMemset(reinterpret_cast<char *>(dst),
+                       static_cast<unsigned char>(value), count);
+  return dst;
+}
+
+} // namespace __llvm_libc

libc/src/string/x86/CMakeLists.txt

 add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_none" REJECT "${ALL_CPU_FEATURES}")
 add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_sse" REQUIRE "SSE" REJECT "SSE2")
 add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_avx" REQUIRE "AVX" REJECT "AVX2")
 add_memcpy("memcpy_${LIBC_TARGET_MACHINE}_opt_avx512f" REQUIRE "AVX512F")
+
+add_memset("memset_${LIBC_TARGET_MACHINE}_opt_none" REJECT "${ALL_CPU_FEATURES}")
+add_memset("memset_${LIBC_TARGET_MACHINE}_opt_sse" REQUIRE "SSE" REJECT "SSE2")
+add_memset("memset_${LIBC_TARGET_MACHINE}_opt_avx" REQUIRE "AVX" REJECT "AVX2")
+add_memset("memset_${LIBC_TARGET_MACHINE}_opt_avx512f" REQUIRE "AVX512F")
+
+add_bzero("bzero_${LIBC_TARGET_MACHINE}_opt_none" REJECT "${ALL_CPU_FEATURES}")
+add_bzero("bzero_${LIBC_TARGET_MACHINE}_opt_sse" REQUIRE "SSE" REJECT "SSE2")
+add_bzero("bzero_${LIBC_TARGET_MACHINE}_opt_avx" REQUIRE "AVX" REJECT "AVX2")
+add_bzero("bzero_${LIBC_TARGET_MACHINE}_opt_avx512f" REQUIRE "AVX512F")

libc/test/src/string/CMakeLists.txt

add_libc_unittest(
   SUITE
     libc_string_unittests
   SRCS
     strlen_test.cpp
   DEPENDS
     libc.src.string.strlen
 )
 
-# Tests all implementations of memcpy that can run on the host.
-get_property(memcpy_implementations GLOBAL PROPERTY memcpy_implementations)
-foreach(memcpy_config_name IN LISTS memcpy_implementations)
-  get_target_property(require_cpu_features libc.src.string.${memcpy_config_name} REQUIRE_CPU_FEATURES)
-  host_supports(can_run "${require_cpu_features}")
+# Tests all implementations that can run on the host.
+function(add_libc_multi_impl_test name)
+  get_property(fq_implementations GLOBAL PROPERTY ${name}_implementations)
+  foreach(fq_config_name IN LISTS fq_implementations)
+    get_target_property(required_cpu_features ${fq_config_name} REQUIRE_CPU_FEATURES)
+    host_supports(can_run "${required_cpu_features}")
     if(can_run)
       add_libc_unittest(
-      ${memcpy_config_name}_test
+        ${fq_config_name}_test
         SUITE
           libc_string_unittests
-      SRCS
-        memcpy_test.cpp
         DEPENDS
-        libc.src.string.${memcpy_config_name}
+          ${fq_config_name}
+        ${ARGN}
       )
     else()
-    message(STATUS "Skipping test for '${memcpy_config_name}' insufficient host cpu features")
+      message(STATUS "Skipping test for '${fq_config_name}' insufficient host cpu features '${required_cpu_features}'")
     endif()
   endforeach()
+endfunction()
+
+add_libc_multi_impl_test(memcpy SRCS memcpy_test.cpp)
+add_libc_multi_impl_test(memset SRCS memset_test.cpp)
+add_libc_multi_impl_test(bzero SRCS bzero_test.cpp)
 

libc/test/src/string/bzero_test.cpp

-//===----------------------- Unittests for memcpy -------------------------===//
+//===----------------------- Unittests for bzero --------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/string/bzero.h"
 #include "utils/CPP/ArrayRef.h"
 #include "utils/UnitTest/Test.h"
-#include "src/string/memcpy.h"
 
 using __llvm_libc::cpp::Array;
 using __llvm_libc::cpp::ArrayRef;
-using __llvm_libc::cpp::MutableArrayRef;
 using Data = Array<char, 2048>;
 
-static const ArrayRef<char> kNumbers("0123456789", 10);
 static const ArrayRef<char> kDeadcode("DEADC0DE", 8);
 
 // Returns a Data object filled with a repetition of `filler`.
 Data getData(ArrayRef<char> filler) {
   Data out;
   for (size_t i = 0; i < out.size(); ++i)
     out[i] = filler[i % filler.size()];
   return out;
 }
 
-TEST(MemcpyTest, Thorough) {
-  const Data groundtruth = getData(kNumbers);
+TEST(BzeroTest, Thorough) {
   const Data dirty = getData(kDeadcode);
   for (size_t count = 0; count < 1024; ++count) {
     for (size_t align = 0; align < 64; ++align) {
       auto buffer = dirty;
-      const char *const src = groundtruth.data();
       char *const dst = &buffer[align];
-      __llvm_libc::memcpy(dst, src, count);
+      __llvm_libc::bzero(dst, count);
       // Everything before copy is untouched.
       for (size_t i = 0; i < align; ++i)
         ASSERT_EQ(buffer[i], dirty[i]);
       // Everything in between is copied.
       for (size_t i = 0; i < count; ++i)
-        ASSERT_EQ(buffer[align + i], groundtruth[i]);
+        ASSERT_EQ(buffer[align + i], char(0));
       // Everything after copy is untouched.
       for (size_t i = align + count; i < dirty.size(); ++i)
         ASSERT_EQ(buffer[i], dirty[i]);
     }
   }
 }
 
 // FIXME: Add tests with reads and writes on the boundary of a read/write
 // protected page to check we're not reading nor writing prior/past the allowed
 // regions.

libc/test/src/string/memcpy_test.cpp

 //===----------------------- Unittests for memcpy -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/string/memcpy.h"
 #include "utils/CPP/ArrayRef.h"
 #include "utils/UnitTest/Test.h"
-#include "src/string/memcpy.h"
 
 using __llvm_libc::cpp::Array;
 using __llvm_libc::cpp::ArrayRef;
-using __llvm_libc::cpp::MutableArrayRef;
 using Data = Array<char, 2048>;
 
 static const ArrayRef<char> kNumbers("0123456789", 10);
 static const ArrayRef<char> kDeadcode("DEADC0DE", 8);
 
 // Returns a Data object filled with a repetition of `filler`.
 Data getData(ArrayRef<char> filler) {
   Data out;
   for (size_t i = 0; i < out.size(); ++i)
     out[i] = filler[i % filler.size()];
   return out;
 }
 
 TEST(MemcpyTest, Thorough) {
   const Data groundtruth = getData(kNumbers);
   const Data dirty = getData(kDeadcode);
   for (size_t count = 0; count < 1024; ++count) {
     for (size_t align = 0; align < 64; ++align) {
       auto buffer = dirty;
       const char *const src = groundtruth.data();
       char *const dst = &buffer[align];
-      __llvm_libc::memcpy(dst, src, count);
+      const void *const ret = __llvm_libc::memcpy(dst, src, count);
+      // Return value is `dst`.
+      ASSERT_EQ(ret, (const void *)dst);
       // Everything before copy is untouched.
       for (size_t i = 0; i < align; ++i)
         ASSERT_EQ(buffer[i], dirty[i]);
       // Everything in between is copied.
       for (size_t i = 0; i < count; ++i)
         ASSERT_EQ(buffer[align + i], groundtruth[i]);
       // Everything after copy is untouched.
       for (size_t i = align + count; i < dirty.size(); ++i)
         ASSERT_EQ(buffer[i], dirty[i]);
     }
   }
 }
 
 // FIXME: Add tests with reads and writes on the boundary of a read/write
 // protected page to check we're not reading nor writing prior/past the allowed
 // regions.

libc/test/src/string/memset_test.cpp

-//===----------------------- Unittests for memcpy -------------------------===//
+//===----------------------- Unittests for memset -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/string/memset.h"
 #include "utils/CPP/ArrayRef.h"
 #include "utils/UnitTest/Test.h"
-#include "src/string/memcpy.h"
 
 using __llvm_libc::cpp::Array;
 using __llvm_libc::cpp::ArrayRef;
-using __llvm_libc::cpp::MutableArrayRef;
 using Data = Array<char, 2048>;
 
-static const ArrayRef<char> kNumbers("0123456789", 10);
 static const ArrayRef<char> kDeadcode("DEADC0DE", 8);
 
 // Returns a Data object filled with a repetition of `filler`.
 Data getData(ArrayRef<char> filler) {
   Data out;
   for (size_t i = 0; i < out.size(); ++i)
     out[i] = filler[i % filler.size()];
   return out;
 }
 
-TEST(MemcpyTest, Thorough) {
-  const Data groundtruth = getData(kNumbers);
+TEST(MemsetTest, Thorough) {
   const Data dirty = getData(kDeadcode);
+  for (int value = -1; value <= 1; ++value) {
     for (size_t count = 0; count < 1024; ++count) {
       for (size_t align = 0; align < 64; ++align) {
         auto buffer = dirty;
-      const char *const src = groundtruth.data();
         char *const dst = &buffer[align];
-      __llvm_libc::memcpy(dst, src, count);
+        const void *const ret = __llvm_libc::memset(dst, value, count);
+        // Return value is `dst`.
+        ASSERT_EQ(ret, (const void *)dst);
         // Everything before copy is untouched.
         for (size_t i = 0; i < align; ++i)
           ASSERT_EQ(buffer[i], dirty[i]);
         // Everything in between is copied.
         for (size_t i = 0; i < count; ++i)
-        ASSERT_EQ(buffer[align + i], groundtruth[i]);
+          ASSERT_EQ(buffer[align + i], (char)value);
         // Everything after copy is untouched.
         for (size_t i = align + count; i < dirty.size(); ++i)
           ASSERT_EQ(buffer[i], dirty[i]);
       }
     }
   }
+}
 
 // FIXME: Add tests with reads and writes on the boundary of a read/write
 // protected page to check we're not reading nor writing prior/past the allowed
 // regions.