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

[Andre Vieira via Phabricator via llvm-commits]

avieira added a comment.

In D105440#2861293 <https://reviews.llvm.org/D105440#2861293>, @gchatelet wrote:

> In D105440#2860255 <https://reviews.llvm.org/D105440#2860255>, @efriedma wrote:
>
>> In D105440#2859993 <https://reviews.llvm.org/D105440#2859993>, @gchatelet wrote:
>>
>>> 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.
>>
>> The problem, from my perspective, is that you'll have to duplicate the logic to some extent anyway.
>
> I see, we'll probably have to extract the shared logic in one or more libraries then.

I initially benchmarked memcmp using the scalar building blocks and I was seeing better codegen with the builtins. I couldn't quite remember what the differences were so I decided to benchmark the scalar implementations again and they seem to perform fairly similar now, there is a small codechange around size 3, but the codegen actually seems better to me with the scalar implementation. I may have been using an older llvm or an earlier version of the building blocks.

>> Even if __builtin_memcmp_inline is technically available, that doesn't let you access the underlying heuristics, so you'll need special-case logic for every target anyway.
>
> Yes indeed, availability of vector extensions could lead to different algorithms so we need to access target specific logic.

I am happy to drop this RFC if there are no objections.


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

https://reviews.llvm.org/D105440