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

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 16/05/2022 20:01, Alexander Monakov wrote:
> 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.

Thanks for the context, this should go into comments.  A wishlist bug 
would be nice but I suspect it'll just gather dust.  Maybe it's still 
useful for someone coming in after 10-15 years looking for more context 
on it.

>> - 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.

I would prefer the earlier variant in generic code, with (if necessary) 
the scheduling hack being a sysdep for x86.  Other architectures that 
want to use the latter should #include it and also post microbenchmark 
results so that we keep track of how we arrived at that decision.

Thanks,
Siddhesh