Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"

From: Ard Biesheuvel <ardb@kernel.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
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
Subject: Re: [PATCH v2] arm64: jump_label: use constraints "Si" instead of "i"
Date: Mon, 19 Feb 2024 11:57:04 +0100	[thread overview]
Message-ID: <CAMj1kXGw+r7yEEBA8gYBcdrqkiP=VYOSzz9YLnNavJn2snmFwA@mail.gmail.com> (raw)
In-Reply-To: <CAMj1kXGaF5bobHTr1pTg+-=s4Ft7+5SSbX72-NxsR_W_Yuxb8Q@mail.gmail.com>

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.org> wrote:
> >
> > Hi Fangrui,
> >
> > On Tue, Feb 6, 2024 at 8:46 AM Fangrui Song <maskray@google.com> wrote:
> > > The generic constraint "i" seems to be copied from x86 or arm (and 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 reference
> > > with a constant offset, supports PIC and has been available in GCC since
> > > 2012 and in Clang since 7.0. However, Clang before 19 does not support
> > > "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://lore.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/szzG3s59K

Wrong link

https://godbolt.org/z/GTnf3vPaT