Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1669684 > unrolled thread

[PATCH v4 3/3] x86/build: Specify stack alignment for clang

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke <mka@chromium.org> - 2017-06-19 21:00 +0200
    Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang hpa@zytor.com - 2017-06-19 22:40 +0200
      Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke <mka@chromium.org> - 2017-06-19 22:50 +0200
        Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang Ingo Molnar <mingo@kernel.org> - 2017-06-20 11:30 +0200
          Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke <mka@chromium.org> - 2017-06-20 19:40 +0200
            Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang Ingo Molnar <mingo@kernel.org> - 2017-06-21 09:20 +0200

#1669684 — [PATCH v4 3/3] x86/build: Specify stack alignment for clang

For gcc stack alignment is configured with -mpreferred-stack-boundary=N,
clang has the option -mstack-alignment=N for that purpose. Use the same
alignment as with gcc.

If the alignment is not specified clang assumes an alignment of
16 bytes, as required by the standard ABI. However as mentioned in
d9b0cde91c60 ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if
supported") the standard kernel entry on x86-64 leaves the stack
on an 8-byte boundary, as a consequence clang will keep the stack
misaligned.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/x86/Makefile | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index b2dae639f778..9406d3670452 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -11,6 +11,14 @@ else
         KBUILD_DEFCONFIG := $(ARCH)_defconfig
 endif
 
+# Handle different option names for specifying stack alignment with gcc and
+# clang.
+ifeq ($(cc-name),clang)
+	cc_stack_align_opt := -mstack-alignment
+else
+	cc_stack_align_opt := -mpreferred-stack-boundary
+endif
+
 # How to compile the 16-bit code.  Note we always compile for -march=i386;
 # that way we can complain to the user if the CPU is insufficient.
 #
@@ -28,7 +36,7 @@ REALMODE_CFLAGS	:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \
 
 REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), -ffreestanding)
 REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), -fno-stack-protector)
-REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), -mpreferred-stack-boundary=2)
+REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS), $(cc_stack_align_opt)=2)
 export REALMODE_CFLAGS
 
 # BITS is used as extension for files which are available in a 32 bit
@@ -65,8 +73,10 @@ ifeq ($(CONFIG_X86_32),y)
         # with nonstandard options
         KBUILD_CFLAGS += -fno-pic
 
-        # prevent gcc from keeping the stack 16 byte aligned
-        KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=2)
+        # Align the stack to the register width instead of using the default
+        # alignment of 16 bytes. This reduces stack usage and the number of
+        # alignment instructions.
+        KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align_opt)=2)
 
         # Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc use
         # a lot more stack due to the lack of sharing of stacklots:
@@ -98,8 +108,14 @@ else
         KBUILD_CFLAGS += $(call cc-option,-mno-80387)
         KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
 
-	# Use -mpreferred-stack-boundary=3 if supported.
-	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
+        # By default gcc and clang use a stack alignment of 16 bytes for x86.
+        # However the standard kernel entry on x86-64 leaves the stack on an
+        # 8-byte boundary. If the compiler isn't informed about the actual
+        # alignment it will generate extra alignment instructions for the
+        # default alignment which keep the stack *mis*aligned.
+        # Furthermore an alignment to the register width reduces stack usage
+        # and the number of alignment instructions.
+        KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align_opt)=3)
 
 	# Use -mskip-rax-setup if supported.
 	KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup)
-- 
2.13.1.518.g3df882009-goog

[toc] | [next] | [standalone]

#1669930

On June 19, 2017 11:37:57 AM PDT, Matthias Kaehlcke <mka@chromium.org> wrote:
>For gcc stack alignment is configured with
>-mpreferred-stack-boundary=N,
>clang has the option -mstack-alignment=N for that purpose. Use the same
>alignment as with gcc.
>
>If the alignment is not specified clang assumes an alignment of
>16 bytes, as required by the standard ABI. However as mentioned in
>d9b0cde91c60 ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if
>supported") the standard kernel entry on x86-64 leaves the stack
>on an 8-byte boundary, as a consequence clang will keep the stack
>misaligned.
>
>Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>---
> arch/x86/Makefile | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
>diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>index b2dae639f778..9406d3670452 100644
>--- a/arch/x86/Makefile
>+++ b/arch/x86/Makefile
>@@ -11,6 +11,14 @@ else
>         KBUILD_DEFCONFIG := $(ARCH)_defconfig
> endif
> 
>+# Handle different option names for specifying stack alignment with
>gcc and
>+# clang.
>+ifeq ($(cc-name),clang)
>+	cc_stack_align_opt := -mstack-alignment
>+else
>+	cc_stack_align_opt := -mpreferred-stack-boundary
>+endif
>+
># How to compile the 16-bit code.  Note we always compile for
>-march=i386;
> # that way we can complain to the user if the CPU is insufficient.
> #
>@@ -28,7 +36,7 @@ REALMODE_CFLAGS	:= $(M16_CFLAGS) -g -Os -D__KERNEL__
>\
> 
>REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
>-ffreestanding)
>REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
>-fno-stack-protector)
>-REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
>-mpreferred-stack-boundary=2)
>+REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
>$(cc_stack_align_opt)=2)
> export REALMODE_CFLAGS
> 
> # BITS is used as extension for files which are available in a 32 bit
>@@ -65,8 +73,10 @@ ifeq ($(CONFIG_X86_32),y)
>         # with nonstandard options
>         KBUILD_CFLAGS += -fno-pic
> 
>-        # prevent gcc from keeping the stack 16 byte aligned
>-        KBUILD_CFLAGS += $(call
>cc-option,-mpreferred-stack-boundary=2)
>+        # Align the stack to the register width instead of using the
>default
>+        # alignment of 16 bytes. This reduces stack usage and the
>number of
>+        # alignment instructions.
>+        KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align_opt)=2)
> 
># Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc
>use
>         # a lot more stack due to the lack of sharing of stacklots:
>@@ -98,8 +108,14 @@ else
>         KBUILD_CFLAGS += $(call cc-option,-mno-80387)
>         KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
> 
>-	# Use -mpreferred-stack-boundary=3 if supported.
>-	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
>+        # By default gcc and clang use a stack alignment of 16 bytes
>for x86.
>+        # However the standard kernel entry on x86-64 leaves the stack
>on an
>+        # 8-byte boundary. If the compiler isn't informed about the
>actual
>+        # alignment it will generate extra alignment instructions for
>the
>+        # default alignment which keep the stack *mis*aligned.
>+        # Furthermore an alignment to the register width reduces stack
>usage
>+        # and the number of alignment instructions.
>+        KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align_opt)=3)
> 
> 	# Use -mskip-rax-setup if supported.
> 	KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup)

Goddammit.

How many times do I have to say NAK to

>+ifeq ($(cc-name),clang)

... before it sinks in?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1669936

El Mon, Jun 19, 2017 at 01:17:03PM -0700 hpa@zytor.com ha dit:

> On June 19, 2017 11:37:57 AM PDT, Matthias Kaehlcke <mka@chromium.org> wrote:
> >For gcc stack alignment is configured with
> >-mpreferred-stack-boundary=N,
> >clang has the option -mstack-alignment=N for that purpose. Use the same
> >alignment as with gcc.
> >
> >If the alignment is not specified clang assumes an alignment of
> >16 bytes, as required by the standard ABI. However as mentioned in
> >d9b0cde91c60 ("x86-64, gcc: Use -mpreferred-stack-boundary=3 if
> >supported") the standard kernel entry on x86-64 leaves the stack
> >on an 8-byte boundary, as a consequence clang will keep the stack
> >misaligned.
> >
> >Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >---
> > arch/x86/Makefile | 26 +++++++++++++++++++++-----
> > 1 file changed, 21 insertions(+), 5 deletions(-)
> >
> >diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >index b2dae639f778..9406d3670452 100644
> >--- a/arch/x86/Makefile
> >+++ b/arch/x86/Makefile
> >@@ -11,6 +11,14 @@ else
> >         KBUILD_DEFCONFIG := $(ARCH)_defconfig
> > endif
> > 
> >+# Handle different option names for specifying stack alignment with
> >gcc and
> >+# clang.
> >+ifeq ($(cc-name),clang)
> >+	cc_stack_align_opt := -mstack-alignment
> >+else
> >+	cc_stack_align_opt := -mpreferred-stack-boundary
> >+endif
> >+
> ># How to compile the 16-bit code.  Note we always compile for
> >-march=i386;
> > # that way we can complain to the user if the CPU is insufficient.
> > #
> >@@ -28,7 +36,7 @@ REALMODE_CFLAGS	:= $(M16_CFLAGS) -g -Os -D__KERNEL__
> >\
> > 
> >REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
> >-ffreestanding)
> >REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
> >-fno-stack-protector)
> >-REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
> >-mpreferred-stack-boundary=2)
> >+REALMODE_CFLAGS += $(call __cc-option, $(CC), $(REALMODE_CFLAGS),
> >$(cc_stack_align_opt)=2)
> > export REALMODE_CFLAGS
> > 
> > # BITS is used as extension for files which are available in a 32 bit
> >@@ -65,8 +73,10 @@ ifeq ($(CONFIG_X86_32),y)
> >         # with nonstandard options
> >         KBUILD_CFLAGS += -fno-pic
> > 
> >-        # prevent gcc from keeping the stack 16 byte aligned
> >-        KBUILD_CFLAGS += $(call
> >cc-option,-mpreferred-stack-boundary=2)
> >+        # Align the stack to the register width instead of using the
> >default
> >+        # alignment of 16 bytes. This reduces stack usage and the
> >number of
> >+        # alignment instructions.
> >+        KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align_opt)=2)
> > 
> ># Disable unit-at-a-time mode on pre-gcc-4.0 compilers, it makes gcc
> >use
> >         # a lot more stack due to the lack of sharing of stacklots:
> >@@ -98,8 +108,14 @@ else
> >         KBUILD_CFLAGS += $(call cc-option,-mno-80387)
> >         KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
> > 
> >-	# Use -mpreferred-stack-boundary=3 if supported.
> >-	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
> >+        # By default gcc and clang use a stack alignment of 16 bytes
> >for x86.
> >+        # However the standard kernel entry on x86-64 leaves the stack
> >on an
> >+        # 8-byte boundary. If the compiler isn't informed about the
> >actual
> >+        # alignment it will generate extra alignment instructions for
> >the
> >+        # default alignment which keep the stack *mis*aligned.
> >+        # Furthermore an alignment to the register width reduces stack
> >usage
> >+        # and the number of alignment instructions.
> >+        KBUILD_CFLAGS += $(call cc-option,$(cc_stack_align_opt)=3)
> > 
> > 	# Use -mskip-rax-setup if supported.
> > 	KBUILD_CFLAGS += $(call cc-option,-mskip-rax-setup)
> 
> Goddammit.
> 
> How many times do I have to say NAK to
> 
> >+ifeq ($(cc-name),clang)
> 
> ... before it sinks in?

The initial version of this patch doesn't have this condition and just
uses cc-option to select the appropriate option. Ingo didn't like the
duplication and suggested the use of a variable, which kinda implies a
check for the compiler name. I also think this is a cleaner
solution. but I'm happy to respin the patch if you have another
suggestion that is ok for both of you.

[toc] | [prev] | [next] | [standalone]

#1670522

* Matthias Kaehlcke <mka@chromium.org> wrote:

> Ingo didn't like the duplication and suggested the use of a variable, which 
> kinda implies a check for the compiler name.

I don't think it implies that: why cannot cc_stack_align_opt probe for the 
compiler option and use whichever is available, without hard-coding the compiler 
name?

> I also think this is a cleaner solution. [...]

I concur with hpa: hard-coding compiler is awfully fragile and ugly as well.

With the proper probing of compiler options it will be possible for compilers to 
consolidate their options, and it would be possible for a third compiler to use a 
mixture of GCC and Clang options. With hard-coding none of that flexibility is 
available.

> but I'm happy to respin the patch if you have another suggestion that is ok for 
> both of you.

Please do.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1670987

El Tue, Jun 20, 2017 at 11:20:54AM +0200 Ingo Molnar ha dit:

> 
> * Matthias Kaehlcke <mka@chromium.org> wrote:
> 
> > Ingo didn't like the duplication and suggested the use of a variable, which 
> > kinda implies a check for the compiler name.
> 
> I don't think it implies that: why cannot cc_stack_align_opt probe for the 
> compiler option and use whichever is available, without hard-coding the compiler 
> name?

We could do this:

ifneq ($(call __cc-option, $(CC), -mno-sse, -mpreferred-stack-boundary=3,),)
        cc_stack_align_opt := -mpreferred-stack-boundary
endif
ifneq ($(call cc-option, -mstack-alignment=3,),)
        cc_stack_align_opt := -mstack-alignment
endif

If preferred cc-option could be used to probe for
-mpreferred-stack-boundary , however it would require REALMODE_CFLAGS
to be moved further down in the Makefile.

Since this solution also won't win a beauty price please let me know
if it is acceptable before respinning the patch or if you have other
suggestions.

> > I also think this is a cleaner solution. [...]
> 
> I concur with hpa: hard-coding compiler is awfully fragile and ugly as well.
> 
> With the proper probing of compiler options it will be possible for compilers to 
> consolidate their options, and it would be possible for a third compiler to use a 
> mixture of GCC and Clang options. With hard-coding none of that flexibility is 
> available.
> 
> > but I'm happy to respin the patch if you have another suggestion that is ok for 
> > both of you.
> 
> Please do.
> 
> Thanks,
> 
> 	Ingo

[toc] | [prev] | [next] | [standalone]

#1671429

* Matthias Kaehlcke <mka@chromium.org> wrote:

> El Tue, Jun 20, 2017 at 11:20:54AM +0200 Ingo Molnar ha dit:
> 
> > 
> > * Matthias Kaehlcke <mka@chromium.org> wrote:
> > 
> > > Ingo didn't like the duplication and suggested the use of a variable, which 
> > > kinda implies a check for the compiler name.
> > 
> > I don't think it implies that: why cannot cc_stack_align_opt probe for the 
> > compiler option and use whichever is available, without hard-coding the compiler 
> > name?
> 
> We could do this:
> 
> ifneq ($(call __cc-option, $(CC), -mno-sse, -mpreferred-stack-boundary=3,),)
>         cc_stack_align_opt := -mpreferred-stack-boundary
> endif
> ifneq ($(call cc-option, -mstack-alignment=3,),)
>         cc_stack_align_opt := -mstack-alignment
> endif

The principle Looks good to me - but I'd make the second probing an 'else' branch, 
i.e. probe for a suitable compiler option until we find one. That would also not 
burden the GCC build with probing for different compiler options.

Please also add a comment in the code that explains that the first option is a GCC 
option and the second one is a Clang-ism.

> Since this solution also won't win a beauty price please let me know
> if it is acceptable before respinning the patch or if you have other
> suggestions.

This one already looks a lot cleaner to me than any of the previous ones.

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web