[libc-commits] [PATCH] D136292: [libc] mem* framework v3

[Clement Courbet via Phabricator via libc-commits]

courbet added inline comments.


================
Comment at: libc/src/string/memory_utils/op_aarch64.h:46
+  static inline void loop_and_tail(Ptr dst, uint8_t value, size_t count) {
+    static_assert(Size > 1);
+    size_t offset = 0;
----------------
Maybe give a better error message: `"a loop of size 1 does not need tail, use 'Loop' instead"` ?


================
Comment at: libc/src/string/memory_utils/op_aarch64.h:71
+///////////////////////////////////////////////////////////////////////////////
+// Memset
+
----------------
?


================
Comment at: libc/src/string/memory_utils/op_generic.h:89
+
+// We currently don't support 8- or 16-bit platforms, it must be 32- or 64-bit.
+static_assert((UINTPTR_MAX == 4294967295U) ||
----------------
Make this the `static_assert` message


================
Comment at: libc/test/src/string/memory_utils/CMakeLists.txt:12
     -ffreestanding
-    -pthread
   DEPENDS
----------------
submit this separately ?


================
Comment at: libc/test/src/string/memory_utils/elements_test.cpp:45
 
-char GetRandomChar() {
+static char GetRandomChar() {
   static constexpr const uint64_t a = 1103515245;
----------------
ditto


================
Comment at: libc/test/src/string/memory_utils/op_tests.cpp:97
+
+template <auto Foo>
+bool CheckMemcpy(cpp::span<char> dst, cpp::span<char> src, size_t size) {
----------------
`FnImpl` ?


================
Comment at: libc/test/src/string/memory_utils/op_tests.cpp:110
+template <typename T> static void MemcpyAdaptor(Ptr dst, CPtr src, size_t) {
+  return T::block(dst, src);
+}
----------------
`assert(size == T::SIZE)` ?


================
Comment at: libc/test/src/string/memory_utils/op_tests.cpp:126
+  }
+  { // Test head tail operations
+    RawAlignedBuffer SrcBuffer(2 * kSize);
----------------
", from `size` to `2*size`"


================
Comment at: libc/test/src/string/memory_utils/op_tests.cpp:136
+  }
+  { // Test loop operations
+    if constexpr (kSize > 1) {
----------------
ditto


================
Comment at: libc/test/src/string/memory_utils/op_tests.cpp:183
+
+template <auto Foo>
+bool CheckMemset(cpp::span<char> dst, uint8_t value, size_t size) {
----------------
ditto (here and below)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136292/new/

https://reviews.llvm.org/D136292