Re: [PATCH v2 7/9] LoongArch: Tweak CFLAGS for Clang compatibility
From: Huacai Chen
Date: Sun Jun 25 2023 - 04:13:55 EST
On Sun, Jun 25, 2023 at 3:48 PM WANG Xuerui <kernel@xxxxxxxxxx> wrote:
>
> On 2023/6/25 15:36, Xi Ruoyao wrote:
> > 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).
>
> Ah I've seen that. So in this case we simply wrap -mexplicit-relocs with
> cc-option and error out in case of CONFIG_MODULE but no model attribute,
> which nicely prevents broken configurations (MODULE && ((old_gcc &&
> new_binutils) || clang)) with feature detection alone.
>
> This seems elegant and better to me; Huacai, WDYT?
OK, perfect.
Huacai
>
> --
> WANG "xen0n" Xuerui
>
> Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
>