Re: [PATCH] Kconfig: Explicitly disable asm goto w/ outputs on gcc-11 (and earlier)

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Nick Desaulniers <ndesaulniers@google.com>,
	Uros Bizjak <ubizjak@gmail.com>,
	 Jakub Jelinek <jakub@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>,
	"Andrew Pinski (QUIC)" <quic_apinski@quicinc.com>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	 Peter Zijlstra <peterz@infradead.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH] Kconfig: Explicitly disable asm goto w/ outputs on gcc-11 (and earlier)
Date: Fri, 9 Feb 2024 12:39:31 -0800	[thread overview]
Message-ID: <CAHk-=wgBt9SsYjyHWn1ZH5V0Q7P6thqv_urVCTYqyWNUWSJ6_g@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wgEABCwu7HkJufpWC=K7u_say8k6Tp9eHvAXFa4DNXgzQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2386 bytes --]

On Fri, 9 Feb 2024 at 11:01, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> We should also probably get rid of the existing "asm_volatile_goto()"
> macro name entirely. That name was always pretty horrific, in that it
> didn't even mark the asm as volatile even in the case where it did
> anything.
>
> So the name of that macro made little sense, and the new workaround
> should be only for asm goto with outputs. So I'd suggest jmaking a new
> macro with that name:
>
>    #define asm_goto_output(x...)
>
> and on gcc use that old workaround, and on clang just make it be a
> plain "asm goto".

So here's a suggested patch that does this.

It's largely done with "git grep" and "sed -i", plus some manual
fixups for the (few) cases where we have outputs.

It looks superficially sane to me, and it passed an allmodconfig build
with gcc, but I'm not going to claim that it is really tested.

Sean? Does this work for the case you noticed?

Basically this gets rid of the old "asm_volatile_goto()" entirely as
useless, but replaces it with "asm_goto_outputs()" for the places
where we have outputs.

And then for gcc, it makes those cases

 (a) use "asm volatile goto" to fix the fact that some versions of gcc
will have missed the "volatile"

 (b) adds that extra empty asm as a second barrier after the "real"
asm goto statement

That (b) is very much voodoo programming, but it matches the old magic
barrier thing that Jakub Jelinek suggested for the really *old* gcc
bug wrt plain (non-output) "asm goto". The underlying bug for _that_
was fixed long ago:

    http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670

We removed that for plain "asm goto" workaround a couple of years ago,
so "asm_volatile_goto()" has been a no-op since June 2022, but this
now resurrects that hack for the output case.

I'm not loving it, but Sean seemed to confirm that it fixes the code
generation problem, so ...

Adding Uros to the cc, since he is both involved with gcc and with the
previous asm goto workaround removal, so maybe he has other
suggestions. Uros, see

    https://lore.kernel.org/all/20240208220604.140859-1-seanjc@google.com/

for background.

Also adding Jakub since I'm re-using the hack he suggested for a
different - but similar - case. He may have strong opinions too, and
may object to that particular monkey-see-monkey-do voodoo programming.

                Linus

[-- Attachment #2: patch.diff --]
[-- Type: application/x-patch, Size: 32071 bytes --]