Re: [PATCH 6/7] stdlib: Optimization qsort{_r} swap implementation

[Paul Eggert <eggert at cs dot ucla dot edu>]

On 01/22/2018 02:55 AM, Adhemerval Zanella wrote:
> On 22/01/2018 06:27, Paul Eggert wrote:
> > Adhemerval Zanella wrote:
> > > +static inline bool
> > > +check_alignment (const void *base, size_t align)
> > > +{
> > > +  return _STRING_ARCH_unaligned || ((uintptr_t)base % (align - 1)) == 0;
> > > +}
> > Surely the '(align - 1)' was supposed to be 'align'. Has this been tested on an architecture that does not allow unaligned access?
> Yes, I checked on sparc64 machine.  This test is similar to the Linux kernel one
> at lib/sort.c.

The Linux kernel lib/sort.c test is (((unsigned long)base & (align - 1)) 
== 0), which is correct. The test above uses '% (align - 1)' instead, 
which is clearly wrong; the "%" should be "&". As the tests evidently 
did not catch the error, they need to be improved to catch it.

> It might be that your approach is faster for other architectures which do not have
> ifunc mempcpy, however I do not want to over-engineer this code since most real
> word correspond to key sizes of 4 and 8.

Thanks, that all makes sense.

One other question. Would it improve performance to partially evaluate 
qsort for the case where the key size is that of a pointer, to allow the 
swap to be done inline with four insns? I would imagine that this is the 
most common case.