[libc-commits] [PATCH] D126768: [libc][mem*] Introduce Sized/Backends for new mem framework

[Clement Courbet via Phabricator via libc-commits]

courbet added inline comments.


================
Comment at: libc/src/string/memory_utils/backend_aarch64.h:16-20
+  template <typename T, Temporality TS, Aligned AS,
+            cpp::EnableIfType<ScalarBackend::IsScalarType<T>, bool> = true>
+  static inline T load(const T *src) {
+    return ScalarBackend::template load<T, TS, AS>(src);
+  }
----------------
It's not clear what this does: 
 - if `T` scalar type, it delegates to the scalar backend, i.e. a noop.
 - if not, the compiler will select the implementation in `ScalarBackend`, which will assert that `T` is a scalar type. 

What am I missing ?


================
Comment at: libc/src/string/memory_utils/backend_aarch64.h:23
+
+// Implementation of the SizedOp abstraction for the set operation.
+struct Zva64 {
----------------
fix comment ?


================
Comment at: libc/src/string/memory_utils/backend_scalar.h:14-15
+  static constexpr bool IsScalarType =
+      cpp::IsSameV<T, uint8_t> || cpp::IsSameV<T, uint16_t> ||
+      cpp::IsSameV<T, uint32_t> || cpp::IsSameV<T, uint64_t>;
+
----------------
is that true of all architectures ?


================
Comment at: libc/src/string/memory_utils/backend_scalar.h:21
+    static_assert(TS == Temporality::TEMPORAL,
+                  "Scalar load does not support non-temporal access");
+    return *src;
----------------
is that true of all architectures ?


================
Comment at: libc/src/string/memory_utils/backend_scalar.h:35
+    static_assert(IsScalarType<T>);
+    return (T(~0) / T(0xFF)) * T(value);
+  }
----------------
It's not immediately obvious that this works for `uint64_t`. What about `~T(0) / T(0xFF)` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126768