Re: GCC 3.4 and broken inlining.

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

Nigel Cunningham <ncunningham@linuxmail.org> writes:

I think a better solution would be to apply the appended patch 

And then just mark the function you know needs to be inlined
as __always_inline__. I did this on x86-64 for some functions
too that need to be always inlined (although using the attribute
directly because all x86-64 compilers support it)

The rationale is that the inlining algorithm in gcc 3.4+ is quite
a lot better and actually eliminates some not-so-great inlining
we had in the past and makes the kernel a bit smaller.

-Andi

P.S.: compiler.h seems to be not "gcc 4.0 safe". Probably that needs
to be fixed too.

diff -u linux-2.6.7-bk9-work/include/linux/compiler.h-o linux-2.6.7-bk9-work/include/linux/compiler.h
--- linux-2.6.7-bk9-work/include/linux/compiler.h-o	2004-07-08 23:58:54.000000000 +0200
+++ linux-2.6.7-bk9-work/include/linux/compiler.h	2004-07-09 08:45:13.465161312 +0200
@@ -120,4 +120,8 @@
 #define noinline
 #endif
 
+#ifndef __always_inline
+#define __always_inline
+#endif
+
 #endif /* __LINUX_COMPILER_H */
diff -u linux-2.6.7-bk9-work/include/linux/compiler-gcc3.h-o linux-2.6.7-bk9-work/include/linux/compiler-gcc3.h
--- linux-2.6.7-bk9-work/include/linux/compiler-gcc3.h-o	2004-07-08 23:58:33.000000000 +0200
+++ linux-2.6.7-bk9-work/include/linux/compiler-gcc3.h	2004-07-09 08:45:11.167510608 +0200
@@ -9,6 +9,10 @@
 # define __inline	__inline__ __attribute__((always_inline))
 #endif
 
+#if __GNUC_MINOR__ >= 1
+# define __always_inline inline __attribute__((always_inline))
+#endif
+
 #if __GNUC_MINOR__ > 0
 # define __deprecated	__attribute__((deprecated))
 #endif

Re: GCC 3.4 and broken inlining.

[Nigel Cunningham]

Hi.

On Fri, 2004-07-09 at 14:51, Andi Kleen wrote:
> Nigel Cunningham <ncunningham@linuxmail.org> writes:

I'm not sure what I wrote that you're replying to.

> I think a better solution would be to apply the appended patch 

I'm going to be a pragmatist :> As long as it works. I do think that
functions being declared inline when they can't be inlined is wrong, but
there are more important things on which to spend my time.

> And then just mark the function you know needs to be inlined
> as __always_inline__. I did this on x86-64 for some functions
> too that need to be always inlined (although using the attribute
> directly because all x86-64 compilers support it)

Should that be __always_inline (no final __ in the patch below, so far
as I can see)?

[...]

> P.S.: compiler.h seems to be not "gcc 4.0 safe". Probably that needs
> to be fixed too.

Yes.

Regards,

Nigel

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

On Fri, Jul 09, 2004 at 02:56:43PM +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Fri, 2004-07-09 at 14:51, Andi Kleen wrote:
> > Nigel Cunningham <ncunningham@linuxmail.org> writes:
> 
> I'm not sure what I wrote that you're replying to.

It was just a general reply to the thread.

> > I think a better solution would be to apply the appended patch 
> 
> I'm going to be a pragmatist :> As long as it works. I do think that
> functions being declared inline when they can't be inlined is wrong, but
> there are more important things on which to spend my time.

Originally as written surely. The problem we have in Linux is that
people write some code, declare functions as inline and it usually
makes sense. But then years pass and people hack and enhance
the code and add more code to the inline functions. And even more
code. But they usually don't drop the inline marker and move it
out of headers when the function has become far too big to still be a 
good inlining candidate. So we end up with functions marked
inlined that should not really be inlined.

One reason is probably that patches are rated for "intrusiveness" 
based on the number of lines they change and when you move an inline
function out of a header even a small change can become quite big.
That's a unfortunate side effect of a normally sound policy.

Anyways, with that problem and the improved inliner in gcc 3.4 
I think it's a good idea to let the compiler decide.

It's too bad that i386 doesn't enable -funit-at-a-time, that improves
the inlining heuristics greatly.


> > And then just mark the function you know needs to be inlined
> > as __always_inline__. I did this on x86-64 for some functions
> > too that need to be always inlined (although using the attribute
> > directly because all x86-64 compilers support it)
> 
> Should that be __always_inline (no final __ in the patch below, so far
> as I can see)?

Yes. I originally wrote it with the final __, but it's better 
to not add it.

-AndI

Re: GCC 3.4 and broken inlining.

[Michael Buesch]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Andi Kleen <ak@muc.de>:
> It's too bad that i386 doesn't enable -funit-at-a-time, that improves
> the inlining heuristics greatly.

- From the gcc manpage:

- -O2 turns on all optimization flags specified by -O. It 
also turns on the following optimization flags: -fforce-mem 
- -foptimize-sibling-calls -fstrength-reduce -fcse-follow-jumps 
- -fcse-skip-blocks -frerun-cse-after-loop -frerun-loop-opt 
- -fgcse -fgcse-lm -fgcse-sm -fgcse-las -fdelete-null-pointer-checks 
- -fexpensive-optimizations -fregmove -fschedule-insns 
- -fschedule-insns2 -fsched-interblock -fsched-spec -fcaller-saves 
- -fpeephole2 -freorder-blocks -freorder-functions -fstrict-aliasing 
- -funit-at-a-time -falign-functions -falign-jumps -falign-loops 
^^^^^^^^^^^^^^^^
- -falign-labels -fcrossjumping 

Do I miss something?

- -- 
Regards Michael Buesch  [ http://www.tuxsoft.de.vu ]


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFA7mjJFGK1OIvVOP4RAuXyAJ0fM5x1jqCZGkzO2jfl42CaNLgJJwCdGZQW
3rUgM3svqyWb8JaCavWAkhg=
=3LeQ
-----END PGP SIGNATURE-----

Re: GCC 3.4 and broken inlining.

[Paweł Sikora]

On Friday 09 of July 2004 11:43, Michael Buesch wrote:
> Quoting Andi Kleen <ak@muc.de>:
> > It's too bad that i386 doesn't enable -funit-at-a-time, that improves
> > the inlining heuristics greatly.
>
> From the gcc manpage:
>
> -O2 turns on all optimization flags specified by -O. It
> also turns on the following optimization flags: -fforce-mem
> -foptimize-sibling-calls -fstrength-reduce -fcse-follow-jumps
> -fcse-skip-blocks -frerun-cse-after-loop -frerun-loop-opt
> -fgcse -fgcse-lm -fgcse-sm -fgcse-las -fdelete-null-pointer-checks
> -fexpensive-optimizations -fregmove -fschedule-insns
> -fschedule-insns2 -fsched-interblock -fsched-spec -fcaller-saves
> -fpeephole2 -freorder-blocks -freorder-functions -fstrict-aliasing
> -funit-at-a-time -falign-functions -falign-jumps -falign-loops
> ^^^^^^^^^^^^^^^^
> -falign-labels -fcrossjumping
>
> Do I miss something?

# gcc-3.4.1/gcc/opts.c

  if (optimize >= 2)
    {
(...)
      flag_unit_at_a_time = 1;
    }

btw).

I *don't trust* manpages ;)

# man gcc

-fomit-frame-pointer

   Don't keep the frame pointer in a register for functions that don't
   need one.  This avoids the instructions to save, set up and restore
   frame pointers; it also makes an extra register available in many
   functions.  It also makes debugging impossible on some machines.
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   (...)
   Enabled at levels -O, -O2, -O3, -Os.
   ^^^^^^^

  if (optimize >= 1)
    {
(...)
#ifdef CAN_DEBUG_WITHOUT_FP
      flag_omit_frame_pointer = 1;
#endif
(...)

finally, at ix86 -O[123s] doesn't turn on -fomit-frame-pointer.
manpage tells somethine else...

-- 
/* Copyright (C) 2003, SCO, Inc. This is valuable Intellectual Property. */

                           #define say(x) lie(x)

Re: GCC 3.4 and broken inlining.

[Alexandre Oliva]

On Jul  9, 2004, Nigel Cunningham <ncunningham@linuxmail.org> wrote:

> I do think that functions being declared inline when they can't be
> inlined is wrong

The problem is not when they can or cannot be inlined.  The inline
keyword has nothing to do with that.  It's a hint to the compiler,
that means that inlining the function is likely to be profitable.
But, like the `register' keyword, it's just a hint.  And, unlike the
`register' keyword, it doesn't make certain operations on objects
marked with it ill-formed (e.g., you can't take the address of an
register variable, but you can take the address of an inline
function).

The issue with inlining that makes it important for the compiler to
have something to say on the decision is that several aspects of the
profit from expanding the function inline is often machine-dependent.
It depends on the ABI (calling conventions), on how slow call
instructions are, on how important instruction cache hits are, etc.
Sure enough, GCC doesn't take all of this into account, so its
heuristics sometimes get it wrong.  But it's getting better.

Meanwhile, you should probably distinguish between must-inline,
should-inline, may-inline, should-not-inline and must-not-inline
functions.  Attribute always_inline covers the must-inline case; the
inline keyword covers the may-inline'case.  The absence of the inline
keyword implies `should-not-inline', and attribute noinline covers
must-not-inline.  should-inline can't be expressed today, and if
may-inline doesn't get the desired effect, you may feel tempted to
abuse always_inline, but I suggest you to do so using a different
macro, such that, if GCC ever introduces a new attribute that's a
stronger hint to inlining that doesn't error out if it can't be done,
you can easily switch to it.

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

On Sat, Jul 10, 2004 at 06:33:40PM -0300, Alexandre Oliva wrote:
> On Jul  9, 2004, Nigel Cunningham <ncunningham@linuxmail.org> wrote:
> 
> > I do think that functions being declared inline when they can't be
> > inlined is wrong
> 
> The problem is not when they can or cannot be inlined.  The inline
> keyword has nothing to do with that.  It's a hint to the compiler,
> that means that inlining the function is likely to be profitable.
> But, like the `register' keyword, it's just a hint.  And, unlike the
> `register' keyword, it doesn't make certain operations on objects
> marked with it ill-formed (e.g., you can't take the address of an
> register variable, but you can take the address of an inline
> function).

The main reason always_inline was added is that gcc 3.3 stopped
inlining copy_from/to_user, which generated horrible code bloat
(because it consists of a lot of code that was supposed to be optimized away,
and putting it in a static into every module generated a lot of useless code) 

At this time the poor person blessed with this compiler y took the easy way out - 
just define inline as always_inline.

It may have been possible to do it only for selected functions, but that
would have been a lot of work: you cannot really expect that the
kernel goes through a large effort just to work around compiler
bugs in specific compiler versions.

The gcc 3.4/3.5 inliner seem to be better, but is still quite bad in cases
(e.g. 3.5 stopped to inline the three line fix_to_virt() which requires 
inlining). For 3.4/3.5 it's probably feasible to do this, but I doubt
it is worth someone's time for 3.3. 

> The issue with inlining that makes it important for the compiler to
> have something to say on the decision is that several aspects of the
> profit from expanding the function inline is often machine-dependent.
> It depends on the ABI (calling conventions), on how slow call
> instructions are, on how important instruction cache hits are, etc.
> Sure enough, GCC doesn't take all of this into account, so its
> heuristics sometimes get it wrong.  But it's getting better.

gcc is extremly dumb at that currently. Linux has a lot of inline
functions like

static inline foo(int arg) 
{ 
	if (__builtin_constant_p(arg)) { 
		/* lots of code that checks for arg and does different things */
	} else { 
		/* simple code */
	}
} 

(e.g. take a look at asm/uaccess.h for extreme examples) 

The gcc inliner doesn't know that all the stuff in the constant case
will be optimized away and it assumes the function is big. That's 
really a bug in the inliner.

But even without that it seems to do badly - example is asm/fixmap.h:fix_to_virt()

#define __fix_to_virt(x)        (FIXADDR_TOP - ((x) << PAGE_SHIFT))
static inline unsigned long fix_to_virt(const unsigned int idx)
{
        if (idx >= __end_of_fixed_addresses)
                __this_fixmap_does_not_exist();

        return __fix_to_virt(idx);
}


This three liner is _not_ inlined in current gcc mainline.
I cannot describe this in any other way than badly broken.

> Meanwhile, you should probably distinguish between must-inline,
> should-inline, may-inline, should-not-inline and must-not-inline
> functions.  Attribute always_inline covers the must-inline case; the

You're asking us to do a lot of work just to work around compiler bugs?

I can see the point of having must-inline - that's so rare that
it can be declared by hand. May inline is also done, except
for a few misguided people who use -O3. should not inline seems
like overkill.

-Andi

Re: GCC 3.4 and broken inlining.

[Alexandre Oliva]

On Jul 11, 2004, Andi Kleen <ak@muc.de> wrote:

>> Meanwhile, you should probably distinguish between must-inline,
>> should-inline, may-inline, should-not-inline and must-not-inline
>> functions.  Attribute always_inline covers the must-inline case; the

> You're asking us to do a lot of work just to work around compiler bugs?

Not asking.  Just suggesting that you make your request to the
compiler clearer.  This may enable the compiler to do a better job for
you.  You don't have to switch it all at once.  Keep inline as
always_inline, if you like, and downgrade other inline requests as you
see fit.

Of course having inline expand to something containing always_inline
will take a bit of preprocessor hackery to get other macros to expand
to the inline keyword without this attribute.

> I can see the point of having must-inline - that's so rare that
> it can be declared by hand. May inline is also done, except
> for a few misguided people who use -O3. should not inline seems
> like overkill.

`should not inline' is the default: a function not declared as inline
won't be inlined unless several conditions are met, e.g., compiling
with -O3 and/or -finline-all-functions.  It's the other cases to tune
inlining directives that would be useful.

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: GCC 3.4 and broken inlining.

[Adrian Bunk]

On Fri, Jul 09, 2004 at 06:51:46AM +0200, Andi Kleen wrote:
> Nigel Cunningham <ncunningham@linuxmail.org> writes:
> 
> I think a better solution would be to apply the appended patch 
> 
> And then just mark the function you know needs to be inlined
> as __always_inline__. I did this on x86-64 for some functions
> too that need to be always inlined (although using the attribute
> directly because all x86-64 compilers support it)
> 
> The rationale is that the inlining algorithm in gcc 3.4+ is quite
> a lot better and actually eliminates some not-so-great inlining
> we had in the past and makes the kernel a bit smaller.

Making the kernel a bit smaller is a small improvement.

Runtime errors caused with gcc 3.4 are IMHO much worse than such a small 
improvement or three dozen compile errors with gcc 3.4 .

The effect of your patch with gcc 3.4 would be nearly:
  Ignore all old inlines and add something new that does what inline
  did before.

Wouldn't it be a better solution if you would audit the existing inlines 
in the kernel for abuse of inline and fix those instead?

> -Andi
> 
> P.S.: compiler.h seems to be not "gcc 4.0 safe". Probably that needs
> to be fixed too.
>...

My copy of compiler.h says:

<--  snip  -->

#if __GNUC__ > 3
# include <linux/compiler-gcc+.h>       /* catch-all for GCC 4, 5, etc. */

<--  snip  -->

What exactly is wrong with this?


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

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

> Runtime errors caused with gcc 3.4 are IMHO much worse than such a small 
> improvement or three dozen compile errors with gcc 3.4 .

What runtime errors? 

Actually requiring inlining is extremly rare and such functions should
get that an explicit always inline just for documentation purposes.
(another issue is not optimized away checks, but that shows at link time) 

In the x86-64 case it was vsyscalls, in Nigel's case it was swsusp.
Both are quite exceptional in what they do.

> Wouldn't it be a better solution if you would audit the existing inlines 
> in the kernel for abuse of inline and fix those instead?

I don't see any point in going through ~1.2MLOC of code by hand
when a compiler can do it for me.

-Andi

Re: GCC 3.4 and broken inlining.

[Adrian Bunk]

On Fri, Jul 09, 2004 at 11:54:15PM +0200, Andi Kleen wrote:
> > Runtime errors caused with gcc 3.4 are IMHO much worse than such a small 
> > improvement or three dozen compile errors with gcc 3.4 .
> 
> What runtime errors? 
> 
> Actually requiring inlining is extremly rare and such functions should
> get that an explicit always inline just for documentation purposes.
> (another issue is not optimized away checks, but that shows at link time) 

First of all, your proposed patch seems to be broken WRT gcc < 3.1 .

> In the x86-64 case it was vsyscalls, in Nigel's case it was swsusp.
> Both are quite exceptional in what they do.
> 
> > Wouldn't it be a better solution if you would audit the existing inlines 
> > in the kernel for abuse of inline and fix those instead?
> 
> I don't see any point in going through ~1.2MLOC of code by hand
> when a compiler can do it for me.

How can a compiler decide whether an "inline" was for a possible small  
speed benefit or whether it's required for correct working?

And I'm not that happy with the fact that gcc 3.3 and gcc 3.4 will 
produce significantly different code for the same file. Besides from the  
3 dozen compile errors I'm currently sorting out, gcc 3.3 and 3.4 should 
behave similar with __attribute__((always_inline)).

> -Andi

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

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

On Sat, Jul 10, 2004 at 12:17:55AM +0200, Adrian Bunk wrote:
> On Fri, Jul 09, 2004 at 11:54:15PM +0200, Andi Kleen wrote:
> > > Runtime errors caused with gcc 3.4 are IMHO much worse than such a small 
> > > improvement or three dozen compile errors with gcc 3.4 .
> > 
> > What runtime errors? 
> > 
> > Actually requiring inlining is extremly rare and such functions should
> > get that an explicit always inline just for documentation purposes.
> > (another issue is not optimized away checks, but that shows at link time) 
> 
> First of all, your proposed patch seems to be broken WRT gcc < 3.1 .

The latest version does fallback #define __always_inline inline
That was indeed broken in the original patch.

> 
> > In the x86-64 case it was vsyscalls, in Nigel's case it was swsusp.
> > Both are quite exceptional in what they do.
> > 
> > > Wouldn't it be a better solution if you would audit the existing inlines 
> > > in the kernel for abuse of inline and fix those instead?
> > 
> > I don't see any point in going through ~1.2MLOC of code by hand
> > when a compiler can do it for me.
> 
> How can a compiler decide whether an "inline" was for a possible small  
> speed benefit or whether it's required for correct working?

It can't, but the 0.001% of needed for inlines that are required 
for correctness should be always marked __always_inline
just for documentation alone.
You probably don't realize how special a case this is. 

> And I'm not that happy with the fact that gcc 3.3 and gcc 3.4 will 
> produce significantly different code for the same file. Besides from the  
> 3 dozen compile errors I'm currently sorting out, gcc 3.3 and 3.4 should 
> behave similar with __attribute__((always_inline)).

They are different compiler versiopns, they generate different code.

-Andi

Re: GCC 3.4 and broken inlining.

[Alexandre Oliva]

On Jul  9, 2004, Andi Kleen <ak@muc.de> wrote:

> Nigel Cunningham <ncunningham@linuxmail.org> writes:

> I think a better solution would be to apply the appended patch 

Agreed.  

> And then just mark the function you know needs to be inlined
> as __always_inline__.

It's probably a good idea to define such functions as `extern inline'
(another GCC extension), such that a definition of the function is
never emitted, and you get a linker error if the compiler somehow
fails to emit an error on a failure to inline the function.

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

On Sat, Jul 10, 2004 at 06:25:52PM -0300, Alexandre Oliva wrote:
> > And then just mark the function you know needs to be inlined
> > as __always_inline__.
> 
> It's probably a good idea to define such functions as `extern inline'
> (another GCC extension), such that a definition of the function is
> never emitted, and you get a linker error if the compiler somehow
> fails to emit an error on a failure to inline the function.

That used to be done in the past for all functions, but it was 
stopped because gcc suddenly stopped inlining functions it did previously 
and it caused a lot of spurious linker errors.

I guess it could be readded if the inlining heuristics were fixed,
but even in gcc 3.5 it still looks quite bleak.

-Andi

Re: GCC 3.4 and broken inlining.

[Andrew Morton]

Andi Kleen <ak@muc.de> wrote:
>
> I guess it could be readded if the inlining heuristics were fixed,
>  but even in gcc 3.5 it still looks quite bleak.

It's very simple.  For use in the kernel we don't *want* any inlining
heuristics.  What we want is:

a) If the programmer says "inline", then inline it.

b) If the programmer didn't say "inline" then don't inline it.

Surely it is not hard to add a new option to gcc to provide these semantics?

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

On Sat, Jul 10, 2004 at 11:55:36PM -0700, Andrew Morton wrote:
> Andi Kleen <ak@muc.de> wrote:
> >
> > I guess it could be readded if the inlining heuristics were fixed,
> >  but even in gcc 3.5 it still looks quite bleak.
> 
> It's very simple.  For use in the kernel we don't *want* any inlining
> heuristics.  What we want is:
> 
> a) If the programmer says "inline", then inline it.

The problem is that we have a lot of "stale" inlines. Inlines that 
made sense a long time ago, but then people added a lot more code
to the function and it would be better to out line it again.
You should know, you seem to do this kind of out-lining most ...

For those it may even make sense to let the compiler chose.

> 
> b) If the programmer didn't say "inline" then don't inline it.
> 
> Surely it is not hard to add a new option to gcc to provide these semantics?

That option is -O2 -Dinline="__attribute__((always_inline))"
But for some reason it was turned off for 3.4/3.5.

-Andi

Re: GCC 3.4 and broken inlining.

[Andrew Morton]

Andi Kleen <ak@muc.de> wrote:
>
> On Sat, Jul 10, 2004 at 11:55:36PM -0700, Andrew Morton wrote:
> > Andi Kleen <ak@muc.de> wrote:
> > >
> > > I guess it could be readded if the inlining heuristics were fixed,
> > >  but even in gcc 3.5 it still looks quite bleak.
> > 
> > It's very simple.  For use in the kernel we don't *want* any inlining
> > heuristics.  What we want is:
> > 
> > a) If the programmer says "inline", then inline it.
> 
> The problem is that we have a lot of "stale" inlines. Inlines that 
> made sense a long time ago, but then people added a lot more code
> to the function and it would be better to out line it again.
> You should know, you seem to do this kind of out-lining most ...

We've already fixed zillions of those, and patches are accepted.  I think
someone wrote a tool to hunt those functions down, too.

> For those it may even make sense to let the compiler chose.

We can see how far that's getting us ;)

> > 
> > b) If the programmer didn't say "inline" then don't inline it.
> > 
> > Surely it is not hard to add a new option to gcc to provide these semantics?
> 
> That option is -O2 -Dinline="__attribute__((always_inline))"
> But for some reason it was turned off for 3.4/3.5.
> 

Please tell me that was just a bug, and it will be fixed very soon.

Re: GCC 3.4 and broken inlining.

[Andi Kleen]

On Sun, Jul 11, 2004 at 01:32:18AM -0700, Andrew Morton wrote:
> Andi Kleen <ak@muc.de> wrote:
> >
> > On Sat, Jul 10, 2004 at 11:55:36PM -0700, Andrew Morton wrote:
> > > Andi Kleen <ak@muc.de> wrote:
> > > >
> > > > I guess it could be readded if the inlining heuristics were fixed,
> > > >  but even in gcc 3.5 it still looks quite bleak.
> > > 
> > > It's very simple.  For use in the kernel we don't *want* any inlining
> > > heuristics.  What we want is:
> > > 
> > > a) If the programmer says "inline", then inline it.
> > 
> > The problem is that we have a lot of "stale" inlines. Inlines that 
> > made sense a long time ago, but then people added a lot more code
> > to the function and it would be better to out line it again.
> > You should know, you seem to do this kind of out-lining most ...
> 
> We've already fixed zillions of those, and patches are accepted.  I think
> someone wrote a tool to hunt those functions down, too.

Ok, we'll see.  Fixing that would be good of course.

> > > 
> > > b) If the programmer didn't say "inline" then don't inline it.
> > > 
> > > Surely it is not hard to add a new option to gcc to provide these semantics?
> > 
> > That option is -O2 -Dinline="__attribute__((always_inline))"
> > But for some reason it was turned off for 3.4/3.5.
> > 
> 
> Please tell me that was just a bug, and it will be fixed very soon.

compiler-gcc3.h: 

#if __GNUC_MINOR__ >= 1  && __GNUC_MINOR__ < 4
# define inline         __inline__ __attribute__((always_inline))
# define __inline__     __inline__ __attribute__((always_inline))
# define __inline       __inline__ __attribute__((always_inline))
#endif

iirc the problem was that gcc errors now out when a function
marked like this cannot be inlined for some reason.

-Andi

Re: GCC 3.4 and broken inlining.

[Adrian Bunk]

On Sun, Jul 11, 2004 at 01:32:18AM -0700, Andrew Morton wrote:
> Andi Kleen <ak@muc.de> wrote:
>...
> > > b) If the programmer didn't say "inline" then don't inline it.
> > > 
> > > Surely it is not hard to add a new option to gcc to provide these semantics?
> > 
> > That option is -O2 -Dinline="__attribute__((always_inline))"
> > But for some reason it was turned off for 3.4/3.5.
> > 
> 
> Please tell me that was just a bug, and it will be fixed very soon.


It's a bug in compiler-gcc3.h, and I already sent the patch below in 
this thread.

I'm currently sending fixes for compile errors this causes with gcc 3.4 
(I've forgotten the exact number, but there were compile errors in at 
about 30 files in a full i386 compile).


<--  snip  -->


[patch] #define inline as __attribute__((always_inline)) also for gcc >= 3.4

Rationale:
- if gcc 3.4 can't inline a function marked as "inline" that's a
  strong hint that further investigation is required
- I strongly prefer a compile error over a potential runtime problem


Signed-off-by: Adrian Bunk <bunk@fs.tum.de>

--- linux-2.6.7-mm6-full-gcc3.4/include/linux/compiler-gcc3.h.old	2004-07-08 23:40:27.000000000 +0200
+++ linux-2.6.7-mm6-full-gcc3.4/include/linux/compiler-gcc3.h	2004-07-08 23:40:37.000000000 +0200
@@ -3,7 +3,7 @@
 /* These definitions are for GCC v3.x.  */
 #include <linux/compiler-gcc.h>
 
-#if __GNUC_MINOR__ >= 1  && __GNUC_MINOR__ < 4
+#if __GNUC_MINOR__ >= 1
 # define inline		__inline__ __attribute__((always_inline))
 # define __inline__	__inline__ __attribute__((always_inline))
 # define __inline	__inline__ __attribute__((always_inline))

Re: GCC 3.4 and broken inlining.

[Arnd Bergmann]

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

On Sonntag, 11. Juli 2004 13:50, Adrian Bunk wrote:
> -#if __GNUC_MINOR__ >= 1  && __GNUC_MINOR__ < 4
> +#if __GNUC_MINOR__ >= 1
>  # define inline                __inline__ __attribute__((always_inline))
>  # define __inline__    __inline__ __attribute__((always_inline))
>  # define __inline      __inline__ __attribute__((always_inline))

While we're there, shouldn't this really be the following?

# define inline          inline   __attribute__((always_inline))
# define __inline__    __inline__ __attribute__((always_inline))
# define __inline      __inline   __attribute__((always_inline))

I find it somewhat annoying that the preprocessor expands every "inline"
to "__inline__ __attribute__((always_inline)) __attribute__((always_inline))"
in the current code.

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[updated 2.6 patch] #define inline as __attribute__((always_inline)) also for gcc >= 3.4

[Adrian Bunk]

On Sun, Jul 11, 2004 at 03:01:18PM +0200, Arnd Bergmann wrote:
> On Sonntag, 11. Juli 2004 13:50, Adrian Bunk wrote:
> > -#if __GNUC_MINOR__ >= 1  && __GNUC_MINOR__ < 4
> > +#if __GNUC_MINOR__ >= 1
> >  # define inline                __inline__ __attribute__((always_inline))
> >  # define __inline__    __inline__ __attribute__((always_inline))
> >  # define __inline      __inline__ __attribute__((always_inline))
> 
> While we're there, shouldn't this really be the following?
> 
> # define inline          inline   __attribute__((always_inline))
> # define __inline__    __inline__ __attribute__((always_inline))
> # define __inline      __inline   __attribute__((always_inline))
> 
> I find it somewhat annoying that the preprocessor expands every "inline"
> to "__inline__ __attribute__((always_inline)) __attribute__((always_inline))"
> in the current code.

Sounds like a good idea.

Updated patch below.

> 	Arnd <><

<--  snip  -->

[patch] #define inline as __attribute__((always_inline)) also for gcc >= 3.4


Rationale:
- if gcc 3.4 can't inline a function marked as "inline" that's a
  strong hint that further investigation is required
- I strongly prefer a compile error over a potential runtime problem


Additionally, it changes all #define's to expand to the correct
{,__}inline{,__} (suggested by Arnd Bergmann <arnd@arndb.de>).


Signed-off-by: Adrian Bunk <bunk@fs.tum.de>

--- linux-2.6.7-mm7-full-gcc3.4/include/linux/compiler-gcc3.h.old	2004-07-13 02:56:53.000000000 +0200
+++ linux-2.6.7-mm7-full-gcc3.4/include/linux/compiler-gcc3.h	2004-07-13 02:58:03.000000000 +0200
@@ -3,10 +3,10 @@
 /* These definitions are for GCC v3.x.  */
 #include <linux/compiler-gcc.h>
 
-#if __GNUC_MINOR__ >= 1  && __GNUC_MINOR__ < 4
-# define inline		__inline__ __attribute__((always_inline))
-# define __inline__	__inline__ __attribute__((always_inline))
-# define __inline	__inline__ __attribute__((always_inline))
+#if __GNUC_MINOR__ >= 1
+# define inline		inline		__attribute__((always_inline))
+# define __inline__	__inline__	__attribute__((always_inline))
+# define __inline	__inline	__attribute__((always_inline))
 #endif
 
 #if __GNUC_MINOR__ > 0