Re: [PATCH v2 7/9] LoongArch: Tweak CFLAGS for Clang compatibility

[Xi Ruoyao]

On Sun, 2023-06-25 at 15:16 +0800, WANG Xuerui wrote:
> On 2023/6/25 10:13, Huacai Chen wrote:
> > Hi, Ruoyao,
> > 
> > On Sun, Jun 25, 2023 at 2:42 AM WANG Xuerui <kernel@xxxxxxxxxx> wrote:
> > > 
> > > From: WANG Xuerui <git@xxxxxxxxxx>
> > > 
> > > Now the arch code is mostly ready for LLVM/Clang consumption, it is time
> > > to re-organize the CFLAGS a little to actually enable the LLVM build.
> > > 
> > > In particular, -mexplicit-relocs and -mdirect-extern-access are not
> > > necessary nor supported on Clang; feature detection via cc-option would
> > > not work, because that way the broken combo of "new GNU as + old GCC"
> > > would seem to get "fixed", but actually produce broken kernels.
> > > Explicitly depending on CONFIG_CC_IS_CLANG is thus necessary to not
> > > regress UX for those building their own kernels.
> > > 
> > > A build with !RELOCATABLE && !MODULE is confirmed working within a QEMU
> > > environment; support for the two features are currently blocked on
> > > LLVM/Clang, and will come later.
> > > 
> > > Signed-off-by: WANG Xuerui <git@xxxxxxxxxx>
> > > ---
> > >   arch/loongarch/Makefile | 6 +++++-
> > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> > > index 366771016b99..82c619791a63 100644
> > > --- a/arch/loongarch/Makefile
> > > +++ b/arch/loongarch/Makefile
> > > @@ -51,7 +51,9 @@ LDFLAGS_vmlinux                       += -static -n -nostdlib
> > > 
> > >   # When the assembler supports explicit relocation hint, we must use it.
> > >   # GCC may have -mexplicit-relocs off by default if it was built with an old
> > > -# assembler, so we force it via an option.
> > > +# assembler, so we force it via an option. For LLVM/Clang the desired behavior
> > > +# is the default, and the flag is not supported, so don't pass it if Clang is
> > > +# being used.
> > >   #
> > >   # When the assembler does not supports explicit relocation hint, we can't use
> > >   # it.  Disable it if the compiler supports it.
> > > @@ -61,8 +63,10 @@ LDFLAGS_vmlinux                      += -static -n -nostdlib
> > >   # combination of a "new" assembler and "old" compiler is not supported.  Either
> > >   # upgrade the compiler or downgrade the assembler.
> > >   ifdef CONFIG_AS_HAS_EXPLICIT_RELOCS
> > > +ifndef CONFIG_CC_IS_CLANG
> > >   cflags-y                       += -mexplicit-relocs
> > >   KBUILD_CFLAGS_KERNEL           += -mdirect-extern-access
> > > +endif
> > I prefer to drop CONFIG_CC_IS_CLANG and use
> > cflags-y                       += $(call cc-option,-mexplicit-relocs)
> > KBUILD_CFLAGS_KERNEL           += $(call cc-option,-mdirect-extern-access)
> > 
> > Then Patch-6 can be merged in this.
> > 
> > What's your opinion?
> 
> FYI: with this approach the build no longer instantly dies with binutils 
> 2.40 + gcc 12.3, but there are also tons of warnings that say the model 
> attribute is being ignored. I checked earlier discussions and this means 
> modules are silently broken at runtime, which is not particularly good UX.

We can add

#if defined(MODULE) && !__has_attribute(model)
#  error some fancy error message
#endif

into percpu.h to error out in this case.  It had been in my earlier
drafts of explicit relocs patches, but we dropped it because there was
no such configuration (unless a snapshot of development GCC is used, and
using such a snapshot is never supported IIUC).


-- 
Xi Ruoyao <xry111@xxxxxxxxxxx>
School of Aerospace Science and Technology, Xidian University