2015-08-12 12:34:53

by Jan-Simon Möller

[permalink] [raw]
Subject: [PATCH] Make alignment cflags configurable.

From: Jan-Simon Möller <[email protected]>

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 <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
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


2015-08-12 12:50:54

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Make alignment cflags configurable.

On Wed, 2015-08-12 at 14:32 +0200, [email protected] wrote:
> From: Jan-Simon Möller <[email protected]>
>
> 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 <[email protected]>

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
[email protected] Intel Corporation


Attachments:
smime.p7s (5.56 kB)

2015-08-12 22:32:07

by Jan-Simon Möller

[permalink] [raw]
Subject: Re: [PATCH] Make alignment cflags configurable.

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

2015-08-12 22:37:53

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] Make alignment cflags configurable.

NAK. This is crazy.

On August 12, 2015 3:30:19 PM PDT, Jan-Simon Moeller <[email protected]> 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.

2015-08-12 23:21:21

by Jan-Simon Möller

[permalink] [raw]
Subject: Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

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 <[email protected]> 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

2015-08-12 23:37:12

by David Woodhouse

[permalink] [raw]
Subject: Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

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
[email protected] Intel Corporation


Attachments:
smime.p7s (5.56 kB)

2015-08-13 00:02:51

by Jan-Simon Möller

[permalink] [raw]
Subject: Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

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 <[email protected]>
Date: Fri Apr 10 14:08:46 2015 +0200

x86: Align jump targets to 1-byte boundaries

Best,
Jan-Simon

2015-08-13 00:05:16

by David Woodhouse

[permalink] [raw]
Subject: Re: [llvmlinux] [PATCH] Make alignment cflags configurable.

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


Attachments:
smime.p7s (5.56 kB)