[PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11

Fri Jul 31 04:16:51 EDT 2020

On 2020-07-31 at 05:58:13 +0200, Palmer Dabbelt <palmer at dabbelt.com> wrote:
> On Tue, 28 Jul 2020 21:44:28 PDT (-0700), tklauser at distanz.ch wrote:
> > Clang before version 11 doesn't support -msave-restore and
> > -mno-save-restore [1].
> > 
> > [1] https://github.com/ClangBuiltLinux/linux/issues/804
> > 
> > This avoids the following message when building with clang 10 and older:
> > 
> >   '-save-restore' is not a recognized feature for this target (ignoring feature)
> > 
> > Signed-off-by: Tobias Klauser <tklauser at distanz.ch>
> > ---
> > v2: use cc-option and check for clang version
> > 
> >  arch/riscv/Makefile | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index fb6e37db836d..30e34946af86 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -44,7 +44,14 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> >  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> >  KBUILD_AFLAGS += -march=$(riscv-march-y)
> > 
> > -KBUILD_CFLAGS += -mno-save-restore
> > +KBUILD_CFLAGS += $(call cc-option,-mno-save-restore)
> > +# Clang versions less than 11 do not support save-restore. See
> > +# https://github.com/ClangBuiltLinux/linux/issues/804
> > +ifeq ($(CONFIG_CC_IS_CLANG), y)
> > +  ifeq ($(shell test $(CONFIG_CLANG_VERSION) -lt 110000; echo $$?),0)
> > +    KBUILD_CFLAGS := $(filter-out -mno-save-restore,$(KBUILD_CFLAGS))
> > +  endif
> > +endif
> >  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
> > 
> >  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
> 
> Ya, that seems worse ;).
> 
> Does this work?  I still get a -mno-save-restore on GCC, but I don't have an
> LLVM lying around to test it with.

Yeah, it should still emit -mno-save-restore on GCC and on Clang >= 11,
which is what it does in my tests here.

It is certainly rather brittle by relying on the specific Clang version
and not very nice to look at, so your solution would certainly be much
nicer. Unfortunately, with cc-option-skipwarn I still get:

  '-save-restore' is not a recognized feature for this target (ignoring feature)

when building with Clang 10 :( I think the issue is that Clang
"somewhat" accepts/handles the option  i.e. doesn't treat it as an
unknown option like others and thus doesn't exit with non-zero, like it
does for other options only available for gcc.

> From 3e3ae8610db0abba9dd6a18e5d6b9bc8c495c2fc Mon Sep 17 00:00:00 2001
> From: Palmer Dabbelt <palmerdabbelt at google.com>
> Date: Thu, 30 Jul 2020 19:41:11 -0700
> Subject: [PATCH 1/2] Kbuild: add cc-option-skipwarn
> 
> The documentation describes the behavior of this function.  In theory we could
> just add -Werror to cc-option, but I don't want to chase around an infinate
> tail of weird build bugs due to compilers that have this odd behavior.
> 
> Signed-off-by: Palmer Dabbelt <palmerdabbelt at google.com>
> ---
> Documentation/kbuild/makefiles.rst | 7 +++++++
> scripts/Kbuild.include             | 6 ++++++
> 2 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
> index 6515ebc12b6f..dba9faf4af29 100644
> --- a/Documentation/kbuild/makefiles.rst
> +++ b/Documentation/kbuild/makefiles.rst
> @@ -507,6 +507,13 @@ more details, with real examples.
> 	cflags-y will be assigned no value if first option is not supported.
> 	Note: cc-option uses KBUILD_CFLAGS for $(CC) options
> 
> +   cc-option-skipwarn
> +	Some compilers emit a warning on unsupported options rather than an
> +	error.  This trips up cc-option, as the test compiler run will succeed
> +	only to spit out the warning for every file.  cc-option-skipwarn allows
> +	the build infastructure to detect compiles that behave this way, but
> +	otherwise acts exactly as cc-option does.
> +
>    cc-option-yn
> 	cc-option-yn is used to check if gcc supports a given option
> 	and return 'y' if supported, otherwise 'n'.
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 9a15fbf66aa1..2efd3a8890c7 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -129,6 +129,12 @@ CC_OPTION_CFLAGS = $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS))
> cc-option = $(call __cc-option, $(CC),\
> 	$(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS),$(1),$(2))
> 
> +# cc-option-skipwarn
> +# Usage: cflags-y += $(call cc-option-skipwarn,-mno-save-restore)
> +
> +cc-option-skipwarn = $(call __cc-option, $(CC),\
> +	$(KBUILD_CPPFLAGS) -Werror $(CC_OPTION_CFLAGS),$(1),$(2))
> +
> # cc-option-yn
> # Usage: flag := $(call cc-option-yn,-march=winchip-c6)
> cc-option-yn = $(call try-run,\
> -- 
> 2.28.0.163.g6104cc2f0b6-goog
> 
> 
> From 8509566a1a3480ea309fd81317d8cd6125b5e294 Mon Sep 17 00:00:00 2001
> From: Tobias Klauser <tklauser at distanz.ch>
> Date: Wed, 29 Jul 2020 06:44:28 +0200
> Subject: [PATCH 2/2] riscv: don't specify -mno-save-restore unless it's
> supported
> 
> Clang before version 11 doesn't support -msave-restore and
> -mno-save-restore [1].
> 
> [1] https://github.com/ClangBuiltLinux/linux/issues/804
> 
> This avoids the following message when building with clang 10 and older:
> 
>  '-save-restore' is not a recognized feature for this target (ignoring feature)
> 
> Signed-off-by: Tobias Klauser <tklauser at distanz.ch>
> [Palmer: Use cc-option-nowarn]
> Signed-off-by: Palmer Dabbelt <palmerdabbelt at google.com>
> ---
> arch/riscv/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index fb6e37db836d..532029c887f1 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -44,7 +44,7 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> KBUILD_AFLAGS += -march=$(riscv-march-y)
> 
> -KBUILD_CFLAGS += -mno-save-restore
> +KBUILD_CFLAGS += $(call cc-option-skipwarn,-mno-save-restore)
> KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
> 
> ifeq ($(CONFIG_CMODEL_MEDLOW),y)
> -- 
> 2.28.0.163.g6104cc2f0b6-goog
> 