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

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

[Tobias Klauser]

Clang doesn't support -msave-restore and -mno-save-restore. This avoids
the following message emitted by clang:

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

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
Resent due to infradead.org mailing list issues.

 arch/riscv/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index fb6e37db836d..cd3720bc45e8 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
 KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
 KBUILD_AFLAGS += -march=$(riscv-march-y)
 
+ifndef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += -mno-save-restore
+endif
 KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
 
 ifeq ($(CONFIG_CMODEL_MEDLOW),y)
-- 
2.27.0


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

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

[Palmer Dabbelt]

On Tue, 28 Jul 2020 06:12:52 PDT (-0700), tklauser@distanz.ch wrote:
> Clang doesn't support -msave-restore and -mno-save-restore. This avoids
> the following message emitted by clang:
>
>   '-save-restore' is not a recognized feature for this target (ignoring feature)
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
> Resent due to infradead.org mailing list issues.
>
>  arch/riscv/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index fb6e37db836d..cd3720bc45e8 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
>  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
>  KBUILD_AFLAGS += -march=$(riscv-march-y)
>
> +ifndef CONFIG_CC_IS_CLANG
>  KBUILD_CFLAGS += -mno-save-restore
> +endif
>  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
>
>  ifeq ($(CONFIG_CMODEL_MEDLOW),y)

Thanks, this one didn't make it the first time.  Is there a reason we can't use
cc-option here?  IIRC that's what we use for the other compiler options that
may be unimplemented, and it has the advantage of avoiding encoding specific
compilers into the build system.

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

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

[Tobias Klauser]

On 2020-07-28 at 17:20:45 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> On Tue, 28 Jul 2020 06:12:52 PDT (-0700), tklauser@distanz.ch wrote:
> > Clang doesn't support -msave-restore and -mno-save-restore. This avoids
> > the following message emitted by clang:
> > 
> >   '-save-restore' is not a recognized feature for this target (ignoring feature)
> > 
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> > Resent due to infradead.org mailing list issues.
> > 
> >  arch/riscv/Makefile | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index fb6e37db836d..cd3720bc45e8 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> >  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> >  KBUILD_AFLAGS += -march=$(riscv-march-y)
> > 
> > +ifndef CONFIG_CC_IS_CLANG
> >  KBUILD_CFLAGS += -mno-save-restore
> > +endif
> >  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
> > 
> >  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
> 
> Thanks, this one didn't make it the first time.  Is there a reason we can't use
> cc-option here?  IIRC that's what we use for the other compiler options that
> may be unimplemented, and it has the advantage of avoiding encoding specific
> compilers into the build system.

Thanks for the hint. I don't know how I could've overlooked cc-option.
Will send a v2 using cc-option.

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

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

[Tobias Klauser]

On 2020-07-28 at 18:06:36 +0200, Tobias Klauser <tklauser@distanz.ch> wrote:
> On 2020-07-28 at 17:20:45 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > On Tue, 28 Jul 2020 06:12:52 PDT (-0700), tklauser@distanz.ch wrote:
> > > Clang doesn't support -msave-restore and -mno-save-restore. This avoids
> > > the following message emitted by clang:
> > > 
> > >   '-save-restore' is not a recognized feature for this target (ignoring feature)
> > > 
> > > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > > ---
> > > Resent due to infradead.org mailing list issues.
> > > 
> > >  arch/riscv/Makefile | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > > index fb6e37db836d..cd3720bc45e8 100644
> > > --- a/arch/riscv/Makefile
> > > +++ b/arch/riscv/Makefile
> > > @@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> > >  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> > >  KBUILD_AFLAGS += -march=$(riscv-march-y)
> > > 
> > > +ifndef CONFIG_CC_IS_CLANG
> > >  KBUILD_CFLAGS += -mno-save-restore
> > > +endif
> > >  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
> > > 
> > >  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
> > 
> > Thanks, this one didn't make it the first time.  Is there a reason we can't use
> > cc-option here?  IIRC that's what we use for the other compiler options that
> > may be unimplemented, and it has the advantage of avoiding encoding specific
> > compilers into the build system.
> 
> Thanks for the hint. I don't know how I could've overlooked cc-option.
> Will send a v2 using cc-option.

Looks like it's a bit more complicated:

Using just cc-option still leads to the warning being emitted, so I
think the CONFIG_CC_IS_CLANG check is still needed (checked using clang
10). Also, it looks like clang 11 and newer support -m{no,}save-restore
[1], so we would want to keep it for these versions. Will send a v1
shortly with a proper clang version check. Also see [2].

[1] https://github.com/llvm/llvm-project/commit/07f7c00208b393296f8f27d6cd3cec2b11d86fd8
[2] https://github.com/ClangBuiltLinux/linux/issues/804

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

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

[Tobias Klauser]

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)
-- 
2.27.0


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

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

[Jerome Forissier]

> On 2020-07-28 at 18:06:36 +0200, Tobias Klauser <tklauser@distanz.ch> wrote:
> > On 2020-07-28 at 17:20:45 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > On Tue, 28 Jul 2020 06:12:52 PDT (-0700), tklauser@distanz.ch wrote:
> > > > Clang doesn't support -msave-restore and -mno-save-restore. This avoids
> > > > the following message emitted by clang:
> > > > 
> > > >   '-save-restore' is not a recognized feature for this target (ignoring feature)
> > > > 
> > > > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > > > ---
> > > > Resent due to infradead.org mailing list issues.
> > > > 
> > > >  arch/riscv/Makefile | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > > > index fb6e37db836d..cd3720bc45e8 100644
> > > > --- a/arch/riscv/Makefile
> > > > +++ b/arch/riscv/Makefile
> > > > @@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> > > >  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> > > >  KBUILD_AFLAGS += -march=$(riscv-march-y)
> > > > 
> > > > +ifndef CONFIG_CC_IS_CLANG
> > > >  KBUILD_CFLAGS += -mno-save-restore
> > > > +endif
> > > >  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
> > > > 
> > > >  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
> > > 
> > > Thanks, this one didn't make it the first time.  Is there a reason we can't use
> > > cc-option here?  IIRC that's what we use for the other compiler options that
> > > may be unimplemented, and it has the advantage of avoiding encoding specific
> > > compilers into the build system.
> > 
> > Thanks for the hint. I don't know how I could've overlooked cc-option.
> > Will send a v2 using cc-option.
> 
> Looks like it's a bit more complicated:
> 
> Using just cc-option still leads to the warning being emitted, so I
> think the CONFIG_CC_IS_CLANG check is still needed (checked using clang

Shouldnt cc-option be fixed instead? The warning effectively means the option
is not supported, so why should cc-option pretend it is?

-- 
Jerome

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

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

[Tobias Klauser]

On 2020-07-29 at 10:08:11 +0200, Jerome Forissier <jerome@forissier.org> wrote:
> > On 2020-07-28 at 18:06:36 +0200, Tobias Klauser <tklauser@distanz.ch> wrote:
> > > On 2020-07-28 at 17:20:45 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > > On Tue, 28 Jul 2020 06:12:52 PDT (-0700), tklauser@distanz.ch wrote:
> > > > > Clang doesn't support -msave-restore and -mno-save-restore. This avoids
> > > > > the following message emitted by clang:
> > > > > 
> > > > >   '-save-restore' is not a recognized feature for this target (ignoring feature)
> > > > > 
> > > > > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > > > > ---
> > > > > Resent due to infradead.org mailing list issues.
> > > > > 
> > > > >  arch/riscv/Makefile | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > > > > index fb6e37db836d..cd3720bc45e8 100644
> > > > > --- a/arch/riscv/Makefile
> > > > > +++ b/arch/riscv/Makefile
> > > > > @@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> > > > >  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
> > > > >  KBUILD_AFLAGS += -march=$(riscv-march-y)
> > > > > 
> > > > > +ifndef CONFIG_CC_IS_CLANG
> > > > >  KBUILD_CFLAGS += -mno-save-restore
> > > > > +endif
> > > > >  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
> > > > > 
> > > > >  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
> > > > 
> > > > Thanks, this one didn't make it the first time.  Is there a reason we can't use
> > > > cc-option here?  IIRC that's what we use for the other compiler options that
> > > > may be unimplemented, and it has the advantage of avoiding encoding specific
> > > > compilers into the build system.
> > > 
> > > Thanks for the hint. I don't know how I could've overlooked cc-option.
> > > Will send a v2 using cc-option.
> > 
> > Looks like it's a bit more complicated:
> > 
> > Using just cc-option still leads to the warning being emitted, so I
> > think the CONFIG_CC_IS_CLANG check is still needed (checked using clang
> 
> Shouldnt cc-option be fixed instead? The warning effectively means the option
> is not supported, so why should cc-option pretend it is?

The problem is that clang somewhat 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. This is
what cc-option relies on.

Like mentioned in the follow-up patch [1] clang 11 will handle
-m{no,}save-restore correctly, so we need to address that case as well
(which [1] does). Unfortunately, I currently see no other way than using
these ifeq's to fix this.

[1] https://lore.kernel.org/linux-riscv/20200729044428.32460-1-tklauser@distanz.ch/T/#u

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

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

[Jerome Forissier]

On 7/29/20 10:29 AM, Tobias Klauser wrote:
> On 2020-07-29 at 10:08:11 +0200, Jerome Forissier <jerome@forissier.org> wrote:
>>> On 2020-07-28 at 18:06:36 +0200, Tobias Klauser <tklauser@distanz.ch> wrote:
>>>> On 2020-07-28 at 17:20:45 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>>>> On Tue, 28 Jul 2020 06:12:52 PDT (-0700), tklauser@distanz.ch wrote:
>>>>>> Clang doesn't support -msave-restore and -mno-save-restore. This avoids
>>>>>> the following message emitted by clang:
>>>>>>
>>>>>>   '-save-restore' is not a recognized feature for this target (ignoring feature)
>>>>>>
>>>>>> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
>>>>>> ---
>>>>>> Resent due to infradead.org mailing list issues.
>>>>>>
>>>>>>  arch/riscv/Makefile | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
>>>>>> index fb6e37db836d..cd3720bc45e8 100644
>>>>>> --- a/arch/riscv/Makefile
>>>>>> +++ b/arch/riscv/Makefile
>>>>>> @@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
>>>>>>  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
>>>>>>  KBUILD_AFLAGS += -march=$(riscv-march-y)
>>>>>>
>>>>>> +ifndef CONFIG_CC_IS_CLANG
>>>>>>  KBUILD_CFLAGS += -mno-save-restore
>>>>>> +endif
>>>>>>  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
>>>>>>
>>>>>>  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
>>>>>
>>>>> Thanks, this one didn't make it the first time.  Is there a reason we can't use
>>>>> cc-option here?  IIRC that's what we use for the other compiler options that
>>>>> may be unimplemented, and it has the advantage of avoiding encoding specific
>>>>> compilers into the build system.
>>>>
>>>> Thanks for the hint. I don't know how I could've overlooked cc-option.
>>>> Will send a v2 using cc-option.
>>>
>>> Looks like it's a bit more complicated:
>>>
>>> Using just cc-option still leads to the warning being emitted, so I
>>> think the CONFIG_CC_IS_CLANG check is still needed (checked using clang
>>
>> Shouldnt cc-option be fixed instead? The warning effectively means the option
>> is not supported, so why should cc-option pretend it is?
> 
> The problem is that clang somewhat 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. This is
> what cc-option relies on.

OK, I was thinking perhaps cc-option should consider a warning as a hint
that the option should not be used. But it is probably not a good idea
after all (could easily break things).

However this makes me think the following could work (untested!):

-KBUILD_CFLAGS += -mno-save-restore
+KBUILD_CFLAGS += $(call cc-option,-mno-save-restore -Werror)

> 
> Like mentioned in the follow-up patch [1] clang 11 will handle
> -m{no,}save-restore correctly, so we need to address that case as well
> (which [1] does). Unfortunately, I currently see no other way than using
> these ifeq's to fix this.
> 
> [1] https://lore.kernel.org/linux-riscv/20200729044428.32460-1-tklauser@distanz.ch/T/#u

The -Werror technique would avoid introducing a test on the version number.

-- 
Jerome

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

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

[Jerome Forissier]

On 7/29/20 11:36 AM, Jerome Forissier wrote:
> 
> 
> On 7/29/20 10:29 AM, Tobias Klauser wrote:
>> On 2020-07-29 at 10:08:11 +0200, Jerome Forissier <jerome@forissier.org> wrote:
>>>> On 2020-07-28 at 18:06:36 +0200, Tobias Klauser <tklauser@distanz.ch> wrote:
>>>>> On 2020-07-28 at 17:20:45 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>>>>> On Tue, 28 Jul 2020 06:12:52 PDT (-0700), tklauser@distanz.ch wrote:
>>>>>>> Clang doesn't support -msave-restore and -mno-save-restore. This avoids
>>>>>>> the following message emitted by clang:
>>>>>>>
>>>>>>>   '-save-restore' is not a recognized feature for this target (ignoring feature)
>>>>>>>
>>>>>>> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
>>>>>>> ---
>>>>>>> Resent due to infradead.org mailing list issues.
>>>>>>>
>>>>>>>  arch/riscv/Makefile | 2 ++
>>>>>>>  1 file changed, 2 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
>>>>>>> index fb6e37db836d..cd3720bc45e8 100644
>>>>>>> --- a/arch/riscv/Makefile
>>>>>>> +++ b/arch/riscv/Makefile
>>>>>>> @@ -44,7 +44,9 @@ riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
>>>>>>>  KBUILD_CFLAGS += -march=$(subst fd,,$(riscv-march-y))
>>>>>>>  KBUILD_AFLAGS += -march=$(riscv-march-y)
>>>>>>>
>>>>>>> +ifndef CONFIG_CC_IS_CLANG
>>>>>>>  KBUILD_CFLAGS += -mno-save-restore
>>>>>>> +endif
>>>>>>>  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
>>>>>>>
>>>>>>>  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
>>>>>>
>>>>>> Thanks, this one didn't make it the first time.  Is there a reason we can't use
>>>>>> cc-option here?  IIRC that's what we use for the other compiler options that
>>>>>> may be unimplemented, and it has the advantage of avoiding encoding specific
>>>>>> compilers into the build system.
>>>>>
>>>>> Thanks for the hint. I don't know how I could've overlooked cc-option.
>>>>> Will send a v2 using cc-option.
>>>>
>>>> Looks like it's a bit more complicated:
>>>>
>>>> Using just cc-option still leads to the warning being emitted, so I
>>>> think the CONFIG_CC_IS_CLANG check is still needed (checked using clang
>>>
>>> Shouldnt cc-option be fixed instead? The warning effectively means the option
>>> is not supported, so why should cc-option pretend it is?
>>
>> The problem is that clang somewhat 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. This is
>> what cc-option relies on.
> 
> OK, I was thinking perhaps cc-option should consider a warning as a hint
> that the option should not be used. But it is probably not a good idea
> after all (could easily break things).
> 
> However this makes me think the following could work (untested!):
> 
> -KBUILD_CFLAGS += -mno-save-restore
> +KBUILD_CFLAGS += $(call cc-option,-mno-save-restore -Werror)

Sorry, please ignore this nonsense. We don't want -Werror to stick in
KBUILD_CFLAGS obviously.

:-/

-- 
Jerome

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

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

[Palmer Dabbelt]

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.

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

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

[Tobias Klauser]

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.

> 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

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

[Palmer Dabbelt]

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

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

[Tobias Klauser]

On 2020-07-31 at 18:16:28 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> 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.

Sorry, I should have been more explicit in my message. I was testing
with cc-option-skipwarn with your two patches applied already. But
unfortunately, the warning still appeared when building with Clang 10
:-(

The only way I can currently think of to detect this case using a
cc-option helper would be to somehow parse clang's stdout/stderr for the
message which IMO would be equally brittle as making the flag version
dependent :-(

> > 
> > > 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

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

[Palmer Dabbelt]

On Sat, 01 Aug 2020 03:30:17 PDT (-0700), tklauser@distanz.ch wrote:
> On 2020-07-31 at 18:16:28 +0200, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> 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.
>
> Sorry, I should have been more explicit in my message. I was testing
> with cc-option-skipwarn with your two patches applied already. But
> unfortunately, the warning still appeared when building with Clang 10
> :-(

Bummer.

> The only way I can currently think of to detect this case using a
> cc-option helper would be to somehow parse clang's stdout/stderr for the
> message which IMO would be equally brittle as making the flag version
> dependent :-(

Ya, that's worse.  I'm going to take a poke at clang and see if I can get
something cleaner to work, otherwise I'll just take your patch.  I should
probably add clang-based builds to my tests anyway.

Thanks!

>
>> >
>> > > 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