argument unused during compilation: '-march=armv6k'

[nickdesaulniers]

forked from #1195

Building 32b arm targets with LLVM_IAS=1 with clang-13 (after https://reviews.llvm.org/rG1d51c699b9e2ebc5bcfdbe85c74cc871426333d4) now produces warnings related to the -march= flags.

The kernel uses the compiler as the driver for assembler input, as it generally is assembler with c preprocessor .S sources.

It seems that the 32b ARM kernel sometimes specifies -march=foo -Wa,-march=foo (ie. tell the compiler and the assembler which arch we're targeting). Based on the code in LLVM (see test cases in https://reviews.llvm.org/rG1d51c699b9e2ebc5bcfdbe85c74cc871426333d4), it seems the compiler argument is redundant.

I assume that the kernel/Kbuild should not be setting -march= for KBUILD_AFLAGS, only -Wa,-march= (so tentatively marking this a bug on the kernel side).

Though, I'm also curious whether we're handing -Qunused-arguments correctly for assembler input, or -Wa,-W correctly. (I haven't checked yet whether either of those are currently set for assembler input. I thought they were but could be misremembering).

cc @arndb @DavidSpickett

[arndb]

I assume that the kernel/Kbuild should not be setting -march= for KBUILD_AFLAGS, only -Wa,-march= (so tentatively marking this a bug on the kernel side).

I think your earlier patch to move all the -Wa,-march= flags into .arch directives in the assembler files is still the best fix here.

Though, I'm also curious whether we're handing -Qunused-arguments correctly for assembler input, or -Wa,-W correctly. (I haven't checked yet whether either of those are currently set for assembler input. I thought they were but could be misremembering).

I don't see any reference to -Qunused-arguments in the Makefile, except for the arm64 vdso32 code, which is probably a mistake. I have sent patches to remove unused arguments in the past, to avoid these warnings.

[nickdesaulniers]

@arndb is carrying an improved version of a patch I wrote for this awhile ago.
https://lore.kernel.org/linux-arm-kernel/20210928154143.2106903-14-arnd@kernel.org/

[nickdesaulniers]

I sent out https://lore.kernel.org/llvm/20220516210954.1660716-1-ndesaulniers@google.com/ for this.

[nickdesaulniers]

This might just be a bug in clang. cc @DavidSpickett

[DavidSpickett]

I'm going to look into this today.

[nickdesaulniers]

@DavidSpickett I'm guessing the warning makes sense for -x assembler, but not -x assembler-with-cpp.

[nickdesaulniers]

I don't think that not warning is the way to go.

I don't think that supporting different values for -march=<foo> -Wa,-march=<foo> is worth it and the kernel doesn't need support for different values between the two though that is a subtle behavior that gcc does support.

@nathanchance sugguested maybe another go of https://lore.kernel.org/llvm/20220516210954.1660716-1-ndesaulniers@google.com/ but rather than just remove -march= we maybe set KBUILD_AFLAGS += -D__thumb2__=2 when CONFIG_THUMB2_KERNEL=y. (2 to avoid unexpected redefinition)

[nathanchance]

I think this is all that should be needed:

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index c5fd0bcf1b5b..b58998749ead 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -132,7 +132,7 @@ AFLAGS_NOWARN       :=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)

 ifeq ($(CONFIG_THUMB2_KERNEL),y)
 CFLAGS_ISA     :=-mthumb -Wa,-mimplicit-it=always $(AFLAGS_NOWARN)
-AFLAGS_ISA     :=$(CFLAGS_ISA) -Wa$(comma)-mthumb
+AFLAGS_ISA     :=$(CFLAGS_ISA) -Wa$(comma)-mthumb -D__thumb2__=2
 else
 CFLAGS_ISA     :=$(call cc-option,-marm,) $(AFLAGS_NOWARN)
 AFLAGS_ISA     :=$(CFLAGS_ISA)

With that, the error that the kernel test robot reported disappears. I do see another set of errors with allmodconfig along the lines of:

../arch/arm/mach-tegra/sleep.S:35:2: error: instruction requires: data-barriers
 dmb @ ensure ordering
 ^
<instantiation>:2:2: error: invalid instruction, any one of the following would fix this:
 ubfx r4, r4, #4, #12
 ^
<instantiation>:6:2: note: while in macro instantiation
 check_cpu_part_num 0xc09, r4, r5
 ^
../arch/arm/mach-tegra/sleep.S:50:2: note: while in macro instantiation
 exit_smp r4, r5
 ^
<instantiation>:2:2: note: instruction requires: armv6t2
 ubfx r4, r4, #4, #12
 ^
<instantiation>:6:2: note: while in macro instantiation
 check_cpu_part_num 0xc09, r4, r5
 ^
../arch/arm/mach-tegra/sleep.S:50:2: note: while in macro instantiation
 exit_smp r4, r5
 ^
<instantiation>:2:2: note: instruction requires: thumb2
 ubfx r4, r4, #4, #12
 ^
<instantiation>:6:2: note: while in macro instantiation
 check_cpu_part_num 0xc09, r4, r5
 ^
../arch/arm/mach-tegra/sleep.S:50:2: note: while in macro instantiation
 exit_smp r4, r5
 ^
...

This is resolved with:

diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
index 8f88944831c5..945f2c1474f7 100644
--- a/arch/arm/mach-tegra/sleep.S
+++ b/arch/arm/mach-tegra/sleep.S
@@ -22,6 +22,8 @@
 #define CLK_RESET_CCLK_BURST   0x20
 #define CLK_RESET_CCLK_DIVIDER  0x24

+.arch armv7-a
+
 #if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PM_SLEEP)
 /*
  * tegra_disable_clean_inv_dcache

The series still applies to -next with:

$ b4 am -o - 20220516210954.1660716-1-ndesaulniers@google.com | git am -3

[nickdesaulniers]

I think this is all that should be needed:

Will GCC warn if you set via command line -D flag a flag its preprocessor would have defined?

[nathanchance]

I think this is all that should be needed:

Will GCC warn if you set via command line -D flag a flag its preprocessor would have defined?

Yes:

$ echo "int main(void) { return __thumb__; }" | arm-none-eabi-gcc -mthumb -Werror -x c -c -o /dev/null -

$ echo "int main(void) { return __thumb__; }" | arm-none-eabi-gcc -mthumb -D__thumb__=2 -Werror -x c -c -o /dev/null -
<command-line>: error: "__thumb__" redefined [-Werror]
<built-in>: note: this is the location of the previous definition
cc1: all warnings being treated as errors

[nickdesaulniers]

v4: https://lore.kernel.org/llvm/20221014201354.3190007-1-ndesaulniers@google.com/

[nickdesaulniers]

I've submitted v4 to rmk's queue:

1. https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9262/1
2. https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9263/1
3. https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9264/1
4. https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9265/1

[nathanchance]

Russell appears to have picked up 9262/1, 9264/1, and 9265/1, but not 9263/1, which seems highly problematic to me, as the final patch heavily relies on the second one, so I suspect ARM builds in -next are about to break heavily :/

[nathanchance]

I've pinged Russell on the v4 thread: https://lore.kernel.org/Y2qgTyFcPdnNfkpj@dev-arch.thelio-3990X/

[nathanchance]

Merged into mainline: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=1d2e9b67b001f8c0d10138797b9df4779609c03c