[PATCH] x86_64: Optimize ffsll function code size.

[Sunil Pandey]

On Thu, Jul 27, 2023 at 7:00 AM Alexander Monakov <amonakov@ispras.ru>
wrote:

>
> On Thu, 27 Jul 2023, Florian Weimer via Libc-alpha wrote:
>
> > * Alexander Monakov:
> >
> > > On Thu, 27 Jul 2023, Florian Weimer via Libc-alpha wrote:
> > >
> > >> * Noah Goldstein via Libc-alpha:
> > >>
> > >> > Likewise for the string/memory library....
> > >> > Why not just update it w.o cmov? Just seems like a waste not to
> > >> > address it while we're at it.
> > >>
> > >> Given that it violates the spec, doing it in an inline function seems
> > >> kind of risky.
> > >
> > > Sorry, what inline function? The function seems to modify an
> implementation
> > > in ffsll.c, nothing else.
> >
> > Yeah, sorry, I was confused.  There's a GCC built-in, right?  So the
> > glibc implementation probably isn't that important on x86.
>
> Yep. The built-in gets expanded even at -Os, so it's quite unusual that
> Glibc's
> implementation get called. I see the following possibilities:
>
> * at -O0 (but then performance doesn't matter, presumably)
> * with -fno-builtin
> * when called via a function pointer
>
> Sunil, could you clear this up, please?
>

This issue only matters if ffsll functionality is implemented in a
function(size > 16 byte) and the
function is not inlined (doesn't matter whether it's implemented in C or
assembly).

By default the function entry point gets aligned to the 16 byte boundary,
so the following layout
are all valid.

64 byte aligned: No issue as 17 byte function will not cause another cache
line load.
48 byte aligned: ~20% regression as 17 byte function will trigger another
cache line load.
32 byte aligned: No issue as 17 byte function will not cause another cache
line load.
16 byte aligned: No issue as 17 byte function will not cause another cache
line load.

Ffsll is one of the benchmark tests in the phoronix test suite, not sure
how much it matters to
the application. Lots of people involved in phoronix benchmark
testing/tracking and this kind
of random perf behavior wastes their time.

Again I'm not against GCC, but if this function exists in glibc, I don't
see any harm in fixing it.


>
> Alexander
>