diff mbox series

[v3,4/4] ARM: pass -march= only to compiler

Message ID 20220516210954.1660716-5-ndesaulniers@google.com (mailing list archive)
State New, archived
Headers show
Series pass -march= only to compiler | expand

Commit Message

Nick Desaulniers May 16, 2022, 9:09 p.m. UTC
When both -march= and -Wa,-march= are specified for assembler or
assembler-with-cpp sources, GCC and Clang will prefer the -Wa,-march=
value but Clang will warn that -march= is unused.

warning: argument unused during compilation: '-march=armv6k'
[-Wunused-command-line-argument]

This is the top group of warnings we observe when using clang to
assemble the kernel via `ARCH=arm make LLVM=1`.

Split the arch-y make variable into two, so that -march= flags only get
passed to the compiler, not the assembler. -D flags are added to
KBUILD_CPPFLAGS which is used for both C and assembler-with-cpp sources.

Link: https://github.com/ClangBuiltLinux/linux/issues/1315
Link: https://github.com/ClangBuiltLinux/linux/issues/1587
Link: https://lore.kernel.org/llvm/628249e8.1c69fb81.d20fd.02ea@mx.google.com/
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm/Makefile | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

Comments

Nick Desaulniers May 16, 2022, 9:16 p.m. UTC | #1
+ Ard (I messed up my command line invocation of git send-email, sorry
for more noise)
https://lore.kernel.org/llvm/20220516210954.1660716-1-ndesaulniers@google.com/

On Mon, May 16, 2022 at 2:10 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> When both -march= and -Wa,-march= are specified for assembler or
> assembler-with-cpp sources, GCC and Clang will prefer the -Wa,-march=
> value but Clang will warn that -march= is unused.
>
> warning: argument unused during compilation: '-march=armv6k'
> [-Wunused-command-line-argument]
>
> This is the top group of warnings we observe when using clang to
> assemble the kernel via `ARCH=arm make LLVM=1`.
>
> Split the arch-y make variable into two, so that -march= flags only get
> passed to the compiler, not the assembler. -D flags are added to
> KBUILD_CPPFLAGS which is used for both C and assembler-with-cpp sources.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1315
> Link: https://github.com/ClangBuiltLinux/linux/issues/1587
> Link: https://lore.kernel.org/llvm/628249e8.1c69fb81.d20fd.02ea@mx.google.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/arm/Makefile | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 1029c2503aef..47a300f6f99c 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -57,21 +57,34 @@ endif
>  KBUILD_CFLAGS  += $(call cc-option,-fno-ipa-sra)
>
>  # This selects which instruction set is used.
> +arch-$(CONFIG_CPU_32v7M)       :=-march=armv7-m
> +arch-$(CONFIG_CPU_32v7)                :=-march=armv7-a
> +arch-$(CONFIG_CPU_32v6)                :=-march=armv6
> +# Only override the compiler option if ARMv6. The ARMv6K extensions are
> +# always available in ARMv7
> +ifeq ($(CONFIG_CPU_32v6),y)
> +arch-$(CONFIG_CPU_32v6K)       :=-march=armv6k
> +endif
> +arch-$(CONFIG_CPU_32v5)                :=-march=armv5te
> +arch-$(CONFIG_CPU_32v4T)       :=-march=armv4t
> +arch-$(CONFIG_CPU_32v4)                :=-march=armv4
> +arch-$(CONFIG_CPU_32v3)                :=-march=armv3m
> +
>  # Note that GCC does not numerically define an architecture version
>  # macro, but instead defines a whole series of macros which makes
>  # testing for a specific architecture or later rather impossible.
> -arch-$(CONFIG_CPU_32v7M)       :=-D__LINUX_ARM_ARCH__=7 -march=armv7-m
> -arch-$(CONFIG_CPU_32v7)                :=-D__LINUX_ARM_ARCH__=7 -march=armv7-a
> -arch-$(CONFIG_CPU_32v6)                :=-D__LINUX_ARM_ARCH__=6 -march=armv6
> -# Only override the compiler opt:ion if ARMv6. The ARMv6K extensions are
> +cpp-$(CONFIG_CPU_32v7M)                :=-D__LINUX_ARM_ARCH__=7
> +cpp-$(CONFIG_CPU_32v7)         :=-D__LINUX_ARM_ARCH__=7
> +cpp-$(CONFIG_CPU_32v6)         :=-D__LINUX_ARM_ARCH__=6
> +# Only override the compiler option if ARMv6. The ARMv6K extensions are
>  # always available in ARMv7
>  ifeq ($(CONFIG_CPU_32v6),y)
> -arch-$(CONFIG_CPU_32v6K)       :=-D__LINUX_ARM_ARCH__=6 -march=armv6k
> +cpp-$(CONFIG_CPU_32v6K)                :=-D__LINUX_ARM_ARCH__=6
>  endif
> -arch-$(CONFIG_CPU_32v5)                :=-D__LINUX_ARM_ARCH__=5 -march=armv5te
> -arch-$(CONFIG_CPU_32v4T)       :=-D__LINUX_ARM_ARCH__=4 -march=armv4t
> -arch-$(CONFIG_CPU_32v4)                :=-D__LINUX_ARM_ARCH__=4 -march=armv4
> -arch-$(CONFIG_CPU_32v3)                :=-D__LINUX_ARM_ARCH__=3 -march=armv3m
> +cpp-$(CONFIG_CPU_32v5)         :=-D__LINUX_ARM_ARCH__=5
> +cpp-$(CONFIG_CPU_32v4T)                :=-D__LINUX_ARM_ARCH__=4
> +cpp-$(CONFIG_CPU_32v4)         :=-D__LINUX_ARM_ARCH__=4
> +cpp-$(CONFIG_CPU_32v3)         :=-D__LINUX_ARM_ARCH__=3
>
>  # This selects how we optimise for the processor.
>  tune-$(CONFIG_CPU_ARM7TDMI)    :=-mtune=arm7tdmi
> @@ -123,8 +136,9 @@ AFLAGS_ISA  :=$(CFLAGS_ISA)
>  endif
>
>  # Need -Uarm for gcc < 3.x
> +KBUILD_CPPFLAGS        +=$(cpp-y)
>  KBUILD_CFLAGS  +=$(CFLAGS_ABI) $(CFLAGS_ISA) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
> -KBUILD_AFLAGS  +=$(CFLAGS_ABI) $(AFLAGS_ISA) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
> +KBUILD_AFLAGS  +=$(CFLAGS_ABI) $(AFLAGS_ISA) -Wa,$(arch-y) $(tune-y) -include asm/unified.h -msoft-float
>
>  CHECKFLAGS     += -D__arm__
>
> --
> 2.36.0.550.gb090851708-goog
>
kernel test robot May 18, 2022, 1:25 a.m. UTC | #2
Hi Nick,

I love your patch! Yet something to improve:

[auto build test ERROR on 0ac824f379fba2c2b17b75fd5ada69cd68c66348]

url:    https://github.com/intel-lab-lkp/linux/commits/Nick-Desaulniers/pass-march-only-to-compiler/20220517-051756
base:   0ac824f379fba2c2b17b75fd5ada69cd68c66348
config: arm-randconfig-r026-20220516 (https://download.01.org/0day-ci/archive/20220518/202205180917.RNpZaxIl-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/6da98100eed87e4316be5ec584fe415134f25a3e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nick-Desaulniers/pass-march-only-to-compiler/20220517-051756
        git checkout 6da98100eed87e4316be5ec584fe415134f25a3e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arch/arm/crypto/poly1305-core.S:16:1: error: target does not support ARM mode
   .code 32
   ^
Nick Desaulniers May 18, 2022, 10:38 p.m. UTC | #3
Lore thread, for context:
https://lore.kernel.org/llvm/20220516210954.1660716-1-ndesaulniers@google.com/

On Tue, May 17, 2022 at 6:26 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Nick,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on 0ac824f379fba2c2b17b75fd5ada69cd68c66348]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Nick-Desaulniers/pass-march-only-to-compiler/20220517-051756
> base:   0ac824f379fba2c2b17b75fd5ada69cd68c66348
> config: arm-randconfig-r026-20220516 (https://download.01.org/0day-ci/archive/20220518/202205180917.RNpZaxIl-lkp@intel.com/config)

^ looks like this is a THUMB2 build.

> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         # https://github.com/intel-lab-lkp/linux/commit/6da98100eed87e4316be5ec584fe415134f25a3e
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Nick-Desaulniers/pass-march-only-to-compiler/20220517-051756
>         git checkout 6da98100eed87e4316be5ec584fe415134f25a3e
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> >> arch/arm/crypto/poly1305-core.S:16:1: error: target does not support ARM mode
>    .code 32
>    ^

It looks like arch/arm/crypto/poly1305-core.S has a preprocessor guard
for __thumb2__.

My change changes the command line from:

from:
$(CC) ... -march=armv7-m ...
to:
$(CC ... -Wa,-march=armv7-m ...

(where ... contains `-mthumb -Wa,-mthumb` for BOTH).

$ arm-linux-gnueabi-gcc -march=armv7-m -mthumb -dM -E - < /dev/null | grep thumb
#define __thumb2__ 1
#define __thumb__ 1
$ arm-linux-gnueabi-gcc -Wa,-march=armv7-m -mthumb -dM -E - <
/dev/null | grep thumb
#define __thumb__ 1
$ clang --target=arm-linux-gnueabi -march=armv7-m -mthumb -dM -E - <
/dev/null | grep thumb
#define __thumb2__ 1
#define __thumb__ 1
$ clang --target=arm-linux-gnueabi -Wa,-march=armv7-m -mthumb -dM -E -
< /dev/null | grep thumb
#define __thumb__ 1
$

(so it seems that the preprocessor definition of `__thumb2__` is
dependent on `-march=`, not `-Wa,-march=`).

David, we might have a very subtle bug in clang:

$ clang --target=arm-linux-gnueabi -mthumb -Wa,-mthumb -march=armv7-m
-Wa,-march=armv7-m -x assembler-with-cpp
-Wunused-command-line-argument - < /dev/null -dM -E
<prints a bunch of preprocessor defines, but no instance of
-Wunused-command-line-argument>
$ clang --target=arm-linux-gnueabi -mthumb -Wa,-mthumb -march=armv7-m
-Wa,-march=armv7-m -x assembler-with-cpp
-Wunused-command-line-argument - < /dev/null -dM -c
clang-15: warning: argument unused during compilation:
'-march=armv7-m' [-Wunused-command-line-argument]

That seems weird because -c vs -E have different behaviors regarding
-Wunused-command-line-argument, and it seems like for `-x
assembler-with-cpp` that the -march= flag without -Wa, prefix *is*
expected to control the behavior of certain preprocessor defines like
`__thumb2__`.

Perhaps a more straightforward test case:
```asm
@ x.S
@ clang --target=arm-linux-gnueabi -mthumb -Wa,-mthumb -march=armv7-m \
@   -Wa,-march=armv7-m -Wunused-command-line-argument x.S -c
.syntax unified
.text
foo:
  movs r0, #__thumb2__
```

$ clang --target=arm-linux-gnueabi -mthumb -Wa,-mthumb -march=armv7-m
-Wa,-march=armv7-m -Wunused-command-line-argument x.S -c
clang-15: warning: argument unused during compilation:
'-march=armv7-m' [-Wunused-command-line-argument]

$ llvm-objdump -dr x.o | tail -n 2
00000000 <foo>:
       0: 01 20        movs r0, #1

Obviously that warning about `-Wunused-command-line-argument` is
incorrect; the value of `__thumb2__` comes from the preprocessor and
is controlled by `-march=armv7-m`. It can't be simultaneously "unused"
and used to define particular preprocessor directives.

Using the above command line invocation without `-march=armv7-m`
produces the assembler-time failure:
/tmp/x-03ab60.s:10:3: error: unsupported relocation on symbol
  movs r0, #__thumb2__
  ^
because __thumb2__ is not defined.

(perhaps `#ifundef __thumb2__ #error "oops" #endif` would be simpler yet).


>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
>


--
Thanks,
~Nick Desaulniers
Russell King (Oracle) May 19, 2022, 9:38 a.m. UTC | #4
On Wed, May 18, 2022 at 03:38:18PM -0700, Nick Desaulniers wrote:
> (so it seems that the preprocessor definition of `__thumb2__` is
> dependent on `-march=`, not `-Wa,-march=`).

Yes, it will be. -Wa,... are only ever passed to the assembler and do
not affect the preprocessor nor the compiler with GCC.
diff mbox series

Patch

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 1029c2503aef..47a300f6f99c 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -57,21 +57,34 @@  endif
 KBUILD_CFLAGS	+= $(call cc-option,-fno-ipa-sra)
 
 # This selects which instruction set is used.
+arch-$(CONFIG_CPU_32v7M)	:=-march=armv7-m
+arch-$(CONFIG_CPU_32v7)		:=-march=armv7-a
+arch-$(CONFIG_CPU_32v6)		:=-march=armv6
+# Only override the compiler option if ARMv6. The ARMv6K extensions are
+# always available in ARMv7
+ifeq ($(CONFIG_CPU_32v6),y)
+arch-$(CONFIG_CPU_32v6K)	:=-march=armv6k
+endif
+arch-$(CONFIG_CPU_32v5)		:=-march=armv5te
+arch-$(CONFIG_CPU_32v4T)	:=-march=armv4t
+arch-$(CONFIG_CPU_32v4)		:=-march=armv4
+arch-$(CONFIG_CPU_32v3)		:=-march=armv3m
+
 # Note that GCC does not numerically define an architecture version
 # macro, but instead defines a whole series of macros which makes
 # testing for a specific architecture or later rather impossible.
-arch-$(CONFIG_CPU_32v7M)	:=-D__LINUX_ARM_ARCH__=7 -march=armv7-m
-arch-$(CONFIG_CPU_32v7)		:=-D__LINUX_ARM_ARCH__=7 -march=armv7-a
-arch-$(CONFIG_CPU_32v6)		:=-D__LINUX_ARM_ARCH__=6 -march=armv6
-# Only override the compiler opt:ion if ARMv6. The ARMv6K extensions are
+cpp-$(CONFIG_CPU_32v7M)		:=-D__LINUX_ARM_ARCH__=7
+cpp-$(CONFIG_CPU_32v7)		:=-D__LINUX_ARM_ARCH__=7
+cpp-$(CONFIG_CPU_32v6)		:=-D__LINUX_ARM_ARCH__=6
+# Only override the compiler option if ARMv6. The ARMv6K extensions are
 # always available in ARMv7
 ifeq ($(CONFIG_CPU_32v6),y)
-arch-$(CONFIG_CPU_32v6K)	:=-D__LINUX_ARM_ARCH__=6 -march=armv6k
+cpp-$(CONFIG_CPU_32v6K)		:=-D__LINUX_ARM_ARCH__=6
 endif
-arch-$(CONFIG_CPU_32v5)		:=-D__LINUX_ARM_ARCH__=5 -march=armv5te
-arch-$(CONFIG_CPU_32v4T)	:=-D__LINUX_ARM_ARCH__=4 -march=armv4t
-arch-$(CONFIG_CPU_32v4)		:=-D__LINUX_ARM_ARCH__=4 -march=armv4
-arch-$(CONFIG_CPU_32v3)		:=-D__LINUX_ARM_ARCH__=3 -march=armv3m
+cpp-$(CONFIG_CPU_32v5)		:=-D__LINUX_ARM_ARCH__=5
+cpp-$(CONFIG_CPU_32v4T)		:=-D__LINUX_ARM_ARCH__=4
+cpp-$(CONFIG_CPU_32v4)		:=-D__LINUX_ARM_ARCH__=4
+cpp-$(CONFIG_CPU_32v3)		:=-D__LINUX_ARM_ARCH__=3
 
 # This selects how we optimise for the processor.
 tune-$(CONFIG_CPU_ARM7TDMI)	:=-mtune=arm7tdmi
@@ -123,8 +136,9 @@  AFLAGS_ISA	:=$(CFLAGS_ISA)
 endif
 
 # Need -Uarm for gcc < 3.x
+KBUILD_CPPFLAGS	+=$(cpp-y)
 KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(CFLAGS_ISA) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
-KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(AFLAGS_ISA) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
+KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(AFLAGS_ISA) -Wa,$(arch-y) $(tune-y) -include asm/unified.h -msoft-float
 
 CHECKFLAGS	+= -D__arm__