[libc-commits] [PATCH] D128335: [libc][mem*] Introduce Algorithms for new mem framework

[Clement Courbet via Phabricator via libc-commits]

courbet added inline comments.


================
Comment at: libc/src/string/memory_utils/algorithm.h:120
+// Precondition: `runtime_size >= Size && runtime_size <= 2 x Size`.
+template <typename SizedOp> struct HeadTail {
+  template <typename DstAddrT, typename SrcAddrT>
----------------
Maybe use a different name, e.g. `SizedOpT` ? `SizedOp` is already the name of a template, and name collisions are always hard to debug (here and everywhere else).


================
Comment at: libc/src/string/memory_utils/algorithm.h:118
+template <typename SizedOp> struct HeadTail {
+  template <typename DstAddrT, typename SrcAddrT>
+  static inline void copy(DstAddrT dst, SrcAddrT src, size_t runtime_size) {
----------------
courbet wrote:
> gchatelet wrote:
> > courbet wrote:
> > > maybe: 
> > > 
> > > ```
> > > static_assert(SizedOp::SIZE >0);
> > > ```
> > > 
> > > to fail explicitly if `SizedOp` is not a statically sized operation.
> > By definition `SizedOp` **is** a statically sized operation 
> That's what I want to check :) 
> 
> I think that you essentially have two kinds of operations:
>  - What I was calling "statically sized operations": `_1`, `_2`, ..., ``.
>  - 
> 
> Maybe a better name would be "Composite operations" for the later and "Elementary operations" for the former.
> 
> My point is that I think some operations only take elementary operations, for example, this does not really make sense:
> 
> `HeadTail<Loop<_16>>`
> 
> We might even want `static constexpr  bool IS_ELEMENTARY = true`, or something like this.
> 
> WDYT ?
> 
> 
> 
Actually an elementary `SizedOp` is an instance of `SizeOp<BackendT, Size>`. So we could have a `IsElementaryOp<SizedOpT>` that checks that.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128335