Re: [PATCH v7] String: Add support for __memcmpeq() ABI on all targets

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Oct 21, 2021 at 1:02 PM Noah Goldstein via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Thu, Oct 21, 2021 at 2:33 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> >
> >
> >
> > On 29/09/2021 12:17, Noah Goldstein via Libc-alpha wrote:
> > > No bug.
> > >
> > > This commit adds support for __memcmpeq() as a new ABI for all
> > > targets. In this commit __memcmpeq() is implemented only as an alias
> > > to the corresponding targets memcmp() implementation. __memcmpeq() is
> > > added as a new symbol starting with GLIBC_2.35 and defined in string.h
> > > with comments explaining its behavior. Basic tests that it is callable
> > > and works where added in string/tester.c
> > >
> > > As discussed in the proposal "Add new ABI '__memcmpeq()' to libc"
> > > __memcmpeq() is essentially a reserved namespace for bcmp(). The means
> > > is shares the same specifications as memcmp() except the return value
> > > for non-equal byte sequences is any non-zero value. This is less
> > > strict than memcmp()'s return value specification and can be better
> > > optimized when a boolean return is all that is needed.
> > >
> > > __memcmpeq() is meant to only be called by compilers if they can prove
> > > that the return value of a memcmp() call is only used for its boolean
> > > value.
> > >
> >
> > Should we export memcmpeq() as a GNU extension as well?
> >
> > > All tests in string/tester.c passed. As well build succeeds on
> > > x86_64-linux-gnu target.
> >
> > This patch requires a NEWS entry for the new symbol and most likely
> > a manual entry to describe its semantic.
>
> My understanding is that since we are only exporting this symbol for
> compiler use (to optimize boolean use of 'memcmp()') we weren't
> planning to add a manual entry.
>
> >
> > I also think we should model a new testing based on test-memcmp.c
> > to check for multiple alignments and random inputs.
>
> In this patch __memcmpeq is always just memcmp which is tested.
>
> In my followup patch (once this one gets through) to add separate
> implementations for x86_64 I add tests. Would it be better to include
> those tests in this patchset?

I think we should add it now since someone can add IFUNC for
a non x86-64 target before x86-64 one is landed.

With __memcmpeq, bcmp should be an alias of __memcmpeq,
not memcmp.

--
H.J.