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

[Clement Courbet via Phabricator via libc-commits]

courbet added inline comments.


================
Comment at: libc/src/string/memory_utils/utils.h:69
+template <size_t alignment>
+intptr_t offset_from_last_aligned_or_zero(const void *ptr) {
   static_assert(is_power2(alignment), "alignment must be a power of 2");
----------------
`offset_to_align_up` would be consistent with existing terminology (`__builtin_align_up` and co.) and would avoid having two meanings for `next`.


================
Comment at: libc/src/string/memory_utils/utils.h:90
+
 // Returns the offset from `ptr` to the next cache line.
 static inline intptr_t offset_to_next_cache_line(const void *ptr) {
----------------
This terminology is not consistent to what's above. It should be `offset_to_cacheline_up` or something like this. Also it would not hurt to comment the behaviour explicitly.


================
Comment at: libc/src/string/memory_utils/utils.h:126
 
-// Loads bytes from memory (possibly unaligned) and materializes them as type.
+template <typename T> struct StrictScalarType {
+  static_assert(cpp::is_integral_v<T>);
----------------
What's strict about that scalar :) ?


================
Comment at: libc/src/string/memory_utils/utils.h:169
+// Advances p1 and p2 so p1 gets aligned to the next SIZE bytes boundary
+// and decrease count by the same amount.
 // We make sure the compiler knows about the adjusted pointer alignment.
----------------
Maybe `// precondition: &p1 != &p2`, because `__restrict` is not part of the signature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135134