Messages in this thread | | | From | Nick Desaulniers <> | Date | Fri, 23 Jun 2023 12:16:26 -0700 | Subject | Re: arm32 build warnings in workqueue.c |
| |
On Fri, Jun 23, 2023 at 11:52 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > [ 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.
Sorry, are those equivalent? Before looks to me like:
if (0xFFFFFFFE0 - (0x1FFFFFFFE0 & rax))
and after
if (0x7FFFFFFF - (rax >> 5))
> > 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.
Yeah, that's stupid. Which symbol's disassembly are you looking at? I couldn't repro with a quick test: https://godbolt.org/z/dz1fEY9Wx.
> > 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.
I don't think so. https://godbolt.org/z/M8746c49z That's LLVM IR as soon as it leaves the front end. Notice the use of `i32` types. AFAIK, the IR does not contain the notion of enums. Clang lowers enums to integral values of a specific width in LLVM IR.
> > 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
-- Thanks, ~Nick Desaulniers
| |