[PATCH] D56593: [SelectionDAG][RFC] Allow the user to specify a memeq function (v5).

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 6 13:12:25 PDT 2021


goldstein.w.n added a comment.

In D56593#3113727 <https://reviews.llvm.org/D56593#3113727>, @MaskRay wrote:

> In D56593#3113701 <https://reviews.llvm.org/D56593#3113701>, @goldstein.w.n wrote:
>
>> In D56593#1393325 <https://reviews.llvm.org/D56593#1393325>, @jyknight wrote:
>>
>>> It'd be great to see this somewhat more widely publicized, outside of just the clang community. If libc implementors are aware of the gains and are willing to provide an actually-faster bcmp implementation, it'd be a lot better, than having this optimization that doesn't really optimize anything without users providing their own bcmp implementation.
>>
>> Tried to add an optimized `bcmp` for GLIBC here <https://marc.info/?t=163157542200002&r=1&w=3>.
>>
>> It was not well received because `bcmp` is not a standard function. It seems GLIBC supports `bcmp` out of reluctant necessity rather than any desire for it to be fast.
>>
>> There was agreement that the functionality was useful so GLIBC landed on `__memcmpeq'`to get the functionality. The patches made it to HEAD <https://sourceware.org/git/?p=glibc.git;a=commit;h=44829b3ddb64e99e37343a0f25b2c082387d31a5>
>> and will be available starting with the 2.35 release. It declared in "string.h" or can be queried with GLIBC version >= 2.35.
>>
>> Currently only x86_64 has an optimized version, the rest of the targets still just redirect to `memcmp`.
>>
>> Working on a patch to add support in LLVM.
>
> I understand and accept the viewpoint that: for glibc, bumping the symbol version for `bcmp` may lead to difficuly-to-debug issues when users try to upgrade glibc.
> An ABI-only symbol in the reserved namespace looks good to me.
>
> AFAIK while LLVM has `Triple::isOSLinux` and `Triple::isGNUEnvironment` and there are optimizations dispatching on linux-gnu there is no way detecting the version.  (well, Apple platforms and some other OSes have such checks)
> The optimizations just assume that a very old glibc version may be used (perhaps 2.10+ or early 2.20+).
> In addition, there are many cross compiling users where no version is specified.

I was planing to add  `Triple::isGNUVersionLT` and use it in a function `hasMemcmpeq` in a similar vein to `hasBcmp`. Will there be an issue with that?

> The glibc 2.35 availability does not make this optimization exploitable, at least for few (5+?) years :)

Indeed :/

> One option is to hide such transformation under a `cl::opt` option.

Sorry not sure I understand, can you explain?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56593



More information about the llvm-commits mailing list