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

From: WANG Xuerui
Date: Sun Jun 25 2023 - 03:48:34 EST


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?

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/