[libc-commits] [libc] 3950e44 - Revert "[libc] handle memset sequence as a separate struct"

[Guillaume Chatelet via libc-commits]

Author: Guillaume Chatelet
Date: 2023-04-18T15:24:24Z
New Revision: 3950e44eb0ad8d12705d9211c76d622c76f387e9

URL: https://github.com/llvm/llvm-project/commit/3950e44eb0ad8d12705d9211c76d622c76f387e9
DIFF: https://github.com/llvm/llvm-project/commit/3950e44eb0ad8d12705d9211c76d622c76f387e9.diff

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

This patch broke the build bots.
This reverts commit 6fdc244e9f13d2ce20a26d05513f22a1e099f90a.

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 e78a4470f4985..dfc7df87aceb2 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::MemsetSequence<uint16_t, uint8_t>::block(dst, value);
+    return generic::Memset<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 924acf40cf9e4..1d203d6261403 100644
--- a/libc/src/string/memory_utils/op_generic.h
+++ b/libc/src/string/memory_utils/op_generic.h
@@ -165,6 +165,11 @@ 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.
@@ -208,8 +213,8 @@ using getTypeFor = cpp::conditional_t<
 // Memset
 ///////////////////////////////////////////////////////////////////////////////
 
-template <typename T> struct Memset {
-  static constexpr size_t SIZE = sizeof(T);
+template <typename T, typename... TS> struct Memset {
+  static constexpr size_t SIZE = sum_sizeof<T, TS...>();
 
   LIBC_INLINE static void block(Ptr dst, uint8_t value) {
     static_assert(is_element_type_v<T>);
@@ -221,6 +226,8 @@ template <typename T> 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) {
@@ -243,22 +250,12 @@ template <typename T> 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)) {
-      return MemsetSequence<TS...>::block(dst + sizeof(T), value);
-    }
-  }
-};
-
 ///////////////////////////////////////////////////////////////////////////////
 // Memmove
 ///////////////////////////////////////////////////////////////////////////////
 
 template <typename T> struct Memmove {
-  static constexpr size_t SIZE = sizeof(T);
+  static constexpr size_t SIZE = sum_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 f845f49a01d90..b63a629da3f05 100644
--- a/libc/test/src/string/memory_utils/op_tests.cpp
+++ b/libc/test/src/string/memory_utils/op_tests.cpp
@@ -21,18 +21,6 @@
 
 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 {
@@ -144,10 +132,7 @@ 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::MemsetSequence<uint8_t, uint8_t>,                           //
-    generic::MemsetSequence<uint16_t, uint8_t>,                          //
-    generic::MemsetSequence<uint32_t, uint16_t, uint8_t>                 //
+    generic::Memset<uint8_t>, generic::Memset<cpp::array<uint8_t, 2>>    //
     >;
 
 // Adapt CheckMemset signature to op implementation signatures.
@@ -173,8 +158,7 @@ TYPED_TEST(LlvmLibcOpTest, Memset, MemsetImplementations) {
       }
     }
   }
-  if constexpr (has_head_tail<Impl>::value) {
-    // Test head tail operations from kSize to 2 * kSize.
+  { // 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) {
@@ -183,8 +167,7 @@ TYPED_TEST(LlvmLibcOpTest, Memset, MemsetImplementations) {
       ASSERT_TRUE(CheckMemset<HeadTailImpl>(dst, value, size));
     }
   }
-  if constexpr (has_loop_and_tail<Impl>::value) {
-    // Test loop operations from kSize to 3 * kSize.
+  { // 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);