Received-SPF: pass (google.com: domain of linux-kernel+bounces-71606-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240206074552.541154-1-maskray@google.com> <CAMuHMdVOoxVFohcwwoQtY0KgZEvbjgJJ6mib8Fabo97P0krEqw@mail.gmail.com>
 <CAMj1kXGaF5bobHTr1pTg+-=s4Ft7+5SSbX72-NxsR_W_Yuxb8Q@mail.gmail.com>
 <CAMj1kXGw+r7yEEBA8gYBcdrqkiP=VYOSzz9YLnNavJn2snmFwA@mail.gmail.com> <CAMuHMdX+6fnAf8Hm6EqYJPAjrrLO9T7c=Gu3S8V_pqjSDowJ6g@mail.gmail.com>
In-Reply-To: <CAMuHMdX+6fnAf8Hm6EqYJPAjrrLO9T7c=Gu3S8V_pqjSDowJ6g@mail.gmail.com>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Mon, 19 Feb 2024 16:41:03 +0100
Message-ID: <CAMj1kXHh_m=V0QsiTpHrUXpFBXFbFfezdysz8quhPSgUrZg1MA@mail.gmail.com>
Subject: Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
To: Geert Uytterhoeven <geert@linux-m68k.org>, Arnd Bergmann <arnd@arndb.de>
Cc: Fangrui Song <maskray@google.com>, Catalin Marinas <catalin.marinas@arm.com>, 
	Will Deacon <will@kernel.org>, linux-arm-kernel@lists.infradead.org, 
	Jisheng Zhang <jszhang@kernel.org>, Dave Martin <Dave.Martin@arm.com>, 
	Peter Smith <peter.smith@arm.com>, llvm@lists.linux.dev, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, 19 Feb 2024 at 15:43, Geert Uytterhoeven <geert@linux-m68k.org> wro=
te:
>
> Hi Ard,
>
> On Mon, Feb 19, 2024 at 11:57=E2=80=AFAM Ard Biesheuvel <ardb@kernel.org>=
 wrote:
> > On Mon, 19 Feb 2024 at 11:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Mon, 19 Feb 2024 at 11:03, Geert Uytterhoeven <geert@linux-m68k.or=
g> wrote:
> > > > On Tue, Feb 6, 2024 at 8:46=E2=80=AFAM Fangrui Song <maskray@google=
com> wrote:
> > > > > The generic constraint "i" seems to be copied from x86 or arm (an=
d with
> > > > > a redundant generic operand modifier "c"). It works with -fno-PIE=
 but
> > > > > not with -fPIE/-fPIC in GCC's aarch64 port.
> > > >
> > > > Thanks for your patch, which is now commit f9daab0ad01cf9d1 ("arm64=
:
> > > > jump_label: use constraints "Si" instead of "i"") in v6.8-rc5.
> > > >
> > > > > The machine constraint "S", which denotes a symbol or label refer=
ence
> > > > > with a constant offset, supports PIC and has been available in GC=
C since
> > > > > 2012 and in Clang since 7.0. However, Clang before 19 does not su=
pport
> > > > > "S" on a symbol with a constant offset [1] (e.g.
> > > > > `static_key_false(&nf_hooks_needed[pf][hook])` in
> > > > > include/linux/netfilter.h), so we use "i" as a fallback.
> > > >
> > > > https://gcc.gnu.org/releases.html says gcc-5 was released in 2015,
> > > > i.e. after 2012 ...
> > > >
> > > > > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > Signed-off-by: Fangrui Song <maskray@google.com>
> > > > > Link: https://github.com/llvm/llvm-project/pull/80255 [1]
> > > > >
> > > > > ---
> > > > > Changes from
> > > > > arm64: jump_label: use constraint "S" instead of "i" (https://lor=
e.kernel.org/all/20240131065322.1126831-1-maskray@google.com/)
> > > > >
> > > > > * Use "Si" as Ard suggested to support Clang<19
> > > > > * Make branch a separate operand
> > > > >
> > > > > Changes from v1:
> > > > >
> > > > > * Use asmSymbolicName for readability
> > > >
> > > > But it still fails on gcc-5:
> > > >
> > > >     arch/arm64/include/asm/jump_label.h:25:2: error: invalid 'asm':
> > > > invalid operand
> > > >       asm goto(
> > > >       ^
> > > >
> > > > http://kisskb.ellerman.id.au/kisskb/buildresult/15129281/
> > > >
> > >
> > > How odd. godbolt.org has 5.4 and it seems perfectly happy with it.
>
> > https://godbolt.org/z/GTnf3vPaT
>
> I could reproduce the issue on v6.8-rc5 using arm64 defconfig
> and x86_64-gcc-5.5.0-nolibc-aarch64-linux.tar.xz from
> https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/:
>

OK, I managed to do so as well.

And GCC 6.4 from the same source works correctly.

Not sure whether there are any plans to bump the minimal GCC version
any time soon (cc'ing Arnd), but we should probably drop this change
until that happens.