2005-12-14 19:10:08

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] fix the EMBEDDED menu

Hi Linus,

your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
the EMBEDDED menu.

This patch fixes this by moving CC_OPTIMIZE_FOR_SIZE above the EMBEDDED
menu.


Signed-off-by: Adrian Bunk <[email protected]>

---

init/Kconfig | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

--- linux-git/init/Kconfig.old 2005-12-14 19:58:05.000000000 +0100
+++ linux-git/init/Kconfig 2005-12-14 19:58:38.000000000 +0100
@@ -256,6 +256,18 @@

source "usr/Kconfig"

+config CC_OPTIMIZE_FOR_SIZE
+ bool "Optimize for size"
+ default y if ARM || H8300
+ help
+ Enabling this option will pass "-Os" instead of "-O2" to gcc
+ resulting in a smaller kernel.
+
+ WARNING: some versions of gcc may generate incorrect code with this
+ option. If problems are observed, a gcc upgrade may be needed.
+
+ If unsure, say N.
+
menuconfig EMBEDDED
bool "Configure standard kernel features (for small systems)"
help
@@ -338,18 +350,6 @@
Disabling this option will cause the kernel to be built without
support for epoll family of system calls.

-config CC_OPTIMIZE_FOR_SIZE
- bool "Optimize for size"
- default y if ARM || H8300
- help
- Enabling this option will pass "-Os" instead of "-O2" to gcc
- resulting in a smaller kernel.
-
- WARNING: some versions of gcc may generate incorrect code with this
- option. If problems are observed, a gcc upgrade may be needed.
-
- If unsure, say N.
-
config SHMEM
bool "Use full shmem filesystem" if EMBEDDED
default y


2005-12-14 22:07:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [2.6 patch] fix the EMBEDDED menu

Adrian Bunk <[email protected]> wrote:
>
> Hi Linus,
>
> your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
> the EMBEDDED menu.

It looks like that patch needs to be reverted or altered anyway. sparc64
machines are failing all over the place, possibly due to newly-exposed
compiler bugs.

Whether it's the compiler or it's genuine kernel bugs, the same problems
are likely to bite other architectures.

2005-12-14 22:13:05

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Wed, Dec 14, 2005 at 02:05:31PM -0800, Andrew Morton wrote:
> Adrian Bunk <[email protected]> wrote:
> >
> > Hi Linus,
> >
> > your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
> > the EMBEDDED menu.
>
> It looks like that patch needs to be reverted or altered anyway. sparc64
> machines are failing all over the place, possibly due to newly-exposed
> compiler bugs.
>
> Whether it's the compiler or it's genuine kernel bugs, the same problems
> are likely to bite other architectures.

The help text already contains a bold warning.

What about marking it as EXPERIMENTAL?
That is not that heavy as EMBEDDED but expresses this.

cu
Adrian


<-- snip -->


CC_OPTIMIZE_FOR_SIZE is still an experimental feature that doesn't work
with all supported gcc/architecture combinations.


Signed-off-by: Adrian Bunk <[email protected]>

--- linux-git/init/Kconfig.old 2005-12-14 23:08:51.000000000 +0100
+++ linux-git/init/Kconfig 2005-12-14 23:09:09.000000000 +0100
@@ -257,7 +257,7 @@
source "usr/Kconfig"

config CC_OPTIMIZE_FOR_SIZE
- bool "Optimize for size"
+ bool "Optimize for size (EXPERIMENTAL)" if EXPERIMENTAL
default y if ARM || H8300
help
Enabling this option will pass "-Os" instead of "-O2" to gcc

2005-12-14 22:17:12

by Russell King

[permalink] [raw]
Subject: Re: [2.6 patch] fix the EMBEDDED menu

On Wed, Dec 14, 2005 at 02:05:31PM -0800, Andrew Morton wrote:
> Adrian Bunk <[email protected]> wrote:
> >
> > Hi Linus,
> >
> > your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
> > the EMBEDDED menu.
>
> It looks like that patch needs to be reverted or altered anyway. sparc64
> machines are failing all over the place, possibly due to newly-exposed
> compiler bugs.
>
> Whether it's the compiler or it's genuine kernel bugs, the same problems
> are likely to bite other architectures.

I believe there are instances where ARM fails if CC_OPTIMIZE_FOR_SIZE
is not set. Luckily we have assertions in the generated assembly to
flag these as assembly errors when they happen, rather than silently
continuing to build.

Maybe CC_OPTIMIZE_FOR_SIZE should be:

bool "..." if BROKEN || (!ARM && !SPARC64)

? 8)

Note also that the help text:

WARNING: some versions of gcc may generate incorrect code with this
option. If problems are observed, a gcc upgrade may be needed.

is reversed for the situation we have with ARM. Hence, I propose we
change this to something like:

WARNING: some versions of gcc may generate incorrect code if this
option is changed form the platform default. If problems are
observed, either a gcc upgrade may be needed or alternatively
the platform default should be selected (=y for ARM and Sparc64,
n for others.)

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-12-14 22:18:13

by Jesper Juhl

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On 12/14/05, Adrian Bunk <[email protected]> wrote:
> On Wed, Dec 14, 2005 at 02:05:31PM -0800, Andrew Morton wrote:
> > Adrian Bunk <[email protected]> wrote:
> > >
> > > Hi Linus,
> > >
> > > your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
> > > the EMBEDDED menu.
> >
> > It looks like that patch needs to be reverted or altered anyway. sparc64
> > machines are failing all over the place, possibly due to newly-exposed
> > compiler bugs.
> >
> > Whether it's the compiler or it's genuine kernel bugs, the same problems
> > are likely to bite other architectures.
>
> The help text already contains a bold warning.
>
> What about marking it as EXPERIMENTAL?
> That is not that heavy as EMBEDDED but expresses this.
>

I, for one, definately think this is a good idea.
Actually, it boggles my mind what this is doing outside of EMBEDDED -
I just noticed it had moved when I build -git4 and oldconfig promted
me about it.


--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

2005-12-14 22:24:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

Adrian Bunk <[email protected]> wrote:
>
> CC_OPTIMIZE_FOR_SIZE is still an experimental feature that doesn't work
> with all supported gcc/architecture combinations.
>
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> --- linux-git/init/Kconfig.old 2005-12-14 23:08:51.000000000 +0100
> +++ linux-git/init/Kconfig 2005-12-14 23:09:09.000000000 +0100
> @@ -257,7 +257,7 @@
> source "usr/Kconfig"
>
> config CC_OPTIMIZE_FOR_SIZE
> - bool "Optimize for size"
> + bool "Optimize for size (EXPERIMENTAL)" if EXPERIMENTAL
> default y if ARM || H8300
> help
> Enabling this option will pass "-Os" instead of "-O2" to gcc

This will cause arm and h8300 to accidentally stop using -Os if they have
!EXPERIMENTAL.

2005-12-14 22:24:26

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] fix the EMBEDDED menu

On Wed, Dec 14, 2005 at 10:17:03PM +0000, Russell King wrote:
> On Wed, Dec 14, 2005 at 02:05:31PM -0800, Andrew Morton wrote:
> > Adrian Bunk <[email protected]> wrote:
> > >
> > > Hi Linus,
> > >
> > > your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
> > > the EMBEDDED menu.
> >
> > It looks like that patch needs to be reverted or altered anyway. sparc64
> > machines are failing all over the place, possibly due to newly-exposed
> > compiler bugs.
> >
> > Whether it's the compiler or it's genuine kernel bugs, the same problems
> > are likely to bite other architectures.
>
> I believe there are instances where ARM fails if CC_OPTIMIZE_FOR_SIZE
> is not set. Luckily we have assertions in the generated assembly to
> flag these as assembly errors when they happen, rather than silently
> continuing to build.
>
> Maybe CC_OPTIMIZE_FOR_SIZE should be:
>
> bool "..." if BROKEN || (!ARM && !SPARC64)
>
> ? 8)
>
> Note also that the help text:
>
> WARNING: some versions of gcc may generate incorrect code with this
> option. If problems are observed, a gcc upgrade may be needed.
>
> is reversed for the situation we have with ARM. Hence, I propose we
> change this to something like:
>
> WARNING: some versions of gcc may generate incorrect code if this
> option is changed form the platform default. If problems are
> observed, either a gcc upgrade may be needed or alternatively
> the platform default should be selected (=y for ARM and Sparc64,
> n for others.)

The patch

[2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

I sent a few minutes ago should handle this:
It allows changing the platform default only if EXPERIMENTAL is enabled.

> Russell King

cu
Adrian

BTW: The second platform defaulting to CC_OPTIMIZE_FOR_SIZE is H8300,
not SPARC64.
--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-12-14 22:26:39

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Wed, Dec 14, 2005 at 02:22:16PM -0800, Andrew Morton wrote:
> Adrian Bunk <[email protected]> wrote:
> >
> > CC_OPTIMIZE_FOR_SIZE is still an experimental feature that doesn't work
> > with all supported gcc/architecture combinations.
> >
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
> >
> > --- linux-git/init/Kconfig.old 2005-12-14 23:08:51.000000000 +0100
> > +++ linux-git/init/Kconfig 2005-12-14 23:09:09.000000000 +0100
> > @@ -257,7 +257,7 @@
> > source "usr/Kconfig"
> >
> > config CC_OPTIMIZE_FOR_SIZE
> > - bool "Optimize for size"
> > + bool "Optimize for size (EXPERIMENTAL)" if EXPERIMENTAL
> > default y if ARM || H8300
> > help
> > Enabling this option will pass "-Os" instead of "-O2" to gcc
>
> This will cause arm and h8300 to accidentally stop using -Os if they have
> !EXPERIMENTAL.

No, arm and h8300 will use -Os with !EXPERIMENTAL.

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-12-14 22:28:16

by Jesper Juhl

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On 12/14/05, Jesper Juhl <[email protected]> wrote:
> On 12/14/05, Adrian Bunk <[email protected]> wrote:
> > On Wed, Dec 14, 2005 at 02:05:31PM -0800, Andrew Morton wrote:
> > > Adrian Bunk <[email protected]> wrote:
> > > >
> > > > Hi Linus,
> > > >
> > > > your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
> > > > the EMBEDDED menu.
> > >
> > > It looks like that patch needs to be reverted or altered anyway. sparc64
> > > machines are failing all over the place, possibly due to newly-exposed
> > > compiler bugs.
> > >
> > > Whether it's the compiler or it's genuine kernel bugs, the same problems
> > > are likely to bite other architectures.
> >
> > The help text already contains a bold warning.
> >
> > What about marking it as EXPERIMENTAL?
> > That is not that heavy as EMBEDDED but expresses this.
> >
>
> I, for one, definately think this is a good idea.
> Actually, it boggles my mind what this is doing outside of EMBEDDED -
> I just noticed it had moved when I build -git4 and oldconfig promted
> me about it.
>
I should probably back this up with *why* it boggles my mind.

-Os has been in EMBEDDED for ages, so it's not been tested by the
majority of users with the wide range of compilers etc that people
use. Putting it in as prominent a location as it occupies now means a
*lot* of people are going to enable it and potentially get breakage -
not good.

If it's generally an improvement to use -Os over -O2 then I'm all for
making it more prominent after a while, but it should spend some time
in -mm first, not mainline, or at least, as Adrian suggests, be marked
EXPERIMENTAL for a while.

Let's keep it EXPERIMENTAL or in -mm only for a while, get the bug
reports flowing from people who *know* they've enabled an experimental
option or who know they are running a development tree (-mm), not from
ordinary users who use the mainline kernel and think they are safe as
long as EXPERIMENTAL, BROKEN & EMBEDDED are not enabled.

Let's care about the average user and give it some proper testing in
the tree that exists for exactely that purpose first or at least mark
it explicitly as an experimental option so that only people who are
willing to risk breakage (and who are probably also more inclined to
send bugreports) will use it for now.


--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

2005-12-14 22:30:20

by Russell King

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Wed, Dec 14, 2005 at 02:22:16PM -0800, Andrew Morton wrote:
> Adrian Bunk <[email protected]> wrote:
> >
> > CC_OPTIMIZE_FOR_SIZE is still an experimental feature that doesn't work
> > with all supported gcc/architecture combinations.
> >
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
> >
> > --- linux-git/init/Kconfig.old 2005-12-14 23:08:51.000000000 +0100
> > +++ linux-git/init/Kconfig 2005-12-14 23:09:09.000000000 +0100
> > @@ -257,7 +257,7 @@
> > source "usr/Kconfig"
> >
> > config CC_OPTIMIZE_FOR_SIZE
> > - bool "Optimize for size"
> > + bool "Optimize for size (EXPERIMENTAL)" if EXPERIMENTAL
> > default y if ARM || H8300
> > help
> > Enabling this option will pass "-Os" instead of "-O2" to gcc
>
> This will cause arm and h8300 to accidentally stop using -Os if they have
> !EXPERIMENTAL.

In effect, the total change effectively replaces EMBEDDED with
EXPERIMENTAL - since the original was:

bool "Optimize for size" if EMBEDDED

and that had the desired result for ARM with the !EMBEDDED case.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-12-14 22:40:31

by Linus Torvalds

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL



On Wed, 14 Dec 2005, Adrian Bunk wrote:
>
> What about marking it as EXPERIMENTAL?

It's not experimental on other architectures, where it is always used.

The best option _may_ be to do something like this, where we use that
"depends on" to set the expectations, and people shouldn't see it unless
they ask for EXPERIMENTAL.

It also refuses to turn on for SPARC64, since that seems to be known
broken. Or should it be just "SPARC"? Davem?

Linus

---
diff --git a/init/Kconfig b/init/Kconfig
index be74adb..6c5dbed 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -256,6 +256,20 @@ config CPUSETS

source "usr/Kconfig"

+config CC_OPTIMIZE_FOR_SIZE
+ bool "Optimize for size (Look out for broken compilers!)"
+ default y
+ depends on ARM || H8300 || EXPERIMENTAL
+ depends on !SPARC64
+ help
+ Enabling this option will pass "-Os" instead of "-O2" to gcc
+ resulting in a smaller kernel.
+
+ WARNING: some versions of gcc may generate incorrect code with this
+ option. If problems are observed, a gcc upgrade may be needed.
+
+ If unsure, say N.
+
menuconfig EMBEDDED
bool "Configure standard kernel features (for small systems)"
help
@@ -338,18 +352,6 @@ config EPOLL
Disabling this option will cause the kernel to be built without
support for epoll family of system calls.

-config CC_OPTIMIZE_FOR_SIZE
- bool "Optimize for size"
- default y if ARM || H8300
- help
- Enabling this option will pass "-Os" instead of "-O2" to gcc
- resulting in a smaller kernel.
-
- WARNING: some versions of gcc may generate incorrect code with this
- option. If problems are observed, a gcc upgrade may be needed.
-
- If unsure, say N.
-
config SHMEM
bool "Use full shmem filesystem" if EMBEDDED
default y

2005-12-14 22:44:07

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Wed, Dec 14, 2005 at 02:36:59PM -0800, Linus Torvalds wrote:
>
>
> On Wed, 14 Dec 2005, Adrian Bunk wrote:
> >
> > What about marking it as EXPERIMENTAL?
>
> It's not experimental on other architectures, where it is always used.
>
> The best option _may_ be to do something like this, where we use that
> "depends on" to set the expectations, and people shouldn't see it unless
> they ask for EXPERIMENTAL.
>...

My patch has the advantage that it doesn't allow the broken
CC_OPTIMIZE_FOR_SIZE=y setting on ARM if !EXPERIMENTAL.

> Linus
>...

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-12-14 22:46:36

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Wed, Dec 14, 2005 at 11:44:06PM +0100, Adrian Bunk wrote:
> On Wed, Dec 14, 2005 at 02:36:59PM -0800, Linus Torvalds wrote:
> >
> >
> > On Wed, 14 Dec 2005, Adrian Bunk wrote:
> > >
> > > What about marking it as EXPERIMENTAL?
> >
> > It's not experimental on other architectures, where it is always used.
> >
> > The best option _may_ be to do something like this, where we use that
> > "depends on" to set the expectations, and people shouldn't see it unless
> > they ask for EXPERIMENTAL.
> >...
>
> My patch has the advantage that it doesn't allow the broken
> CC_OPTIMIZE_FOR_SIZE=y setting on ARM if !EXPERIMENTAL.

s/CC_OPTIMIZE_FOR_SIZE=y/CC_OPTIMIZE_FOR_SIZE=n/

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-12-14 23:35:57

by Linus Torvalds

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL



On Wed, 14 Dec 2005, Adrian Bunk wrote:
>
> My patch has the advantage that it doesn't allow the broken
> CC_OPTIMIZE_FOR_SIZE=y setting on ARM if !EXPERIMENTAL.

That isn't how it was before either.

Before, it _asked_ you if EMBEDDED was set, and "y" was just the default
(but you could select "n" if you wanted to). I don't think it's
necessarily wrong to allow a -O2 ARM or H8300 kernel, although apparently
there are compilers that are broken that way too..

So my patch should give the old behaviour for the EMBEDDED platforms, and
_allow_ it for non-embedded unless SPARC64 is set, or EXPERIMENTAL isn't
set.

That sounds like the right thing to do to me..

Of course, the really right thing would be to chase down what goes wrong
with -Os. It might be a compiler bug, but it might be a real kernel bug
that just happens to bite us (-Os works fine for me on ppc64, and
apparently Fedora has used it at least on x86-64, but it might be
something subtle).

Linus

2005-12-14 23:43:14

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Wed, Dec 14, 2005 at 03:32:28PM -0800, Linus Torvalds wrote:
>
>
> On Wed, 14 Dec 2005, Adrian Bunk wrote:
> >
> > My patch has the advantage that it doesn't allow the broken
> > CC_OPTIMIZE_FOR_SIZE=y setting on ARM if !EXPERIMENTAL.
>
> That isn't how it was before either.
>
> Before, it _asked_ you if EMBEDDED was set, and "y" was just the default
> (but you could select "n" if you wanted to). I don't think it's
> necessarily wrong to allow a -O2 ARM or H8300 kernel, although apparently
> there are compilers that are broken that way too..
>
> So my patch should give the old behaviour for the EMBEDDED platforms, and
> _allow_ it for non-embedded unless SPARC64 is set, or EXPERIMENTAL isn't
> set.
>...

No, your patch allows ARM users to set CC_OPTIMIZE_FOR_SIZE=n no matter
whether they have any options set.

Before, ARM users had to set EMBEDDED=y, and with my patch they would
have to set EXPERIMENTAL=y for getting this broken configuration.


config CC_OPTIMIZE_FOR_SIZE
bool "Optimize for size (Look out for broken compilers!)"
default y
depends on ARM || H8300 || EXPERIMENTAL


does allow CC_OPTIMIZE_FOR_SIZE=n if ARM=y and EXPERIMENTAL=n
(which was reported as being broken).


config CC_OPTIMIZE_FOR_SIZE
bool "Optimize for size (EXPERIMENTAL)" if EXPERIMENTAL
default y if ARM || H8300

does not allow CC_OPTIMIZE_FOR_SIZE=n if ARM=y and EXPERIMENTAL=n.

> Linus

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-12-14 23:49:04

by David Miller

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

From: Linus Torvalds <[email protected]>
Date: Wed, 14 Dec 2005 14:36:59 -0800 (PST)

> It also refuses to turn on for SPARC64, since that seems to be known
> broken. Or should it be just "SPARC"? Davem?

Refuse it just for SPARC64.

I intend to track down what the problem is eventually.
But for now, allowing folks to enable it is only
resulting in tons of duplicate bug reports.

2005-12-15 00:41:27

by Dave Jones

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Wed, Dec 14, 2005 at 11:28:13PM +0100, Jesper Juhl wrote:

> I should probably back this up with *why* it boggles my mind.
>
> -Os has been in EMBEDDED for ages, so it's not been tested by the
> majority of users with the wide range of compilers etc that people
> use.

Fedora has had this enabled most of the time for x86, x86-64, ia64,
s390, s390x, ppc32 and ppc64 for a long time. From time to time
when a gcc bug has been tickled it's been disabled again until its
been worked out, but for the most part, it's been a complete non-event
wrt regressions. In the ~2 years that we've had it enabled I recall
2-3 occasions where it broke something badly (and it was really noticable,
like "networking doesn't work any more", or "x86-64 stopped booting"[1]),
and once or twice when moving to a newer gcc point release, it tripped an ICE.

The RHEL4 kernel has also been built this way since day 1.

Dave

[1] In that particular case, it was broken asm-x86-64/ macros that
just happened to work at -O2 by chance, so it actually found latent bugs.

2005-12-15 05:32:52

by David Miller

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

From: Dave Jones <[email protected]>
Date: Wed, 14 Dec 2005 19:40:06 -0500

> [1] In that particular case, it was broken asm-x86-64/ macros that
> just happened to work at -O2 by chance, so it actually found latent bugs.

I'm hoping it's something similar on sparc64.

I have the problem narrowed down to compiling kernel/sched.o with
"-Os" on sparc64. Let's see if I can zero in on the exact problem
quickly...

2005-12-15 06:54:23

by David Miller

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

From: Linus Torvalds <[email protected]>
Date: Wed, 14 Dec 2005 15:32:28 -0800 (PST)

> Of course, the really right thing would be to chase down what goes
> wrong with -Os.

It turns out to be a sparc64 bug, technically, in my case.

What happens is that with -Os gcc _INLINES_ schedule() into
wait_for_completion(). No that's not a typo, when optimizing
for space it inlines a huge function like schedule() whereas
without -Os it does not. :-/

Anyways, switch_to() in sparc64 (and sparc) does not work properly
when this happens. schedule() needs to execute in it's own stack
frame for the stack switching in switch_to() to work.

Would anyone be against adding "noinline" to kernel/sched.c:schedule()?
I'm about to test that, but I'm extremely positive that it makes the
problem go away.

2005-12-15 13:01:59

by Rogério Brito

[permalink] [raw]
Subject: Re: [2.6 patch] fix the EMBEDDED menu

Hi, Andrew et. al.

On Dec 14 2005, Andrew Morton wrote:
> It looks like that patch needs to be reverted or altered anyway.
> sparc64 machines are failing all over the place, possibly due to
> newly-exposed compiler bugs.

Just a (luser) datapoint here: since I have computers with small cache
(Celeron Mendocino and Duron Spitfire), I am always trying to save some
bytes here (being minimalistic as much as I can) and there and I have
been compiling kernels since about 2.6.10 with -Os in my Debian systems.

I'm currently using Debian's GCC 4.0.2 and everything seems fine here
with kernel 2.6.15-rc5.

Hope a data point helps towards making this option more visible.


Regards, Rog?rio Brito.

--
Rog?rio Brito : [email protected] : http://www.ime.usp.br/~rbrito
Homepage of the algorithms package : http://algorithms.berlios.de
Homepage on freshmeat: http://freshmeat.net/projects/algorithms/

2005-12-15 15:01:52

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

Dave Jones <[email protected]> writes:

> Fedora has had this enabled most of the time for x86, x86-64, ia64,
> s390, s390x, ppc32 and ppc64 for a long time. From time to time
> when a gcc bug has been tickled it's been disabled again until its
> been worked out, but for the most part, it's been a complete non-event
> wrt regressions.

BTW: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=173764

gcc generates incorrect code with -Os (i386).

Version-Release number of selected component (if applicable):
gcc-4.0.1-4.fc4 and gcc-4.0.2-8.fc4 (i.e., FC4-current)

gcc -W -Wall -Os test.c -o test -Werror && ./test
array: 1 2
array: 1 2

gcc -W -Wall -O2 test.c -o test -Werror && ./test
array: 1 2
array: 2 3

Additional info:
Marked "high" as it miscompiles the Linux kernel.

#include <stdio.h>

void test_it(int *array)
{
int i;

for (i = 0; i < 2; i++) {
int value = array[i];
if (i != 1000 && i != 1001)
array[i] = value + 1;
}
}

int main(void)
{
int array[2] = {1, 2};

printf("array: %i %i\n", array[0], array[1]);

test_it(array);

printf("array: %i %i\n", array[0], array[1]);

return 0;
}

--
Krzysztof Halasa

2005-12-15 17:36:29

by Dave Jones

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Thu, Dec 15, 2005 at 04:01:44PM +0100, Krzysztof Halasa wrote:
> Dave Jones <[email protected]> writes:
>
> > Fedora has had this enabled most of the time for x86, x86-64, ia64,
> > s390, s390x, ppc32 and ppc64 for a long time. From time to time
> > when a gcc bug has been tickled it's been disabled again until its
> > been worked out, but for the most part, it's been a complete non-event
> > wrt regressions.
>
> BTW: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=173764
>
> gcc generates incorrect code with -Os (i386).

Ok, I'll concede and add that one to the list too ;)

Dave

2005-12-15 19:20:23

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Thursday 15 December 2005 00:28, Jesper Juhl wrote:
> On 12/14/05, Jesper Juhl <[email protected]> wrote:
> > On 12/14/05, Adrian Bunk <[email protected]> wrote:
> > > On Wed, Dec 14, 2005 at 02:05:31PM -0800, Andrew Morton wrote:
> > > > Adrian Bunk <[email protected]> wrote:
> > > > >
> > > > > Hi Linus,
> > > > >
> > > > > your patch to allow CC_OPTIMIZE_FOR_SIZE even for EMBEDDED=n has broken
> > > > > the EMBEDDED menu.
> > > >
> > > > It looks like that patch needs to be reverted or altered anyway. sparc64
> > > > machines are failing all over the place, possibly due to newly-exposed
> > > > compiler bugs.
> > > >
> > > > Whether it's the compiler or it's genuine kernel bugs, the same problems
> > > > are likely to bite other architectures.
> > >
> > > The help text already contains a bold warning.
> > >
> > > What about marking it as EXPERIMENTAL?
> > > That is not that heavy as EMBEDDED but expresses this.
> > >
> >
> > I, for one, definately think this is a good idea.
> > Actually, it boggles my mind what this is doing outside of EMBEDDED -
> > I just noticed it had moved when I build -git4 and oldconfig promted
> > me about it.
> >
> I should probably back this up with *why* it boggles my mind.
>
> -Os has been in EMBEDDED for ages, so it's not been tested by the
> majority of users with the wide range of compilers etc that people
> use. Putting it in as prominent a location as it occupies now means a
> *lot* of people are going to enable it and potentially get breakage -
> not good.

I always compile with -Os, not only kernel, but most of userspace.
So far no problems on i386 arch.
--
vda

2005-12-17 21:20:15

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [2.6 patch] fix the EMBEDDED menu

On Wed, 14 Dec 2005, Russell King wrote:

> I believe there are instances where ARM fails if CC_OPTIMIZE_FOR_SIZE
> is not set. Luckily we have assertions in the generated assembly to
> flag these as assembly errors when they happen, rather than silently
> continuing to build.

Actually that gcc problem was unrelated to -Os vs -O2. I was able to
produce a test case that triggered the bug even with -Os at the time.
It is true that, for the kernel, using -Os made the gcc bug more
unlikely, but it could have happened regardless. This particular gcc
bug was fixed quite a while ago so using either -O2 and -Os on ARM
should be fine (especially since we fail the compilation if a bad
compiler triggers the bug).


Nicolas

2006-01-19 17:59:19

by Eric Lammerts

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL


Linus Torvalds wrote:
> +config CC_OPTIMIZE_FOR_SIZE
> + bool "Optimize for size (Look out for broken compilers!)"
> + default y
> + depends on ARM || H8300 || EXPERIMENTAL
> + depends on !SPARC64
> + help
> + Enabling this option will pass "-Os" instead of "-O2" to gcc
> + resulting in a smaller kernel.
> +
> + WARNING: some versions of gcc may generate incorrect code with this
> + option. If problems are observed, a gcc upgrade may be needed.
> +
> + If unsure, say N.

"default y" and "If unsure, say N" ?

If not sure, don't go with the default?

Eric

2006-01-19 19:15:45

by Dave Jones

[permalink] [raw]
Subject: Re: [2.6 patch] offer CC_OPTIMIZE_FOR_SIZE only if EXPERIMENTAL

On Thu, Jan 19, 2006 at 06:17:56PM +0000, Nick wrote:

> > Version-Release number of selected component (if applicable):
> > gcc-4.0.1-4.fc4 and gcc-4.0.2-8.fc4 (i.e., FC4-current)
> >
>
> I use Os on all my builds of everything, including the option for the kernel
> - that bug doesn't hit me on (heavily updated) Slackware 10, and I have
> never seen no ill effect:
>
> gcc version 3.3.4

It's a regression introduced in gcc4

Dave