* [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang @ 2020-07-28 13:12 Tobias Klauser 2020-07-28 15:20 ` Palmer Dabbelt 2020-07-29 4:44 ` [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 Tobias Klauser 0 siblings, 2 replies; 14+ messages in thread From: Tobias Klauser @ 2020-07-28 13:12 UTC (permalink / raw) To: Palmer Dabbelt, Paul Walmsley, Albert Ou; +Cc: linux-riscv, Palmer Dabbelt 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang 2020-07-28 13:12 [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang Tobias Klauser @ 2020-07-28 15:20 ` Palmer Dabbelt 2020-07-28 16:06 ` Tobias Klauser 2020-07-29 4:44 ` [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 Tobias Klauser 1 sibling, 1 reply; 14+ messages in thread From: Palmer Dabbelt @ 2020-07-28 15:20 UTC (permalink / raw) To: tklauser; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang 2020-07-28 15:20 ` Palmer Dabbelt @ 2020-07-28 16:06 ` Tobias Klauser 2020-07-29 4:41 ` Tobias Klauser 0 siblings, 1 reply; 14+ messages in thread From: Tobias Klauser @ 2020-07-28 16:06 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang 2020-07-28 16:06 ` Tobias Klauser @ 2020-07-29 4:41 ` Tobias Klauser 2020-07-29 8:08 ` Jerome Forissier 0 siblings, 1 reply; 14+ messages in thread From: Tobias Klauser @ 2020-07-29 4:41 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang 2020-07-29 4:41 ` Tobias Klauser @ 2020-07-29 8:08 ` Jerome Forissier 2020-07-29 8:29 ` Tobias Klauser 0 siblings, 1 reply; 14+ messages in thread From: Jerome Forissier @ 2020-07-29 8:08 UTC (permalink / raw) To: tklauser; +Cc: jerome, linux-riscv, aou, palmer, paul.walmsley > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang 2020-07-29 8:08 ` Jerome Forissier @ 2020-07-29 8:29 ` Tobias Klauser 2020-07-29 9:36 ` Jerome Forissier 0 siblings, 1 reply; 14+ messages in thread From: Tobias Klauser @ 2020-07-29 8:29 UTC (permalink / raw) To: Jerome Forissier; +Cc: linux-riscv, aou, palmer, paul.walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang 2020-07-29 8:29 ` Tobias Klauser @ 2020-07-29 9:36 ` Jerome Forissier 2020-07-29 9:43 ` Jerome Forissier 0 siblings, 1 reply; 14+ messages in thread From: Jerome Forissier @ 2020-07-29 9:36 UTC (permalink / raw) To: Tobias Klauser; +Cc: linux-riscv, aou, palmer, paul.walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang 2020-07-29 9:36 ` Jerome Forissier @ 2020-07-29 9:43 ` Jerome Forissier 0 siblings, 0 replies; 14+ messages in thread From: Jerome Forissier @ 2020-07-29 9:43 UTC (permalink / raw) To: Tobias Klauser; +Cc: linux-riscv, aou, palmer, paul.walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 2020-07-28 13:12 [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang Tobias Klauser 2020-07-28 15:20 ` Palmer Dabbelt @ 2020-07-29 4:44 ` Tobias Klauser 2020-07-31 3:58 ` Palmer Dabbelt 1 sibling, 1 reply; 14+ messages in thread From: Tobias Klauser @ 2020-07-29 4:44 UTC (permalink / raw) To: Palmer Dabbelt, Paul Walmsley, Albert Ou; +Cc: linux-riscv, Palmer Dabbelt 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 2020-07-29 4:44 ` [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 Tobias Klauser @ 2020-07-31 3:58 ` Palmer Dabbelt 2020-07-31 8:16 ` Tobias Klauser 0 siblings, 1 reply; 14+ messages in thread From: Palmer Dabbelt @ 2020-07-31 3:58 UTC (permalink / raw) To: tklauser; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 2020-07-31 3:58 ` Palmer Dabbelt @ 2020-07-31 8:16 ` Tobias Klauser 2020-07-31 16:16 ` Palmer Dabbelt 0 siblings, 1 reply; 14+ messages in thread From: Tobias Klauser @ 2020-07-31 8:16 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 2020-07-31 8:16 ` Tobias Klauser @ 2020-07-31 16:16 ` Palmer Dabbelt 2020-08-01 10:30 ` Tobias Klauser 0 siblings, 1 reply; 14+ messages in thread From: Palmer Dabbelt @ 2020-07-31 16:16 UTC (permalink / raw) To: tklauser; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 2020-07-31 16:16 ` Palmer Dabbelt @ 2020-08-01 10:30 ` Tobias Klauser 2020-08-03 20:28 ` Palmer Dabbelt 0 siblings, 1 reply; 14+ messages in thread From: Tobias Klauser @ 2020-08-01 10:30 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 2020-08-01 10:30 ` Tobias Klauser @ 2020-08-03 20:28 ` Palmer Dabbelt 0 siblings, 0 replies; 14+ messages in thread From: Palmer Dabbelt @ 2020-08-03 20:28 UTC (permalink / raw) To: tklauser; +Cc: linux-riscv, aou, Paul Walmsley 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-08-03 20:28 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-28 13:12 [PATCH RESEND] riscv: don't specify -mno-save-restore when building with clang Tobias Klauser 2020-07-28 15:20 ` Palmer Dabbelt 2020-07-28 16:06 ` Tobias Klauser 2020-07-29 4:41 ` Tobias Klauser 2020-07-29 8:08 ` Jerome Forissier 2020-07-29 8:29 ` Tobias Klauser 2020-07-29 9:36 ` Jerome Forissier 2020-07-29 9:43 ` Jerome Forissier 2020-07-29 4:44 ` [PATCH v2] riscv: don't specify -mno-save-restore when building with clang < 11 Tobias Klauser 2020-07-31 3:58 ` Palmer Dabbelt 2020-07-31 8:16 ` Tobias Klauser 2020-07-31 16:16 ` Palmer Dabbelt 2020-08-01 10:30 ` Tobias Klauser 2020-08-03 20:28 ` Palmer Dabbelt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).