diff mbox series

[v3,1/2] sparc/build: Put usage of -fcall-used* flags behind cc-option

Message ID 20241029-sparc-cflags-v3-1-b28745a6bd71@protonmail.com (mailing list archive)
State New
Headers show
Series sparc/build: Rework CFLAGS for clang compatibility | expand

Commit Message

Koakuma via B4 Relay Oct. 29, 2024, 2:49 p.m. UTC 
From: Koakuma <koachan@protonmail.com>

Place -fcall-used* flags behind cc-option so that clang (which doesn't
support them) can still compile the kernel.

This is a safe change, the reasoning is as follows:

In the (normal) 32-bit ABI, %g5 and %g7 is normally reserved, and in
the 64-bit ABI, %g7 is the reserved one.
Linux turns them into volatile registers by the way of -fcall-used-*,
but on the other hand, omitting the flags shouldn't be harmful;
compilers will now simply refuse to touch them, and any assembly
code that happens to touch them would still work like usual (because
Linux' conventions already treats them as volatile anyway).

Signed-off-by: Koakuma <koachan@protonmail.com>
---
 arch/sparc/Makefile      | 4 ++--
 arch/sparc/vdso/Makefile | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Nathan Chancellor Oct. 29, 2024, 10:24 p.m. UTC | #1 
On Tue, Oct 29, 2024 at 09:49:07PM +0700, Koakuma via B4 Relay wrote:
> From: Koakuma <koachan@protonmail.com>
> 
> Place -fcall-used* flags behind cc-option so that clang (which doesn't
> support them) can still compile the kernel.
> 
> This is a safe change, the reasoning is as follows:
> 
> In the (normal) 32-bit ABI, %g5 and %g7 is normally reserved, and in
> the 64-bit ABI, %g7 is the reserved one.
> Linux turns them into volatile registers by the way of -fcall-used-*,
> but on the other hand, omitting the flags shouldn't be harmful;
> compilers will now simply refuse to touch them, and any assembly
> code that happens to touch them would still work like usual (because
> Linux' conventions already treats them as volatile anyway).
> 
> Signed-off-by: Koakuma <koachan@protonmail.com>

Clang builds now succeed with this series and builds with GCC 14.2.0
continue to pass and boot successfully.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>

One comment below, please carry these tags forward if there are future
revisions without substantial technical changes.

> ---
>  arch/sparc/Makefile      | 4 ++--
>  arch/sparc/vdso/Makefile | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile
> index 757451c3ea1df63b948e68a45988c78f5974f9ea..0400078076e588be93a702d1c64eb9fd34466075 100644
> --- a/arch/sparc/Makefile
> +++ b/arch/sparc/Makefile
> @@ -29,7 +29,7 @@ UTS_MACHINE    := sparc
>  # versions of gcc.  Some gcc versions won't pass -Av8 to binutils when you
>  # give -mcpu=v8.  This silently worked with older bintutils versions but
>  # does not any more.
> -KBUILD_CFLAGS  += -m32 -mcpu=v8 -pipe -mno-fpu -fcall-used-g5 -fcall-used-g7
> +KBUILD_CFLAGS  += -m32 -mcpu=v8 -pipe -mno-fpu $(call cc-option,-fcall-used-g5) $(call cc-option,-fcall-used-g7)

Small nit, this (and the one in the vdso) could probably be one
cc-option call? Is it likely that one flag would be implemented in the
compiler without the other?

  $(call cc-option,-fcall-used-g5 -fcall-used-g7)

>  KBUILD_CFLAGS  += -Wa,-Av8
>  
>  KBUILD_AFLAGS  += -m32 -Wa,-Av8
> @@ -45,7 +45,7 @@ export BITS   := 64
>  UTS_MACHINE   := sparc64
>  
>  KBUILD_CFLAGS += -m64 -pipe -mno-fpu -mcpu=ultrasparc -mcmodel=medlow
> -KBUILD_CFLAGS += -ffixed-g4 -ffixed-g5 -fcall-used-g7 -Wno-sign-compare
> +KBUILD_CFLAGS += -ffixed-g4 -ffixed-g5 $(call cc-option,-fcall-used-g7) -Wno-sign-compare
>  KBUILD_CFLAGS += -Wa,--undeclared-regs
>  KBUILD_CFLAGS += $(call cc-option,-mtune=ultrasparc3)
>  KBUILD_AFLAGS += -m64 -mcpu=ultrasparc -Wa,--undeclared-regs
> diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile
> index 243dbfc4609d804fc221c3591eebe891107ffdab..50ec2978cda5397841daad6ffdc9682811b9b38e 100644
> --- a/arch/sparc/vdso/Makefile
> +++ b/arch/sparc/vdso/Makefile
> @@ -46,7 +46,7 @@ CFL := $(PROFILING) -mcmodel=medlow -fPIC -O2 -fasynchronous-unwind-tables -m64
>         -fno-omit-frame-pointer -foptimize-sibling-calls \
>         -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
>  
> -SPARC_REG_CFLAGS = -ffixed-g4 -ffixed-g5 -fcall-used-g5 -fcall-used-g7
> +SPARC_REG_CFLAGS = -ffixed-g4 -ffixed-g5 $(call cc-option,-fcall-used-g5) $(call cc-option,-fcall-used-g7)
>  
>  $(vobjs): KBUILD_CFLAGS := $(filter-out $(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) $(SPARC_REG_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)
>  
> 
> -- 
> 2.47.0
> 
> 
>
Koakuma Nov. 7, 2024, 4:59 a.m. UTC | #2 
Nathan Chancellor <nathan@kernel.org> wrote:
> 
> Clang builds now succeed with this series and builds with GCC 14.2.0
> continue to pass and boot successfully.
> 
> Reviewed-by: Nathan Chancellor nathan@kernel.org
> 
> Tested-by: Nathan Chancellor nathan@kernel.org
> 
> One comment below, please carry these tags forward if there are future
> revisions without substantial technical changes.

Forgive me for still being unfamiliar with the term, but does this mean that
when I send a v4 I should paste the Reviewed-by and Tested-by lines into the
commit message of the patch?

> > -KBUILD_CFLAGS += -m32 -mcpu=v8 -pipe -mno-fpu -fcall-used-g5 -fcall-used-g7
> > +KBUILD_CFLAGS += -m32 -mcpu=v8 -pipe -mno-fpu $(call cc-option,-fcall-used-g5) $(call cc-option,-fcall-used-g7)
> 
> 
> Small nit, this (and the one in the vdso) could probably be one
> cc-option call? Is it likely that one flag would be implemented in the
> compiler without the other?
> 
> $(call cc-option,-fcall-used-g5 -fcall-used-g7)

Ah, didn't know it's possible to do that, the other uses of it I see seem
to use one flag per call. I'll test and send a new revision, thanks.
Nathan Chancellor Nov. 7, 2024, 4:01 p.m. UTC | #3 
On Thu, Nov 07, 2024 at 04:59:14AM +0000, Koakuma wrote:
> Nathan Chancellor <nathan@kernel.org> wrote:
> > 
> > Clang builds now succeed with this series and builds with GCC 14.2.0
> > continue to pass and boot successfully.
> > 
> > Reviewed-by: Nathan Chancellor nathan@kernel.org
> > 
> > Tested-by: Nathan Chancellor nathan@kernel.org
> > 
> > One comment below, please carry these tags forward if there are future
> > revisions without substantial technical changes.
> 
> Forgive me for still being unfamiliar with the term, but does this mean that

No worries, it is definitely a customary thing.

> when I send a v4 I should paste the Reviewed-by and Tested-by lines into the
> commit message of the patch?

Yes, you should add them either right above or right below your signoff.
It is up to the submitter to add tags that have been sent on prior
revisions when sending an updated version, assuming that there has not
been a reason to drop them, such as substantial changes from a prior
version that might require a new review or testing. In that case, I
typically add a note in the changelog as to why I did not carry them
forward.

The tip documentation 4.2.3 through 4.2.6 has some good information
about some other Linux kernel commit message expectations if you find
yourself submitting more patches in the future:

https://docs.kernel.org/process/maintainer-tip.html#changelog

> > > -KBUILD_CFLAGS += -m32 -mcpu=v8 -pipe -mno-fpu -fcall-used-g5 -fcall-used-g7
> > > +KBUILD_CFLAGS += -m32 -mcpu=v8 -pipe -mno-fpu $(call cc-option,-fcall-used-g5) $(call cc-option,-fcall-used-g7)
> > 
> > 
> > Small nit, this (and the one in the vdso) could probably be one
> > cc-option call? Is it likely that one flag would be implemented in the
> > compiler without the other?
> > 
> > $(call cc-option,-fcall-used-g5 -fcall-used-g7)
> 
> Ah, didn't know it's possible to do that, the other uses of it I see seem
> to use one flag per call. I'll test and send a new revision, thanks.

Yeah, I would agree that it is typical to use cc-option for one flag at
a time but the entire string just gets passed to $(CC), so there is
technically no limitation for how many flags can be tested. This happens
to be a rare instance where the flags share a common internal compiler
implementation so we know that one cannot be implemented without the
other.

Cheers,
Nathan
diff mbox series

Patch

diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile
index 757451c3ea1df63b948e68a45988c78f5974f9ea..0400078076e588be93a702d1c64eb9fd34466075 100644
--- a/arch/sparc/Makefile
+++ b/arch/sparc/Makefile
@@ -29,7 +29,7 @@  UTS_MACHINE    := sparc
 # versions of gcc.  Some gcc versions won't pass -Av8 to binutils when you
 # give -mcpu=v8.  This silently worked with older bintutils versions but
 # does not any more.
-KBUILD_CFLAGS  += -m32 -mcpu=v8 -pipe -mno-fpu -fcall-used-g5 -fcall-used-g7
+KBUILD_CFLAGS  += -m32 -mcpu=v8 -pipe -mno-fpu $(call cc-option,-fcall-used-g5) $(call cc-option,-fcall-used-g7)
 KBUILD_CFLAGS  += -Wa,-Av8
 
 KBUILD_AFLAGS  += -m32 -Wa,-Av8
@@ -45,7 +45,7 @@  export BITS   := 64
 UTS_MACHINE   := sparc64
 
 KBUILD_CFLAGS += -m64 -pipe -mno-fpu -mcpu=ultrasparc -mcmodel=medlow
-KBUILD_CFLAGS += -ffixed-g4 -ffixed-g5 -fcall-used-g7 -Wno-sign-compare
+KBUILD_CFLAGS += -ffixed-g4 -ffixed-g5 $(call cc-option,-fcall-used-g7) -Wno-sign-compare
 KBUILD_CFLAGS += -Wa,--undeclared-regs
 KBUILD_CFLAGS += $(call cc-option,-mtune=ultrasparc3)
 KBUILD_AFLAGS += -m64 -mcpu=ultrasparc -Wa,--undeclared-regs
diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile
index 243dbfc4609d804fc221c3591eebe891107ffdab..50ec2978cda5397841daad6ffdc9682811b9b38e 100644
--- a/arch/sparc/vdso/Makefile
+++ b/arch/sparc/vdso/Makefile
@@ -46,7 +46,7 @@  CFL := $(PROFILING) -mcmodel=medlow -fPIC -O2 -fasynchronous-unwind-tables -m64
        -fno-omit-frame-pointer -foptimize-sibling-calls \
        -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
 
-SPARC_REG_CFLAGS = -ffixed-g4 -ffixed-g5 -fcall-used-g5 -fcall-used-g7
+SPARC_REG_CFLAGS = -ffixed-g4 -ffixed-g5 $(call cc-option,-fcall-used-g5) $(call cc-option,-fcall-used-g7)
 
 $(vobjs): KBUILD_CFLAGS := $(filter-out $(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) $(SPARC_REG_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)