[libc-commits] [PATCH] D106641: [libc] rewrite aarch64 memcmp implementation

[Andre Vieira via Phabricator via libc-commits]

avieira added a comment.

LGTM



================
Comment at: libc/src/string/aarch64/memcmp.cpp:43
+  // [64, 127]
+  return ThreeWayCompare<Skip<32>::Then<Loop<_16>>>(lhs, rhs, count);
 }
----------------
avieira wrote:
> gchatelet wrote:
> > @avieira shouldn't this be `Skip<64>` instead of `Skip<32>` here?
> No I think 32 is right, It only goes through two _16 Equals before reaching this on lines 34 and 38. However, since it's guaranteed >= 64 we could do something like Skip<32>::Then<Chained<_32,Loop<_16>> (untested).
> 
> I'll benchmark it and let you know.
Sorry it took so long, kinda fell through the cracks. But yeah use this patch as is, don't make it Skip<64>.

I benchmarked the Chained<_32, Loop<_16>> approach and it lead to slightly worst codegen. FYI I didn't use Chained as that requires the Tail to be a constant size operation, so I had locally created a new element to do a sequence, almost like a 'ChainEnd' where the tail can be variable size and you can't chain it further.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106641