Received-SPF: pass (google.com: domain of linux-kernel+bounces-59865-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240208220604.140859-1-seanjc@google.com> <CAKwvOdk_obRUkD6WQHhS9uoFVe3HrgqH5h+FpqsNNgmj4cmvCQ@mail.gmail.com>
 <DM6PR02MB40587AD6ABBF1814E9CCFA7CB84B2@DM6PR02MB4058.namprd02.prod.outlook.com>
 <CAHk-=wi3p5C1n03UYoQhgVDJbh_0ogCpwbgVGnOdGn6RJ6hnKA@mail.gmail.com>
 <ZcZyWrawr1NUCiQZ@google.com> <CAKwvOdmKaYYxf7vjvPf2vbn-Ly+4=JZ_zf+OcjYOkWCkgyU_kA@mail.gmail.com>
 <CAHk-=wgEABCwu7HkJufpWC=K7u_say8k6Tp9eHvAXFa4DNXgzQ@mail.gmail.com>
In-Reply-To: <CAHk-=wgEABCwu7HkJufpWC=K7u_say8k6Tp9eHvAXFa4DNXgzQ@mail.gmail.com>
From: Nick Desaulniers <ndesaulniers@google.com>
Date: Fri, 9 Feb 2024 11:20:35 -0800
Message-ID: <CAKwvOdk5E=vu8RNOOsJmTu2ioTYhiqZ=xSj-4YwkKhH8d4wb7A@mail.gmail.com>
Subject: Re: [PATCH] Kconfig: Explicitly disable asm goto w/ outputs on gcc-11
 (and earlier)
To: Linus Torvalds <torvalds@linux-foundation.org>
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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 9, 2024 at 11:01=E2=80=AFAM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, 9 Feb 2024 at 10:55, Nick Desaulniers <ndesaulniers@google.com> w=
rote:
> >
> > And also pessimizes all asm gotos (for gcc) including ones that don't
> > contain output as described in 43c249ea0b1e.  The version checks would
> > at least not pessimize those.
>
> Yeah, no, we should limit that workaround only to the asm goto with
> outputs case.
>
> 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.

+1.

43c249ea0b1e could have done so, but I'm guessing "tree wide changes
in Linux are not fun" was perhaps the reason it wasn't done so then.

I also think folks are too aggressive putting volatile on asm
statements that might not need them; it's definitely less cognitive
burden to just always put `volatile` on inline asm but I suspect
that's leaving some performance on the floor in certain cases.  (I had
a patch to clang to warn when the volatile was unnecessary in cases
where it was explicit, but that was shot down in code review as being
harassing to users).

>
> 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".
>
> Hmm?

Thinking through the tradeoffs, the Kconfig approach would pessimize
GCC versions with the "lack of implicit volatile" bug to not use asm
goto w/ outputs at all.

Having a new macro would just make the `volatile` qualifier explicit,
which is a no-op on gcc versions that don't contain the bug (while
allowing them to use asm goto with outputs, which is probably better
for codegen).

So I agree a new macro seems better than disabling things for kconfig.
(Assuming the only issue is the need to make `volatile` explicit for a
few GCC versions; it's not clear to me from Sean's latest response if
there's more than one bug here).  I'm not signing up to shave either
yak though.
--=20
Thanks,
~Nick Desaulniers