On Tue, Feb 28, 2023 at 11:39 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> This actually looks sane enough that I might even boot it. Call me crazy.
Oh, and while I haven't actually booted it or tested it in any way, I
did verify that it changes
- movq 48(%rbx), %rax
- movq 56(%rbx), %rcx
- cmpl %eax, %ecx
- jne .LBB58_13
- shrq $32, %rax
- shrq $32, %rcx
- cmpl %eax, %ecx
- jne .LBB58_13
into
+ movq 56(%rbx), %rax
+ cmpq 48(%rbx), %rax
+ jne .LBB58_12
because it looks like clang was smart enough to unroll the silly
fixed-size loop and do the two adjacent 32-bit loads of the old cap[]
array as one 64-bit load, but then was _not_ smart enough to combine
the two 32-bit compares into one 64-bit one.
And gcc didn't do the load optimization (which is questionable since
it then just results in extra shifts and extra registers), so it just
kept it as two 32-bit loads and compares. Again, with the patch, gcc
obviously does the sane "one 64-bit load, one 64-bit compare" too.
There's a lot to be said for compiler optimizations fixing up silly
source code, but I personally would just prefer to make the source
code DTRT.
Could the compiler have been even smarter and generated the same code?
Yes it could. We shouldn't expect that, though. Particularly when the
sane code is much more legible to humans too.
Linus
