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

[Palmer Dabbelt <palmer@dabbelt.com>]

On Fri, 31 Jul 2020 01:16:51 PDT (-0700), tklauser@distanz.ch wrote:
> On 2020-07-31 at 05:58:13 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> On Tue, 28 Jul 2020 21:44:28 PDT (-0700), tklauser@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@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.

Sorry, I was asking if my patches below work.  Your stuff seems like it'd
pretty obviously work, I just don't like sticking the version in there.  The
patches are below.

>
>> From 3e3ae8610db0abba9dd6a18e5d6b9bc8c495c2fc Mon Sep 17 00:00:00 2001
>> From: Palmer Dabbelt <palmerdabbelt@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@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@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@distanz.ch>
>> [Palmer: Use cc-option-nowarn]
>> Signed-off-by: Palmer Dabbelt <palmerdabbelt@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
>>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv