Re: [PATCH v8 6/6] elf: Optimize _dl_new_hash in dl-new-hash.h

[Alexander Monakov <amonakov@ispras.ru>]

On Mon, 16 May 2022, Siddhesh Poyarekar wrote:

> There are a couple of things that seem problematic to me about this:
> 
> - It seems like we're trying to fix a gcc issue in glibc.  Couldn't we file a
> gcc bug and explore ways in which this could be supported in the compiler?  In
> fact, it might make sense to do that for the original loop; it looks like a
> missed optimization that gcc ought to fix.  IMO the bug should be filed even
> if we do end up with this micro-optimization in glibc.

This issue involves a chain of dependencies that goes across all loop
iterations, but relevant compiler optimization (reassociation, register
allocation, scheduling) do not consider such global chains. You might
file this as a "wishlist" bug, but compiler infrastructure is simply
not designed to make such nontrivial decisions.

> - The patch controls an instruction schedule so that it works well on
> out-of-order processors but then only quoting one microarchitecture.

It's not specific to out-of-order processors: a long chain of dependencies
restricts OoO scheduling in the CPU. So in the end it benefits "classic"
and OoO pipelines in a similar fashion.

> If it
> works well on TigerLake (and on x86 in general) then it might be better to add
> it as a sysdep override; I assumed that was the point of breaking the function
> out into its header anyway.  If it is more generally useful then please share
> numbers to that effect in the commit message and also explicitly state in the
> comments why we're trying to exert this level of control on codegen in generic
> C code and why it is good for all architectures.

I guess it's up to you and Noah to hash it out, but I'd like to remind that
there was an alternative variant which is a strict win on all architectures
(same code size, same instruction mix, no dependency on fast multiplication).
That might be easier to justify from generic code point of view.

Alexander