public inbox for linux-arch@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [2.6 patch] re-add -ffreestanding
       [not found]     ` <20060906223748.GC12157@stusta.de>
@ 2006-09-07  6:30       ` Russell King
  2006-09-07 10:27         ` Adrian Bunk
  0 siblings, 1 reply; 7+ messages in thread
From: Russell King @ 2006-09-07  6:30 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Andi Kleen, Andrew Morton, linux-kernel, Roman Zippel, linux-arch

On Thu, Sep 07, 2006 at 12:37:48AM +0200, Adrian Bunk wrote:
> On Wed, Aug 30, 2006 at 07:39:05PM +0100, Russell King wrote:
> > Looking at the effect of -ffreestanding on ARM, it appears that on one
> > hand, the overall image size is reduced by 0.016% but we end up with worse
> > code - eg, strlen() of the same string in the same function evaluated
> > multiple times vs once without -ffreestanding.
> > 
> > The difference probably comes down to the lack of __attribute__((pure))
> > on our string functions in linux/string.h.
> > 
> > If we are going to go for -ffreestanding, we need to fix linux/string.h
> > in that respect _first_.
> 
> We are talking about reverting the patch that removed -ffreestanding, 
> and that broke at least two architectures although it wrongly claimed 
> it would have been a safe patch.

Wrong.  Your patch unconditionally adds it for _ALL_ architectures.
Below is the extract which you posted which supports this fact.

For the elimination of any doubt, I do _NOT_ want this patch merged as
is.  Take that as the _third_ architecture maintainer who has NACK'd
your patch (as you should've taken my first objection as that and
apparantly didn't.)

... and maybe you should copy linux-arch with architecture-wide changes
so that all architecture maintainers are aware of what you're trying to
do?

--- linux-2.6.18-rc4-mm3/Makefile.old   2006-08-30 16:59:31.000000000 +0200
+++ linux-2.6.18-rc4-mm3/Makefile       2006-08-30 17:02:42.000000000 +0200
@@ -308,7 +308,7 @@
 CPPFLAGS        := -D__KERNEL__ $(LINUXINCLUDE)

 CFLAGS          := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-                   -fno-strict-aliasing -fno-common
+                   -fno-strict-aliasing -fno-common -ffreestanding
 AFLAGS          := -D__ASSEMBLY__

 # Read KERNELRELEASE from include/config/kernel.release (if it exists)

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [2.6 patch] re-add -ffreestanding
  2006-09-07  6:30       ` [2.6 patch] re-add -ffreestanding Russell King
@ 2006-09-07 10:27         ` Adrian Bunk
  2006-09-07 11:40           ` Roman Zippel
  2006-09-07 11:43           ` Russell King
  0 siblings, 2 replies; 7+ messages in thread
From: Adrian Bunk @ 2006-09-07 10:27 UTC (permalink / raw)
  To: Andi Kleen, Andrew Morton, linux-kernel, Roman Zippel, linux-arch

On Thu, Sep 07, 2006 at 07:30:49AM +0100, Russell King wrote:
>...
> ... and maybe you should copy linux-arch with architecture-wide changes
> so that all architecture maintainers are aware of what you're trying to
> do?
>...

Andis patch "x86_64: Don't define string functions to builtin" (sic) 
that removed -ffreestanding for all architectures except i386 and that 
broke at least two architectures was neither sent to linux-arch nor to 
linux-kernel.

And I'm getting bashed for sendind a patch to revert it "only" to 
linux-kernel...

> Russell King

cu
Adrian
(who has just deleted all his cross compilers for getting rid of all 
 these troubles)

-- 

       "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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [2.6 patch] re-add -ffreestanding
  2006-09-07 10:27         ` Adrian Bunk
@ 2006-09-07 11:40           ` Roman Zippel
  2006-09-07 11:43           ` Russell King
  1 sibling, 0 replies; 7+ messages in thread
From: Roman Zippel @ 2006-09-07 11:40 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Andi Kleen, Andrew Morton, linux-kernel, linux-arch

Hi,

On Thu, 7 Sep 2006, Adrian Bunk wrote:

> And I'm getting bashed for sendind a patch to revert it "only" to 
> linux-kernel...

No, you get "bashed" for pushing a patch that only hides problems instead 
of fixing them and that only creates new problems.

> (who has just deleted all his cross compilers for getting rid of all 
>  these troubles)

Your help is certainly appreciated, but you have to work a bit more with 
the people who actually use these platforms.

bye, Roman

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [2.6 patch] re-add -ffreestanding
  2006-09-07 10:27         ` Adrian Bunk
  2006-09-07 11:40           ` Roman Zippel
@ 2006-09-07 11:43           ` Russell King
  2006-09-07 14:03             ` Kyle Moffett
  1 sibling, 1 reply; 7+ messages in thread
From: Russell King @ 2006-09-07 11:43 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Andi Kleen, Andrew Morton, linux-kernel, Roman Zippel, linux-arch

On Thu, Sep 07, 2006 at 12:27:40PM +0200, Adrian Bunk wrote:
> And I'm getting bashed for sendind a patch to revert it "only" to 
> linux-kernel...

You're not getting bashed - you're getting directed to the correct place
to discuss this issue.  There are some architecture maintainers who might
have some input who aren't subscribed to linux-kernel.

Incidentally, the addition of -ffreestanding was only sent to linux-kernel
in the first place back in 2004, so this actually gives those architecture
maintainers a chance to actually comment on it.  At the time, no one
commented on it - maybe it got missed?  I certainly did.

 http://marc.theaimsgroup.com/?l=linux-kernel&m=110194462121275&w=2

As far as your argument that the kernel is not a hosted environment, that's
debatable (as you're finding out).

If we decide that we want the compiler to treat our source as if it were a
hosted environment, and we provide sufficient implementation of a conforming
nature of a hosted environment then that is our perogative to do so.  That
is a decision that we are entirely free to make.  By doing so, we take on
the responsibility to provide whatever is required for a hosted environment
as opposed to the more limited functionality of a freestanding environment.

We _have_ taken on that responsibility.  We have even taken on the
responsibility to provide the compiler's internal library functions in
some cases.

The question really comes down to two points:

1. do the C standard defined library services we use in the kernel have
   the behaviour required by the C standard
2. do we provide everything that the compiler would want to use in the
   environment mode we have chosen.

The answer to both is generally yes, except when something is omitted for
a reason.  (eg, float support.)

And two final points:

- according to what I read in the gcc manual, free standing environments
  are required to provide float support.  We don't, so it could be possible
  to argue that we provide neither a freestanding nor a hosted environment,
  and, therefore, we shouldn't be using gcc at all.

- architecture maintainers should be left to decide what implementation
  option they require the kernel to be built in, since it's the architecture
  support code which provides most of the runtime environment.

> Adrian
> (who has just deleted all his cross compilers for getting rid of all 
>  these troubles)

If that's how you feel and what you've done, you seem to have disqualified
yourself from handling generic kernel changes.

I notice that you didn't respond to anything else in my message, so I can
only assume that you've accepted my reasoning and are no longer going to
push this patch.  So treat the above as the finer detailed reasoning.

I'm not anti--ffreestanding per se, but if we _are_ going to switch (back)
to that compiler mode, I want to ensure that we aren't walking backwards
from the point I _thought_ we were at.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [2.6 patch] re-add -ffreestanding
  2006-09-07 11:43           ` Russell King
@ 2006-09-07 14:03             ` Kyle Moffett
  2006-09-07 14:25               ` Russell King
  2006-09-07 14:29               ` Roman Zippel
  0 siblings, 2 replies; 7+ messages in thread
From: Kyle Moffett @ 2006-09-07 14:03 UTC (permalink / raw)
  To: Russell King
  Cc: Adrian Bunk, Andi Kleen, Andrew Morton, linux-kernel,
	Roman Zippel, linux-arch

On Sep 07, 2006, at 07:43:58, Russell King wrote:
> On Thu, Sep 07, 2006 at 12:27:40PM +0200, Adrian Bunk wrote:
>> And I'm getting bashed for sendind a patch to revert it "only" to  
>> linux-kernel...
>
> As far as your argument that the kernel is not a hosted  
> environment, that's debatable (as you're finding out).
>
> If we decide that we want the compiler to treat our source as if it  
> were a hosted environment, and we provide sufficient implementation  
> of a conforming nature of a hosted environment then that is our  
> perogative to do so.  That is a decision that we are entirely free  
> to make.  By doing so, we take on the responsibility to provide  
> whatever is required for a hosted environment as opposed to the  
> more limited functionality of a freestanding environment.

Ick, can anybody be persuaded to post actual effective code changes?   
IMHO the kernel should use "-ffreestanding", which is defined as  
follows in the GCC manual (which, I admit, has been wrong before, but  
it's a start at least):

"Assert that the compilation takes place in a freestanding  
environment. Implies "-fno-builtin".  A freestanding environment is  
one in which the standard library may not exist, and program startup  
may not necessarily be at "main" (which does not necessarily have a  
return type of "int").  The most obvious example is an OS kernel."

According to the above, there are two effects for turning on "- 
ffreestanding":
   1) "main" doesn't have a special argument list and return value.
   2) "-fno-builtin"

The docs for "-fno-builtin" say:

"Don't recognize built-in functions that do not begin with __builtin_  
as prefix.

GCC normally generates special code to handle certain built-in  
functions more efficiently; for instance, calls to "alloca" may  
become single instructions that adjust the stack directly, and calls  
to "memcpy" may become inline copy loops.  The resulting code is  
often both smaller and faster, but since the function calls no longer  
appear as such, you cannot set a breakpoint on those calls, nor can  
you change the behavior of the functions by linking with a different  
library.  In addition, when a function is recognized as a built-in  
function, GCC may use information about that function to warn about  
problems with calls to that function, or to generate more efficient  
code, even if the resulting code still contains calls to that  
function.  For example, warnings are given with "-Wformat" for bad  
calls to "printf", when "printf" is a builtin, and "strlen" is known  
not to modify global memory.

[...]

If you wish to enable built-in functions selectively when using -fno- 
builtin or -ffreestanding, you may define macros such as:
#define abs(n) __builtin_abs((n))
#define strcpy(d, s) __builtin_strcpy((d),(s))"

SO, the total effects of "-ffreestanding" are:
   1) "main" is not a special function
   2) "foo" does not automatically mean "__builtin_foo"

Why are you arguing so much over this change?  As far as I can tell,  
turning on "-ffreestanding" is a no-brainer; "main" isn't special in  
the kernel and allowing arch maintainers to selectively turn on or  
off "__builtin_*" effects for kernel functions in per-arch headers is  
a good thing.

If your arch absolutely *must* have "strlen" act like  
"__builtin_strlen" for some reason, just do this:
#define strlen(x) __builtin_strlen(x)

But please note that __builtin_ functions are defined by the GCC  
people to be able to decay to _any_ function call.  For example, the  
GCC people replied to
Adrian Bunk's email this way:
   http://lists.debian.org/debian-gcc/2005/03/msg00101.html

"You explicitly requested the __builtin_sprintf implementation, which  
is allowed to decay to strcpy, and overrode the -ffreestanding in  
this case."

So the meaning of __builtin_sprintf is NOT 'have GCC emit sprintf  
code', and it is NOT 'emit assembly where easy and sprintf where  
otherwise', it is 'emit calls to any function(s) in the standard C  
library which implement the requested functionality.'

So it may be OK for sprintf(buf,"%s",str); to decay to strcpy(buf,  
str) in the kernel, but if it's not the ONLY ways to turn it off are - 
fno-builtin-sprintf, -fno-builtin, and -ffreestanding.  Explicitly  
disabling these optimizations is virtually guaranteed that we'll miss  
one, we should turn them all off and selectively enable the ones that  
make sense.

> - according to what I read in the gcc manual, free standing  
> environments are required to provide float support.  We don't, so  
> it could be possible to argue that we provide neither a  
> freestanding nor a hosted environment, and, therefore, we shouldn't  
> be using gcc at all.

Freestanding environments need to provide float support  
_if_they_need_it_.  We don't need it, don't use it, and so don't have  
to provide it.

Cheers,
Kyle Moffett



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [2.6 patch] re-add -ffreestanding
  2006-09-07 14:03             ` Kyle Moffett
@ 2006-09-07 14:25               ` Russell King
  2006-09-07 14:29               ` Roman Zippel
  1 sibling, 0 replies; 7+ messages in thread
From: Russell King @ 2006-09-07 14:25 UTC (permalink / raw)
  To: Kyle Moffett
  Cc: Adrian Bunk, Andi Kleen, Andrew Morton, linux-kernel,
	Roman Zippel, linux-arch

On Thu, Sep 07, 2006 at 10:03:16AM -0400, Kyle Moffett wrote:
> On Sep 07, 2006, at 07:43:58, Russell King wrote:
> >On Thu, Sep 07, 2006 at 12:27:40PM +0200, Adrian Bunk wrote:
> >>And I'm getting bashed for sendind a patch to revert it "only" to  
> >>linux-kernel...
> >
> >As far as your argument that the kernel is not a hosted  
> >environment, that's debatable (as you're finding out).
> >
> >If we decide that we want the compiler to treat our source as if it  
> >were a hosted environment, and we provide sufficient implementation  
> >of a conforming nature of a hosted environment then that is our  
> >perogative to do so.  That is a decision that we are entirely free  
> >to make.  By doing so, we take on the responsibility to provide  
> >whatever is required for a hosted environment as opposed to the  
> >more limited functionality of a freestanding environment.
> 
> Ick, can anybody be persuaded to post actual effective code changes?   

I've already specified the changes on ARM, and suggested a fix for
them - but that got poo-poo'd.  I said:

On Wed, Aug 30, 2006 at 07:39:05PM +0100, Russell King wrote:
> Looking at the effect of -ffreestanding on ARM, it appears that on one
> hand, the overall image size is reduced by 0.016% but we end up with worse
> code - eg, strlen() of the same string in the same function evaluated
> multiple times vs once without -ffreestanding.
>
> The difference probably comes down to the lack of __attribute__((pure))
> on our string functions in linux/string.h.
>
> If we are going to go for -ffreestanding, we need to fix linux/string.h
> in that respect _first_.

So the effective code changes you ask for are: "multiple calls to
standard library functions that would not otherwise be made without
-ffreestanding".

Hence, for -ffreestanding to be acceptable to me, we need to fix
linux/string.h _first_.  That's really all I'm asking for but apparantly
that's too much to ask for.

It's not realistic to post the actual code changes because virtually every
line is different - due to differences in the register allocation caused
by the variations in code generation.  Hence, to compare it properly it's
a painstaking line by line read of each to understand what's going on and
manual compare.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [2.6 patch] re-add -ffreestanding
  2006-09-07 14:03             ` Kyle Moffett
  2006-09-07 14:25               ` Russell King
@ 2006-09-07 14:29               ` Roman Zippel
  1 sibling, 0 replies; 7+ messages in thread
From: Roman Zippel @ 2006-09-07 14:29 UTC (permalink / raw)
  To: Kyle Moffett
  Cc: Russell King, Adrian Bunk, Andi Kleen, Andrew Morton,
	linux-kernel, linux-arch

Hi,

On Thu, 7 Sep 2006, Kyle Moffett wrote:

> So it may be OK for sprintf(buf,"%s",str); to decay to strcpy(buf, str) in the
> kernel, but if it's not the ONLY ways to turn it off are -fno-builtin-sprintf,
> -fno-builtin, and -ffreestanding.  Explicitly disabling these optimizations is
> virtually guaranteed that we'll miss one, we should turn them all off and
> selectively enable the ones that make sense.

No, that would be a complete PITA and so far no arch maintainer wants to 
do this.
gcc cannot do arbitrary transformations, the result has to be the same 
according to the standard and since the standard functions we provide are 
standards compliant (modulo bugs, minus fp support) there is no problem. 
It's possible that gcc generates a call to a function we don't provide, 
but the resulting link failure is hard to miss.

bye, Roman

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2006-09-07 14:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20060830175727.GI18276@stusta.de>
     [not found] ` <200608302013.58122.ak@suse.de>
     [not found]   ` <20060830183905.GB31594@flint.arm.linux.org.uk>
     [not found]     ` <20060906223748.GC12157@stusta.de>
2006-09-07  6:30       ` [2.6 patch] re-add -ffreestanding Russell King
2006-09-07 10:27         ` Adrian Bunk
2006-09-07 11:40           ` Roman Zippel
2006-09-07 11:43           ` Russell King
2006-09-07 14:03             ` Kyle Moffett
2006-09-07 14:25               ` Russell King
2006-09-07 14:29               ` Roman Zippel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox