Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 23 Jun 2023 11:52:37 -0700 | Subject | Re: arm32 build warnings in workqueue.c |
| |
[ Adding clang people. See this for background:
https://lore.kernel.org/all/CAHk-=wi=eDN4Ub0qSN27ztBAvHSXCyiY2stu3_XbTpYpQX4x7w@mail.gmail.com/
where that patch not only cleans things up, but seems to make a difference to clang ]
On Fri, 23 Jun 2023 at 11:24, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > So I really think that code needs fixing, and the gcc warning was very valid. > > Maybe something like the attached. Does this fix the gcc warning? > Tejun, comments?
Whee. Inexplicably, that patch also improves code generation with clang, with things like this:
- movabsq $137438953440, %rcx # imm = 0x1FFFFFFFE0 - andq %rax, %rcx - movabsq $68719476704, %rdx # imm = 0xFFFFFFFE0 - cmpq %rdx, %rcx + shrq $5, %rax + cmpl $2147483647, %eax # imm = 0x7FFFFFFF
in several places.
Or, even more amusingly, this:
- movabsq $68719476704, %rax # imm = 0xFFFFFFFE0 - orq $1, %rax + movabsq $68719476705, %rax # imm = 0xFFFFFFFE1
where the old code was some truly crazy stuff.
I have *no* idea what drugs clang is on, but clearly clang does some really really bad things with large enums, and doesn't simplify things correctly.
That "I can't even do a constant 'or' at compile time when it involves an enum" is all kinds of odd.
Does this matter in the big picture? No. But I think the take-away here should be that you really shouldn't use enums for random things. Compilers know enums are used as small enumerated constants, and get pissy and confused when you use them as some kind of generic storage pool for values.
My guess is that clang keeps an enum as an enum as long as possible - including past some (really) simple simplification phases of the optimizer.
With gcc, code generation didn't change from that patch with my defconfig on x86-64 (apart from line numbers changing).
Now, clang improving code generation with that patch is obviously a good thing for the patch, but it does mean that clang really messed up before.
Linus
| |