Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAPM=9twNnV4zMCvrPkw3H-ajZOH-01JVh_kDrxdPYQErz8ZTdA@mail.gmail.com>
 <CAHk-=wi=eDN4Ub0qSN27ztBAvHSXCyiY2stu3_XbTpYpQX4x7w@mail.gmail.com> <CAHk-=wg9p+oJtzC0582vke8whcWPNSC9eq0Z6kdnSqWMEpah7g@mail.gmail.com>
In-Reply-To: <CAHk-=wg9p+oJtzC0582vke8whcWPNSC9eq0Z6kdnSqWMEpah7g@mail.gmail.com>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Fri, 23 Jun 2023 12:16:26 -0700
Message-ID: <CAKwvOd=dUWWSzQEWdguhe3SjYTw7a76skx370DbU2Y+5o=M8BA@mail.gmail.com>
Subject: Re: arm32 build warnings in workqueue.c
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Dave Airlie <airlied@gmail.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>, Tejun Heo <tj@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        clang-built-linux <llvm@lists.linux.dev>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Jun 23, 2023 at 11:52=E2=80=AFAM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> [ Adding clang people. See this for background:
>
>     https://lore.kernel.org/all/CAHk-=3Dwi=3DeDN4Ub0qSN27ztBAvHSXCyiY2stu=
3_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 =3D 0x1FFFFFFFE0
> -       andq    %rax, %rcx
> -       movabsq $68719476704, %rdx              # imm =3D 0xFFFFFFFE0
> -       cmpq    %rdx, %rcx
> +       shrq    $5, %rax
> +       cmpl    $2147483647, %eax               # imm =3D 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 =3D 0xFFFFFFFE0
> -       orq     $1, %rax
> +       movabsq $68719476705, %rax              # imm =3D 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


--=20
Thanks,
~Nick Desaulniers