[PATCH] Make alignment cflags configurable.

[PATCH] Make alignment cflags configurable.

[dl9pf]

From: Jan-Simon Möller <dl9pf@gmx.de>

This patch adds switches for
-falign-jumps=1
and
-falign-loops=1

Default is off by intention to allow seamless operation.

Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>
CC: tglx@linutronix.de
CC: mingo@redhat.com
CC: hpa@zytor.com
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
CC: llvmlinux@lists.linuxfoundation.org
CC: dwmw2@infradead.org
CC: behanw@converseincode.com
---
 arch/x86/Kconfig  | 22 ++++++++++++++++++++++
 arch/x86/Makefile |  4 ++++
 2 files changed, 26 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b3a1a5d..ebd4b03 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -644,6 +644,28 @@ config SCHED_OMIT_FRAME_POINTER
 
 	  If in doubt, say "Y".
 
+config ALIGN_JUMP_TARGETS_NONDEFAULT
+       bool "Align jump targets to 1 byte"
+       default n
+       ---help---
+         Align jump targets to 1 byte, not the default 16 bytes.
+         This results in a smaller packaging of the Kernel
+         (around 300k in a usual configuration) by removing
+         noops that would otherwise be inserted for
+         alignment reasons.
+         The default is off to make sure all compilers work.
+
+config ALIGN_LOOPS_NONDEFAULT
+       bool "Pack loops tightly"
+       default n
+       ---help---
+         Align loops to 1 byte, not the default 16 bytes.
+         This results in a smaller packaging of the Kernel
+         by removing noops that would otherwise be inserted for
+         alignment reasons.
+         The default is off to make sure all compilers work.
+
+
 menuconfig HYPERVISOR_GUEST
 	bool "Linux guest support"
 	---help---
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 118e6de..38c38f4 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -77,11 +77,15 @@ else
         KBUILD_AFLAGS += -m64
         KBUILD_CFLAGS += -m64
 
+ifdef ALIGN_JUMP_TARGETS_NONDEFAULT
         # Align jump targets to 1 byte, not the default 16 bytes:
         KBUILD_CFLAGS += -falign-jumps=1
+endif
 
+ifdef ALIGN_LOOPS_NONDEFAULT
         # Pack loops tightly as well:
         KBUILD_CFLAGS += -falign-loops=1
+endif
 
         # Don't autogenerate traditional x87 instructions
         KBUILD_CFLAGS += $(call cc-option,-mno-80387)
-- 
2.5.0

Re: [PATCH] Make alignment cflags configurable.

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

On Wed, 2015-08-12 at 14:32 +0200, dl9pf@gmx.de wrote:
> From: Jan-Simon Möller <dl9pf@gmx.de>
> 
> This patch adds switches for
> -falign-jumps=1
> and
> -falign-loops=1
> 
> Default is off by intention to allow seamless operation.
> 
> Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>

You could mention that this is to fix the clang build. But why is it
needed? It isn't that clang just doesn't accept the option, is it?
Otherwise we could just use $(call cc-option, -falign-jumps=1) etc.

Did you get to the bottom of the clang failure here? Just turning this
off without a coherent explanation doesn't seem like the right thing to
do.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

Re: [PATCH] Make alignment cflags configurable.

[Jan-Simon Moeller]

Hi all!

> You could mention that this is to fix the clang build. But why is it
> needed? It isn't that clang just doesn't accept the option, is it?
> Otherwise we could just use $(call cc-option, -falign-jumps=1) etc.

Yes it is to fix the build with clang. 
I tried cc-option, but it does not improve the situation (more below).
This is  why I chose the config option approach in the patch.

 
> Did you get to the bottom of the clang failure here? Just turning this
> off without a coherent explanation doesn't seem like the right thing to
> do.

I know it is not the final solution which is why I turned it into a config 
option. We can still debate if default should be "y" or "n". This way we all 
can proceed.

@Ingo: would it be fine if we wrap it into a config option defaulting to "y" ?


What I can say so far is that although clang warns about the unknown option
and ignores it, the resulting kernel still fails to boot somewhere early in 
start_kernel(). I'm still investigating. 

My current trace ends like this:
page_address_init ~ setup_arch ~ then arch/x86/kernel/setup.c:898
setup.c:898 is a printk actually ... 
early_idt_handler_array[i]  ~> early_idt_handler_common

The mail thread is here:
http://lists.linuxfoundation.org/pipermail/llvmlinux/2015-August/001276.html


<wild guess>
We still build with -no-integrated-as which means we use gas. Maybe the flag
is passed-on there and things get confused. 
</wile guess>

Best,
Jan-Simon

Re: [PATCH] Make alignment cflags configurable.

[H. Peter Anvin]

NAK.  This is crazy.

On August 12, 2015 3:30:19 PM PDT, Jan-Simon Moeller <dl9pf@gmx.de> wrote:
>Hi all!
>
>> You could mention that this is to fix the clang build. But why is it
>> needed? It isn't that clang just doesn't accept the option, is it?
>> Otherwise we could just use $(call cc-option, -falign-jumps=1) etc.
>
>Yes it is to fix the build with clang. 
>I tried cc-option, but it does not improve the situation (more below).
>This is  why I chose the config option approach in the patch.
>
> 
>> Did you get to the bottom of the clang failure here? Just turning
>this
>> off without a coherent explanation doesn't seem like the right thing
>to
>> do.
>
>I know it is not the final solution which is why I turned it into a
>config 
>option. We can still debate if default should be "y" or "n". This way
>we all 
>can proceed.
>
>@Ingo: would it be fine if we wrap it into a config option defaulting
>to "y" ?
>
>
>What I can say so far is that although clang warns about the unknown
>option
>and ignores it, the resulting kernel still fails to boot somewhere
>early in 
>start_kernel(). I'm still investigating. 
>
>My current trace ends like this:
>page_address_init ~ setup_arch ~ then arch/x86/kernel/setup.c:898
>setup.c:898 is a printk actually ... 
>early_idt_handler_array[i]  ~> early_idt_handler_common
>
>The mail thread is here:
>http://lists.linuxfoundation.org/pipermail/llvmlinux/2015-August/001276.html
>
>
><wild guess>
>We still build with -no-integrated-as which means we use gas. Maybe the
>flag
>is passed-on there and things get confused. 
></wile guess>
>
>Best,
>Jan-Simon

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

[Jan-Simon Moeller]

Am Mittwoch, 12. August 2015, 15:37:05 schrieb H. Peter Anvin:
> NAK.  This is crazy.

Ok roger that. What about the cc-option at least?

This way we can figure why it does not work for clang and keep things as-is 
for gcc.

JS

 
> On August 12, 2015 3:30:19 PM PDT, Jan-Simon Moeller <dl9pf@gmx.de> wrote:
> >Hi all!
> >
> >> You could mention that this is to fix the clang build. But why is it
> >> needed? It isn't that clang just doesn't accept the option, is it?
> >> Otherwise we could just use $(call cc-option, -falign-jumps=1) etc.
> >
> >Yes it is to fix the build with clang.
> >I tried cc-option, but it does not improve the situation (more below).
> >This is  why I chose the config option approach in the patch.
> >
> >> Did you get to the bottom of the clang failure here? Just turning
> >
> >this
> >
> >> off without a coherent explanation doesn't seem like the right thing
> >
> >to
> >
> >> do.
> >
> >I know it is not the final solution which is why I turned it into a
> >config
> >option. We can still debate if default should be "y" or "n". This way
> >we all
> >can proceed.
> >
> >@Ingo: would it be fine if we wrap it into a config option defaulting
> >to "y" ?
> >
> >
> >What I can say so far is that although clang warns about the unknown
> >option
> >and ignores it, the resulting kernel still fails to boot somewhere
> >early in
> >start_kernel(). I'm still investigating.
> >
> >My current trace ends like this:
> >page_address_init ~ setup_arch ~ then arch/x86/kernel/setup.c:898
> >setup.c:898 is a printk actually ...
> >early_idt_handler_array[i]  ~> early_idt_handler_common
> >
> >The mail thread is here:
> >http://lists.linuxfoundation.org/pipermail/llvmlinux/2015-August/001276.htm
> >l
> >
> >
> ><wild guess>
> >We still build with -no-integrated-as which means we use gas. Maybe the
> >flag
> >is passed-on there and things get confused.
> ></wile guess>
> >
> >Best,
> >Jan-Simon

Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Thu, 2015-08-13 at 01:17 +0200, Jan-Simon Moeller wrote:
> This way we can figure why it does not work for clang and keep things 
> as-is for gcc.

Let's figure it out first. Or at *least* bisect and find which kernel
commit broke it.

Then we can talk about the best way to fix it.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

[Jan-Simon Moeller]

Am Donnerstag, 13. August 2015, 00:37:05 schrieb David Woodhouse:
> On Thu, 2015-08-13 at 01:17 +0200, Jan-Simon Moeller wrote:
> > This way we can figure why it does not work for clang and keep things
> > as-is for gcc.
> 
> Let's figure it out first. Or at *least* bisect and find which kernel
> commit broke it.
> 
> Then we can talk about the best way to fix it.

I bisected it already.

down to -faling-jumps in

be6cb02779ca74d83481f017db21578cfe92891c is the first bad commit
commit be6cb02779ca74d83481f017db21578cfe92891c
Author: Ingo Molnar <mingo@kernel.org>
Date:   Fri Apr 10 14:08:46 2015 +0200

    x86: Align jump targets to 1-byte boundaries

Best,
Jan-Simon

Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]

On Thu, 2015-08-13 at 01:59 +0200, Jan-Simon Moeller wrote:
> 
> I bisected it already.
> 
> down to -faling-jumps in

Can you work out on which file(s) this change actually makes the
difference?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]