On Tue, Jan 17, 2023 at 11:17 AM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> Perhaps that was compiler version or config specific?
Possible, but...
The clang code generation annoyed me enough that I actually ended up
rewriting the unlikely test to be outside the loop in commit
ae2a823643d7 ("dcache: move the DCACHE_OP_COMPARE case out of the
__d_lookup_rcu loop").
I think that then made clang no longer have the whole "rotate loop
with unlikely case in the middle" issue.
And then because clang *still* messed up by trying to be too clever (see
https://lore.kernel.org/all/CAHk-=wjyOB66pofW0mfzDN7SO8zS1EMRZuR-_2aHeO+7kuSrAg@xxxxxxxxxxxxxx/
for details), I also ended up doing commit c4e34dd99f2e ("x86:
simplify load_unaligned_zeropad() implementation").
The end result is that now the compiler almost *cannot* mess up any more.
So the reason clang now does a good job on __d_lookup_rcu() is largely
that I took away all the places where it did badly ;)
That said, clang still generates more register pressure than gcc,
causing the function prologue and epilogue to be rather bigger
(pushing and popping six registers, as opposed to gcc that only needs
three)
Gcc is also better able to schedule the prologue and epilogue together
with the work of the function, which clang seems to always do it as a
"push all" and "pop all" sequence.
That scheduling doesn't matter in that particular place (although it
does make the unlikely case of calling __d_lookup_rcu_op_compare
pointlessly push all regs only to then pop them), but I've seen a few
other cases where it ends up meaning that it always does that full
function prologue even when the *likely* case then returns early and
doesn't actually need any of that work because it didn't use any of
those registers.
But yeah, the RCU pathname lookup looks fine these days. And I don't
actually think it was due to clang changes ;)
Linus
