GCC 3.4 and broken inlining.
Nigel Cunningham
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Jakub Jelinek
> 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
Nigel Cunningham
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
Adrian Bunk
> 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
Arjan van de Ven
> > 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.....
Nigel Cunningham
> 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
Nigel Cunningham
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!).
Adrian Bunk
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 <bu...@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))
-
Adrian Bunk
> 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
-
Zan Lynx
> 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 <zl...@acm.org>
Nigel Cunningham
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
Andi Kleen
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
Nigel Cunningham
On Fri, 2004-07-09 at 14:51, Andi Kleen wrote:
> Nigel Cunningham <ncunn...@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
Andi Kleen
> Hi.
>
> On Fri, 2004-07-09 at 14:51, Andi Kleen wrote:
> > Nigel Cunningham <ncunn...@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
Arjan van de Ven
>
> 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...
Arjan van de Ven
>
> 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..
Michael Buesch
Hash: SHA1
Quoting Andi Kleen <a...@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-----
Andi Kleen
>
> Do I miss something?
Yes. Take a look at arch/i386/Makefile
-Andi
Paweł Sikora
> Quoting Andi Kleen <a...@muc.de>:
> > It's too bad that i386 doesn't enable -funit-at-a-time, that improves
> > the inlining heuristics greatly.
>
>
> -O2 turns on all optimization flags specified by -O. It
> also turns on the following optimization flags: -fforce-mem
> -fcse-skip-blocks -frerun-cse-after-loop -frerun-loop-opt
> -fgcse -fgcse-lm -fgcse-sm -fgcse-las -fdelete-null-pointer-checks
> -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
> ^^^^^^^^^^^^^^^^
>
> 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)
Adrian Bunk
> Nigel Cunningham <ncunn...@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
-
Andi Kleen
> 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
Adrian Bunk
> > 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
-
Adrian Bunk
How could inlining increase stack usage?
William Lee Irwin III
>> 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
Robert Hancock
on architectures where debugging is possible without a frame pointer. If you
use the actual -fomit-frame-pointer option, it is always omitted.
Andi Kleen
> 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
Arjan van de Ven
> > 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.
Alexandre Oliva
> 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}
Alexandre Oliva
But that's because of excessive inlining. I suppose the abuse of
attribute always_inline may very well be the culprit.
Alexandre Oliva
> Nigel Cunningham <ncunn...@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}
Alexandre Oliva
> 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}
Andi Kleen
> On Jul 9, 2004, Nigel Cunningham <ncunn...@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
Andi Kleen
> > 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
Andrew Morton
>
> 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?
Andi Kleen
> Andi Kleen <a...@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
Andrew Morton
>
> On Sat, Jul 10, 2004 at 11:55:36PM -0700, Andrew Morton wrote:
> > Andi Kleen <a...@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.
Andi Kleen
> Andi Kleen <a...@muc.de> wrote:
> >
> > On Sat, Jul 10, 2004 at 11:55:36PM -0700, Andrew Morton wrote:
> > > Andi Kleen <a...@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
Adrian Bunk
> > > 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 <bu...@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))
-
Arnd Bergmann
> -#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 <><
Adrian Bunk
> On Sonntag, 11. Juli 2004 13:50, Adrian Bunk wrote:
> > -#if __GNUC_MINOR__ >= 1 && __GNUC_MINOR__ < 4
> > +#if __GNUC_MINOR__ >= 1
_inline__ __attribute__((always_inline))
> > # define __inline__ __inline__ __attribute__((always_=
inline))
> > # define __inline __inline__ __attribute__((alw=
ays_inline))
>
> While we're there, shouldn't this really be the following?
>
ys_inline))
> # define __inline__ __inline__ __attribute__((always_inlin=
e))
> # define __inline __inline __attribute__((always_i=
nline))
>
> I find it somewhat annoying that the preprocessor expands every "inli=
ne"
> to "__inline__ __attribute__((always_inline)) __attribute__((always_i=
nline))"
> 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 <ar...@arndb.de>).
Signed-off-by: Adrian Bunk <bu...@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-1=
3 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel"=
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. :)
Alexandre Oliva
>> 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}