[libc-commits] [libc] 458734b - [reland][libc] handle memset sequence as a separate struct

[Guillaume Chatelet via libc-commits]

Author: Guillaume Chatelet
Date: 2023-04-19T08:37:49Z
New Revision: 458734b8b9ab457e3c88b76e87761a8f4c27a290

URL: https://github.com/llvm/llvm-project/commit/458734b8b9ab457e3c88b76e87761a8f4c27a290
DIFF: https://github.com/llvm/llvm-project/commit/458734b8b9ab457e3c88b76e87761a8f4c27a290.diff

LOG: [reland][libc] handle memset sequence as a separate struct

These sequence of calls don't really make sense for head_tail and loop_and_tail.

Added: 
    

Modified: 
    libc/src/string/memory_utils/memset_implementations.h
    libc/src/string/memory_utils/op_generic.h
    libc/test/src/string/memory_utils/op_tests.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/string/memory_utils/memset_implementations.h b/libc/src/string/memory_utils/memset_implementations.h
index dfc7df87aceb2..e78a4470f4985 100644
--- a/libc/src/string/memory_utils/memset_implementations.h
+++ b/libc/src/string/memory_utils/memset_implementations.h
@@ -57,7 +57,7 @@ inline_memset_x86(Ptr dst, uint8_t value, size_t count) {
   if (count == 2)
     return generic::Memset<uint16_t>::block(dst, value);
   if (count == 3)
-    return generic::Memset<uint16_t, uint8_t>::block(dst, value);
+    return generic::MemsetSequence<uint16_t, uint8_t>::block(dst, value);
   if (count <= 8)
     return generic::Memset<uint32_t>::head_tail(dst, value, count);
   if (count <= 16)

diff  --git a/libc/src/string/memory_utils/op_generic.h b/libc/src/string/memory_utils/op_generic.h
index 1d203d6261403..663f42809ecc9 100644
--- a/libc/src/string/memory_utils/op_generic.h
+++ b/libc/src/string/memory_utils/op_generic.h
@@ -165,11 +165,6 @@ template <typename First, typename... Ts> struct SupportedTypes {
   using TypeFor = typename details::Largest<Size, First, Ts...>::type;
 };
 
-// Returns the sum of the sizeof of all the TS types.
-template <typename... TS> static constexpr size_t sum_sizeof() {
-  return (... + sizeof(TS));
-}
-
 // Map from sizes to structures offering static load, store and splat methods.
 // Note: On platforms lacking vector support, we use the ArrayType below and
 // decompose the operation in smaller pieces.
@@ -213,8 +208,8 @@ using getTypeFor = cpp::conditional_t<
 // Memset
 ///////////////////////////////////////////////////////////////////////////////
 
-template <typename T, typename... TS> struct Memset {
-  static constexpr size_t SIZE = sum_sizeof<T, TS...>();
+template <typename T> struct Memset {
+  static constexpr size_t SIZE = sizeof(T);
 
   LIBC_INLINE static void block(Ptr dst, uint8_t value) {
     static_assert(is_element_type_v<T>);
@@ -226,8 +221,6 @@ template <typename T, typename... TS> struct Memset {
       for (size_t I = 0; I < array_size_v<T>; ++I)
         store<value_type>(dst + (I * sizeof(value_type)), Splat);
     }
-    if constexpr (sizeof...(TS))
-      Memset<TS...>::block(dst + sizeof(T), value);
   }
 
   LIBC_INLINE static void tail(Ptr dst, uint8_t value, size_t count) {
@@ -250,12 +243,22 @@ template <typename T, typename... TS> struct Memset {
   }
 };
 
+template <typename T, typename... TS> struct MemsetSequence {
+  static constexpr size_t SIZE = (sizeof(T) + ... + sizeof(TS));
+  LIBC_INLINE static void block(Ptr dst, uint8_t value) {
+    Memset<T>::block(dst, value);
+    if constexpr (sizeof...(TS) > 0) {
+      return MemsetSequence<TS...>::block(dst + sizeof(T), value);
+    }
+  }
+};
+
 ///////////////////////////////////////////////////////////////////////////////
 // Memmove
 ///////////////////////////////////////////////////////////////////////////////
 
 template <typename T> struct Memmove {
-  static constexpr size_t SIZE = sum_sizeof<T>();
+  static constexpr size_t SIZE = sizeof(T);
 
   LIBC_INLINE static void block(Ptr dst, CPtr src) {
     store<T>(dst, load<T>(src));

diff  --git a/libc/test/src/string/memory_utils/op_tests.cpp b/libc/test/src/string/memory_utils/op_tests.cpp
index b63a629da3f05..f845f49a01d90 100644
--- a/libc/test/src/string/memory_utils/op_tests.cpp
+++ b/libc/test/src/string/memory_utils/op_tests.cpp
@@ -21,6 +21,18 @@
 
 namespace __llvm_libc {
 
+template <typename T> struct has_head_tail {
+  template <typename C> static char sfinae(decltype(&C::head_tail));
+  template <typename C> static uint16_t sfinae(...);
+  static constexpr bool value = sizeof(sfinae<T>(0)) == sizeof(char);
+};
+
+template <typename T> struct has_loop_and_tail {
+  template <typename C> static char sfinae(decltype(&C::loop_and_tail));
+  template <typename C> static uint16_t sfinae(...);
+  static constexpr bool value = sizeof(sfinae<T>(0)) == sizeof(char);
+};
+
 // Allocates two Buffer and extracts two spans out of them, one
 // aligned and one misaligned. Tests are run on both spans.
 struct Buffers {
@@ -132,7 +144,10 @@ using MemsetImplementations = testing::TypeList<
 #endif
     generic::Memset<uint32_t>, generic::Memset<cpp::array<uint32_t, 2>>, //
     generic::Memset<uint16_t>, generic::Memset<cpp::array<uint16_t, 2>>, //
-    generic::Memset<uint8_t>, generic::Memset<cpp::array<uint8_t, 2>>    //
+    generic::Memset<uint8_t>, generic::Memset<cpp::array<uint8_t, 2>>,   //
+    generic::MemsetSequence<uint8_t, uint8_t>,                           //
+    generic::MemsetSequence<uint16_t, uint8_t>,                          //
+    generic::MemsetSequence<uint32_t, uint16_t, uint8_t>                 //
     >;
 
 // Adapt CheckMemset signature to op implementation signatures.
@@ -158,7 +173,8 @@ TYPED_TEST(LlvmLibcOpTest, Memset, MemsetImplementations) {
       }
     }
   }
-  { // Test head tail operations from kSize to 2 * kSize.
+  if constexpr (has_head_tail<Impl>::value) {
+    // Test head tail operations from kSize to 2 * kSize.
     static constexpr auto HeadTailImpl = SetAdaptor<Impl::head_tail>;
     Buffer DstBuffer(2 * kSize);
     for (size_t size = kSize; size < 2 * kSize; ++size) {
@@ -167,7 +183,8 @@ TYPED_TEST(LlvmLibcOpTest, Memset, MemsetImplementations) {
       ASSERT_TRUE(CheckMemset<HeadTailImpl>(dst, value, size));
     }
   }
-  { // Test loop operations from kSize to 3 * kSize.
+  if constexpr (has_loop_and_tail<Impl>::value) {
+    // Test loop operations from kSize to 3 * kSize.
     if constexpr (kSize > 1) {
       static constexpr auto LoopImpl = SetAdaptor<Impl::loop_and_tail>;
       Buffer DstBuffer(3 * kSize);