[libc-commits] [PATCH] D135134: [libc] New version of the mem* framework

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet added inline comments.


================
Comment at: libc/src/string/memory_utils/op_aarch64.h:36
+  static inline void block(Ptr dst, uint8_t) {
+    static_assert(Size == 64);
+#if __SIZEOF_POINTER__ == 4
----------------
courbet wrote:
> Does the class need to be a template ?
It does for the loop and tail function below.
I'd rather not have the template on the function instead of the struct because it will break code symmetry in the implementation.


================
Comment at: libc/src/string/memory_utils/op_generic.h:93
+          CTPair<2, ScalarType<uint16_t>>, //
+          CTPair<4, ScalarType<uint32_t>>, //
+#if !defined(LLVM_LIBC_ARCH_X86_32)        //
----------------
courbet wrote:
> Do we support any non-32-bit platform ? e.g. 8- ot 16- bit ones ?
No. I've added a static_assert.


================
Comment at: libc/src/string/memory_utils/op_generic.h:353-359
+  // e.g. Moving two bytes and making sure dst is then aligned.
+  // [  |       |       |       |      ]
+  // [____XXXXXXXXXXXXXXXXXXXXXXXXXXXX_]
+  // [____LLLLLLLL_____________________]
+  // [______LLLLLLLL___________________]
+  // [_SSSSSSSS________________________]
+  // [___SSSSSSSS______________________]
----------------
courbet wrote:
> not sure why there are two diagrams (here and below)
This is to illustrate source vs destination alignment.
I've slightly edited the diagrams to point out where the buffer is actually aligned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135134