[PATCH] D105440: [RFC] Implement support for __builtin_memcmp_inline

Guillaume Chatelet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 6 08:43:45 PDT 2021


gchatelet added a comment.

In D105440#2858438 <https://reviews.llvm.org/D105440#2858438>, @efriedma wrote:

> The motivation here seems weaker than it was for memcpy.  memcmp is much less fundamental to LLVM optimizations.  It looks like the primary motivation here is to avoid duplicating the logic in llvm/lib/CodeGen/ExpandMemCmp.cpp ?  But really, it isn't very much target-independent logic, and the only target-specific bit is the heuristic for load widths.

I'd rather not duplicate logic in `llvm/lib/CodeGen/ExpandMemCmp.cpp` they may get out of sync.

>> I'm split on what to do for when the memcmp size is large enough to hit the maximum number of loads. I think there are three options: a) We ignore the maximum number of loads and just generate a lot of code... (kind of the same strategy with memcpy) b) We use a call to memcmp (and issue a diagnostic saying we did so?) c) Error out I'm not keen on (c), I suspect (a) would probably make the most sense and hope that users of the builtin are sensible?
>
> Or (d) generate a loop.  But it would be a bunch of work to implement that, and it's not clear the users of the function would want that.  (a) seems reasonable.

I concur, (a) is the way to go.
Loop generation is an option but it's much more work and involves spreading the logic between layers, I would prefer not going in that direction if possible.
The goal here is to provide building block to create libc memory functions, it's unlikely that the size is going to be big and it is a requirement that it should never call the libc (or it will stackoverflow (or loop infinitely if tail recursion)).
Also the size has to be statically known so I'd be surprised if someone would want to compare 1KiB of memory this way... and the loop is easy to implement on top of the builtin.


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

https://reviews.llvm.org/D105440



More information about the llvm-commits mailing list