GCC 3.4 and broken inlining.

GCC 3.4 and broken inlining.

[Nigel Cunningham]

Hi.

In response to a user report that suspend2 was broken when compiled with
gcc 3.4, I upgraded my compiler to 3.4.1-0.1mdk. I've found that the
restore_processor_context, defined as follows:

static inline void restore_processor_context(void)

doesn't get inlined. GCC doesn't complain when compiling the file, and
so far as I can see, there's no reason for it not to inline the routine.
But that leaves me confused because some other inlined functions do seem
to get inlined. Can someone tell me what I'm missing?

Regards,

Nigel

Re: GCC 3.4 and broken inlining.

[Jakub Jelinek]

On Thu, Jul 08, 2004 at 09:46:39PM +1000, Nigel Cunningham wrote:
> In response to a user report that suspend2 was broken when compiled with
> gcc 3.4, I upgraded my compiler to 3.4.1-0.1mdk. I've found that the
> restore_processor_context, defined as follows:
> 
> static inline void restore_processor_context(void)
> 
> doesn't get inlined. GCC doesn't complain when compiling the file, and
> so far as I can see, there's no reason for it not to inline the routine.

Try passing -Winline, it will tell you when a function marked inline is not
actually inlined.
Presence of inline keyword is not a guarantee the function will not be
inlined, it is a hint to the compiler.
GCC 3.4 is much bettern than earlier 3.x GCCs in actually inlining functions
marked as inline, but there are still cases when it decides not to inline
for various reasons.  E.g. in C++ world, lots of things are inline, yet
honoring that everywhere would mean very inefficient huge programs.
If a function relies for correctness on being inlined, then it should use
inline __attribute__((always_inline)).

	Jakub

Re: GCC 3.4 and broken inlining.

[Nigel Cunningham]

Hi.

On Thu, 2004-07-08 at 22:07, Jakub Jelinek wrote:
> Try passing -Winline, it will tell you when a function marked inline is not
> actually inlined.

That's what I was using; no error message is printed. It is for other
functions.

> Presence of inline keyword is not a guarantee the function will not be
> inlined, it is a hint to the compiler.


> GCC 3.4 is much bettern than earlier 3.x GCCs in actually inlining functions
> marked as inline, but there are still cases when it decides not to inline
> for various reasons.  E.g. in C++ world, lots of things are inline, yet
> honoring that everywhere would mean very inefficient huge programs.
> If a function relies for correctness on being inlined, then it should use
> inline __attribute__((always_inline)).

Okay. I'll do that then.

Thanks.

Nigel

Re: GCC 3.4 and broken inlining.

[Adrian Bunk]

On Thu, Jul 08, 2004 at 08:07:19AM -0400, Jakub Jelinek wrote:
> On Thu, Jul 08, 2004 at 09:46:39PM +1000, Nigel Cunningham wrote:
> > In response to a user report that suspend2 was broken when compiled with
> > gcc 3.4, I upgraded my compiler to 3.4.1-0.1mdk. I've found that the
> > restore_processor_context, defined as follows:
> > 
> > static inline void restore_processor_context(void)
> > 
> > doesn't get inlined. GCC doesn't complain when compiling the file, and
> > so far as I can see, there's no reason for it not to inline the routine.
> 
> Try passing -Winline, it will tell you when a function marked inline is not
> actually inlined.
> Presence of inline keyword is not a guarantee the function will not be
> inlined, it is a hint to the compiler.
> GCC 3.4 is much bettern than earlier 3.x GCCs in actually inlining functions
> marked as inline, but there are still cases when it decides not to inline
> for various reasons.  E.g. in C++ world, lots of things are inline, yet
> honoring that everywhere would mean very inefficient huge programs.
> If a function relies for correctness on being inlined, then it should use
> inline __attribute__((always_inline)).

include/linux/compiler-gcc3.h says:

<--  snip  -->

#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

<--  snip  -->


@Arjan:
This was added as part of your
  [PATCH] ia32: 4Kb stacks (and irqstacks) patch
What's the recommended solution for Nigel's problem?


> 	Jakub

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.

[Arjan van de Ven]

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

On Thu, Jul 08, 2004 at 10:52:25PM +0200, Adrian Bunk wrote:
> > marked as inline, but there are still cases when it decides not to inline
> > for various reasons.  E.g. in C++ world, lots of things are inline, yet
> > honoring that everywhere would mean very inefficient huge programs.
> > If a function relies for correctness on being inlined, then it should use
> > inline __attribute__((always_inline)).
> 
> include/linux/compiler-gcc3.h says:
> 
> <--  snip  -->
> 
> #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
> 
> <--  snip  -->
> 
> 
> @Arjan:
> This was added as part of your
>   [PATCH] ia32: 4Kb stacks (and irqstacks) patch
> What's the recommended solution for Nigel's problem?

the problem I've seen is that when gcc doesn't honor normal inline, it will
often error out if you always inline....
I'm open to removing the < 4 but as jakub said, 3.4 is quit good at honoring
normal inline, and when it doesn't there often is a strong reason.....

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

Re: GCC 3.4 and broken inlining.

[Nigel Cunningham]

On Fri, 2004-07-09 at 07:36, Denis Vlasenko wrote:
> It was decided to #define inline so that it means always_inline for lk.
> Dunno why include/linux/compiler-gcc3.h stopped doing that
> specifically for gcc 3.4...

I tried getting it to use the always_inline definition for gcc 3.4. It
resulted in the compilation failing in a number of places. The fixes
were generally trivial, involving rearranging the contents of files so
that inline function bodies appear before routines calling them, or
removing the inline where this isn't possible. IMHO, this is what should
be done. I didn't complete the changes, however: I thought I'd try for a
simpler solution, just in case I'm wrong.

Regards,

Nigel

Re: GCC 3.4 and broken inlining.

[Nigel Cunningham]

Hi.

On Fri, 2004-07-09 at 07:09, Arjan van de Ven wrote:
> the problem I've seen is that when gcc doesn't honor normal inline, it will
> often error out if you always inline....
> I'm open to removing the < 4 but as jakub said, 3.4 is quit good at honoring
> normal inline, and when it doesn't there often is a strong reason.....

I'm busy for the next couple of days, but if you want, I'll make
allyesconfig next week and go through fixing the compilation errors so
that the < 4 can be removed. Rearranging code so that inline functions
are defined before they're called or not declared inline if they can't
always be inlined seems to me to be the right thing to do. (Feel free to
say I'm wrong!).

Regards,

Nigel

[2.6 patch] Re: GCC 3.4 and broken inlining.

[Adrian Bunk]

On Thu, Jul 08, 2004 at 11:09:26PM +0200, Arjan van de Ven wrote:
> On Thu, Jul 08, 2004 at 10:52:25PM +0200, Adrian Bunk wrote:
> > > marked as inline, but there are still cases when it decides not to inline
> > > for various reasons.  E.g. in C++ world, lots of things are inline, yet
> > > honoring that everywhere would mean very inefficient huge programs.
> > > If a function relies for correctness on being inlined, then it should use
> > > inline __attribute__((always_inline)).
> > 
> > include/linux/compiler-gcc3.h says:
> > 
> > <--  snip  -->
> > 
> > #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
> > 
> > <--  snip  -->
> > 
> > 
> > @Arjan:
> > This was added as part of your
> >   [PATCH] ia32: 4Kb stacks (and irqstacks) patch
> > What's the recommended solution for Nigel's problem?
> 
> the problem I've seen is that when gcc doesn't honor normal inline, it will
> often error out if you always inline....
> I'm open to removing the < 4 but as jakub said, 3.4 is quit good at honoring
> normal inline, and when it doesn't there often is a strong reason.....


I'm suggesting the following patch to remove the < 4 :


#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.

[Adrian Bunk]

On Fri, Jul 09, 2004 at 08:08:21AM +1000, Nigel Cunningham wrote:

> Hi.

Hi Nigel,

> On Fri, 2004-07-09 at 07:09, Arjan van de Ven wrote:
> > the problem I've seen is that when gcc doesn't honor normal inline, it will
> > often error out if you always inline....
> > I'm open to removing the < 4 but as jakub said, 3.4 is quit good at honoring
> > normal inline, and when it doesn't there often is a strong reason.....
> 
> I'm busy for the next couple of days, but if you want, I'll make
> allyesconfig next week and go through fixing the compilation errors so
> that the < 4 can be removed. Rearranging code so that inline functions
> are defined before they're called or not declared inline if they can't
> always be inlined seems to me to be the right thing to do. (Feel free to
> say I'm wrong!).

I'm currently working on fixing the compile errors and I plan to send 
some fixes later.

> Regards,
> 
> Nigel

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.

[Nigel Cunningham]

Hi.

On Fri, 2004-07-09 at 08:25, Adrian Bunk wrote:
> I'm currently working on fixing the compile errors and I plan to send 
> some fixes later.

Great! Thanks!

Nigel

Re: GCC 3.4 and broken inlining.

[Zan Lynx]

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

On Thu, 2004-07-08 at 16:00, Nigel Cunningham wrote:
> On Fri, 2004-07-09 at 07:36, Denis Vlasenko wrote:
> > It was decided to #define inline so that it means always_inline for lk.
> > Dunno why include/linux/compiler-gcc3.h stopped doing that
> > specifically for gcc 3.4...
> 
> I tried getting it to use the always_inline definition for gcc 3.4. It
> resulted in the compilation failing in a number of places. The fixes
> were generally trivial, involving rearranging the contents of files so
> that inline function bodies appear before routines calling them, or
> removing the inline where this isn't possible. IMHO, this is what should
> be done. I didn't complete the changes, however: I thought I'd try for a
> simpler solution, just in case I'm wrong.

I believe that just adding -funit-at-a-time as a compile option solves
the problems with inline function body ordering.
-- 
Zan Lynx <zlynx@acm.org>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: GCC 3.4 and broken inlining.

[Arjan van de Ven]

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

On Fri, Jul 09, 2004 at 08:08:21AM +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Fri, 2004-07-09 at 07:09, Arjan van de Ven wrote:
> > the problem I've seen is that when gcc doesn't honor normal inline, it will
> > often error out if you always inline....
> > I'm open to removing the < 4 but as jakub said, 3.4 is quit good at honoring
> > normal inline, and when it doesn't there often is a strong reason.....
> 
> I'm busy for the next couple of days, but if you want, I'll make
> allyesconfig next week and go through fixing the compilation errors so
> that the < 4 can be removed. Rearranging code so that inline functions
> are defined before they're called or not declared inline if they can't
> always be inlined seems to me to be the right thing to do. (Feel free to
> say I'm wrong!).

one thing to note is that you also need to monitor stack usage then :)
inlining somewhat blows up stack usage so do monitor it...

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

Re: GCC 3.4 and broken inlining.

[Arjan van de Ven]

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


> 
> I believe that just adding -funit-at-a-time as a compile option solves
> the problems with inline function body ordering.

... except that -funit-at-a-time causes some functions to use more than
4Kb of *extra* stack, even without CONFIG_4KSTACKS that's a ticking
timebomb of enormous magnitude..

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: GCC 3.4 and broken inlining.

[Adrian Bunk]

On Fri, Jul 09, 2004 at 08:24:03AM +0200, Arjan van de Ven wrote:
> On Fri, Jul 09, 2004 at 08:08:21AM +1000, Nigel Cunningham wrote:
> > Hi.
> > 
> > On Fri, 2004-07-09 at 07:09, Arjan van de Ven wrote:
> > > the problem I've seen is that when gcc doesn't honor normal inline, it will
> > > often error out if you always inline....
> > > I'm open to removing the < 4 but as jakub said, 3.4 is quit good at honoring
> > > normal inline, and when it doesn't there often is a strong reason.....
> > 
> > I'm busy for the next couple of days, but if you want, I'll make
> > allyesconfig next week and go through fixing the compilation errors so
> > that the < 4 can be removed. Rearranging code so that inline functions
> > are defined before they're called or not declared inline if they can't
> > always be inlined seems to me to be the right thing to do. (Feel free to
> > say I'm wrong!).
> 
> one thing to note is that you also need to monitor stack usage then :)
> inlining somewhat blows up stack usage so do monitor it...

How could inlining increase stack usage?

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.

[William Lee Irwin III]

On Fri, Jul 09, 2004 at 08:24:03AM +0200, Arjan van de Ven wrote:
>> one thing to note is that you also need to monitor stack usage then :)
>> inlining somewhat blows up stack usage so do monitor it...

On Sat, Jul 10, 2004 at 03:21:17AM +0200, Adrian Bunk wrote:
> How could inlining increase stack usage?

As more variables go into scope, gcc's stack slot allocation bug bites
progressively harder and stackspace requirements grow without bound.


-- wli

Re: GCC 3.4 and broken inlining.

[Arjan van de Ven]

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

On Sat, Jul 10, 2004 at 03:21:17AM +0200, Adrian Bunk wrote:
> > one thing to note is that you also need to monitor stack usage then :)
> > inlining somewhat blows up stack usage so do monitor it...
> 
> How could inlining increase stack usage?

void foo1(void)
{
	char array[200];
	do_something(array);
}

void foo2(void)
{
	char other_array[200];
	do_somethingelse(other_array);
}

void function_to_which_they_inline(void)
{
	foo1();
	foo2();
}

(assume the do_* functions get inlined into foo or are defines or whatever)

without inlining it's clear that the max stack usage is 200, the lifetimes
of the 2 arrays are 100% exclusive.

With inlining, gcc reorders instructions in it's optimisation passes, and as
a result the lifetimes of the 2 arrays no longer are exclusive and as a
result gcc has no choice to have both separately on the stack.


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

Re: GCC 3.4 and broken inlining.

[Alexandre Oliva]

On Jul  8, 2004, Arjan van de Ven <arjanv@redhat.com> wrote:

> the problem I've seen is that when gcc doesn't honor normal inline, it will
> often error out if you always inline....

Because the always_inline was designed to mark functions that *must*
be inlined otherwise the program breaks (e.g., early dynamic loader
code where you still can't do function calls), not as an
`I'd-really-like-this-to-be-inlined-dammit' flag.

We could add another attribute/keyword/whatever to give a stronger
inline hint, but that would still leave room for the compiler to
decide it can't inline the function for whatever reason.  So you have
to know what to demand from the compiler:
inline-or-fail-because-there's-no-point-otherwise (attribute
always_inline) or inline-if-it-makes-the-program-faster (inline
keyword).  The latter is obviously based on heuristics, so it
sometimes gets things wrong, especially when inlining would enable a
lot of simplification that the compiler can't foresee without actually
doing the work and somehow backtracking if it turns out to not be
profitable (which would still be a guess).

-- 
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.

[Alexandre Oliva]

On Jul  9, 2004, Arjan van de Ven <arjanv@redhat.com> wrote:

>> 
>> I believe that just adding -funit-at-a-time as a compile option solves
>> the problems with inline function body ordering.

> ... except that -funit-at-a-time causes some functions to use more than
> 4Kb of *extra* stack, even without CONFIG_4KSTACKS that's a ticking
> timebomb of enormous magnitude..

But that's because of excessive inlining.  I suppose the abuse of
attribute always_inline may very well be the culprit.

-- 
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.

[Timothy Miller]

William Lee Irwin III wrote:
> On Fri, Jul 09, 2004 at 08:24:03AM +0200, Arjan van de Ven wrote:
> 
>>>one thing to note is that you also need to monitor stack usage then :)
>>>inlining somewhat blows up stack usage so do monitor it...
> 
> 
> On Sat, Jul 10, 2004 at 03:21:17AM +0200, Adrian Bunk wrote:
> 
>>How could inlining increase stack usage?
> 
> 
> As more variables go into scope, gcc's stack slot allocation bug bites
> progressively harder and stackspace requirements grow without bound.


Blah... you should see what Sun's compiler does with volatiles.

Imagine you have some pointers like this:

volatile int *a, *b, *c, *d, *e;

And they are all valid pointers.

And then you have an expression like this:

x = ((((*a + *b) + *c) + *d) + *e);

Since *a and *b are volatile, the Sun compiler thinks that the sum of 
the two is also volatile, allocates stack space for it.  It computes (*a 
+ *b), stores it on the stack, and then loads it back from the stack, 
and then computes that plus *c, stores that result on the stack, then 
reloads it, etc.

I had a case where pointers had to be volatile, because I needed the 
memory space (over PCI) read at the right point in the code, and I 
needed to do some math on what was read.  I had 32 lines of code each of 
which got allocated 5 temporary variables on the stack, for absolutely 
no good reason.  The solution was to cast away the volatile a la ((int)*a).

Now, we all know that it makes no sense for the sum of two volatiles to 
also be volatile.  Once *a and *b are dereferenced and their sum 
computed, the sum isn't going to change, and it isn't even an lvalue, so 
nothing can modify it!

So, you want to talk about bugs.... give GCC a little slack.  :)