Hi Alan,
following is patch which was needed for compiling 2.4.7-ac1
on box equipped with 'gcc version 3.0.1 20010721 (Debian prerelease)'.
As I did not see such complaint yet - here it is.
Patch does NOT change all extern inline -> static inline, but only
changes extern -> static on functions which were not inlined and
due to which build failed (except one of get_pgd_slow, but I changed
both just for symmetry). There is high probability that other
sound drivers are affected too...
If you think that gcc is too lazy on inlining (I think so...),
tell me and I'll complain to gcc team instead of here.
Best regards,
Petr Vandrovec
[email protected]
diff -urdN linux/drivers/sound/es1370.c linux/drivers/sound/es1370.c
--- linux/drivers/sound/es1370.c Thu Jul 26 15:46:55 2001
+++ linux/drivers/sound/es1370.c Thu Jul 26 16:53:41 2001
@@ -649,7 +649,7 @@
return diff;
}
-extern inline void clear_advance(void *buf, unsigned bsize, unsigned bptr, unsigned len, unsigned char c)
+static inline void clear_advance(void *buf, unsigned bsize, unsigned bptr, unsigned len, unsigned char c)
{
if (bptr + len > bsize) {
unsigned x = bsize - bptr;
diff -urdN linux/drivers/sound/es1371.c linux/drivers/sound/es1371.c
--- linux/drivers/sound/es1371.c Thu Jul 26 15:46:55 2001
+++ linux/drivers/sound/es1371.c Thu Jul 26 16:53:31 2001
@@ -983,7 +983,7 @@
return diff;
}
-extern inline void clear_advance(void *buf, unsigned bsize, unsigned bptr, unsigned len, unsigned char c)
+static inline void clear_advance(void *buf, unsigned bsize, unsigned bptr, unsigned len, unsigned char c)
{
if (bptr + len > bsize) {
unsigned x = bsize - bptr;
diff -urdN linux/drivers/sound/esssolo1.c linux/drivers/sound/esssolo1.c
--- linux/drivers/sound/esssolo1.c Sun Jul 15 23:22:23 2001
+++ linux/drivers/sound/esssolo1.c Thu Jul 26 16:54:13 2001
@@ -480,7 +480,7 @@
return 0;
}
-extern inline int prog_dmabuf_adc(struct solo1_state *s)
+static inline int prog_dmabuf_adc(struct solo1_state *s)
{
unsigned long va;
int c;
@@ -508,7 +508,7 @@
return 0;
}
-extern inline int prog_dmabuf_dac(struct solo1_state *s)
+static inline int prog_dmabuf_dac(struct solo1_state *s)
{
unsigned long va;
int c;
@@ -531,7 +531,7 @@
return 0;
}
-extern inline void clear_advance(void *buf, unsigned bsize, unsigned bptr, unsigned len, unsigned char c)
+static inline void clear_advance(void *buf, unsigned bsize, unsigned bptr, unsigned len, unsigned char c)
{
if (bptr + len > bsize) {
unsigned x = bsize - bptr;
diff -urdN linux/include/asm-i386/pgalloc.h linux/include/asm-i386/pgalloc.h
--- linux/include/asm-i386/pgalloc.h Thu Jul 26 15:46:58 2001
+++ linux/include/asm-i386/pgalloc.h Thu Jul 26 16:33:44 2001
@@ -29,7 +29,7 @@
extern void init_pae_pgd_cache(void);
-extern __inline__ pgd_t *get_pgd_slow(void)
+static __inline__ pgd_t *get_pgd_slow(void)
{
int i;
pgd_t *pgd = kmem_cache_alloc(pae_pgd_cachep, GFP_KERNEL);
@@ -54,7 +54,7 @@
#else
-extern __inline__ pgd_t *get_pgd_slow(void)
+static __inline__ pgd_t *get_pgd_slow(void)
{
pgd_t *pgd = (pgd_t *)__get_free_page(GFP_KERNEL);
diff -urdN linux/include/asm-i386/siginfo.h linux/include/asm-i386/siginfo.h
--- linux/include/asm-i386/siginfo.h Fri Jul 20 19:52:18 2001
+++ linux/include/asm-i386/siginfo.h Thu Jul 26 16:33:01 2001
@@ -216,7 +216,7 @@
#ifdef __KERNEL__
#include <linux/string.h>
-extern inline void copy_siginfo(siginfo_t *to, siginfo_t *from)
+static inline void copy_siginfo(siginfo_t *to, siginfo_t *from)
{
if (from->si_code < 0)
memcpy(to, from, sizeof(siginfo_t));
diff -urdN linux/net/core/rtnetlink.c linux/net/core/rtnetlink.c
--- linux/net/core/rtnetlink.c Mon Feb 28 02:45:10 2000
+++ linux/net/core/rtnetlink.c Thu Jul 26 16:27:03 2001
@@ -274,7 +274,7 @@
/* Process one rtnetlink message. */
-extern __inline__ int
+static __inline__ int
rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, int *errp)
{
struct rtnetlink_link *link;
> following is patch which was needed for compiling 2.4.7-ac1
> on box equipped with 'gcc version 3.0.1 20010721 (Debian prerelease)'.
> As I did not see such complaint yet - here it is.
> If you think that gcc is too lazy on inlining (I think so...),
> tell me and I'll complain to gcc team instead of here.
Fix gcc. We use extern inline to say 'must be inlined' and that was the
semantic it used to have. Some of our inlines will not work if the compiler
uninlines them.
Alan
> tell me and I'll complain to gcc team instead of here.
PS: If in fact there is a gcc 3.0.1 -fsomething option we should be forcing
on instead then that would be even better
Alan
On 26 Jul 01 at 18:52, Alan Cox wrote:
> > following is patch which was needed for compiling 2.4.7-ac1
> > on box equipped with 'gcc version 3.0.1 20010721 (Debian prerelease)'.
> > As I did not see such complaint yet - here it is.
> > If you think that gcc is too lazy on inlining (I think so...),
> > tell me and I'll complain to gcc team instead of here.
>
> Fix gcc. We use extern inline to say 'must be inlined' and that was the
> semantic it used to have. Some of our inlines will not work if the compiler
> uninlines them.
Just adding '-finline-limit=150' fixes all of them (critical limit
is somewhere between 120 and 150 on my kernel). As '-finline-limit'
is documented as being 10000 by default, it looks like that someone
changed default value to some really unreasonable value (probably 100).
Best regards,
Petr Vandrovec
[email protected]
In article <[email protected]>,
Alan Cox <[email protected]> wrote:
>> following is patch which was needed for compiling 2.4.7-ac1
>> on box equipped with 'gcc version 3.0.1 20010721 (Debian prerelease)'.
>> As I did not see such complaint yet - here it is.
>> If you think that gcc is too lazy on inlining (I think so...),
>> tell me and I'll complain to gcc team instead of here.
>
>Fix gcc. We use extern inline to say 'must be inlined' and that was the
>semantic it used to have. Some of our inlines will not work if the compiler
>uninlines them.
No, we had this fight with the gcc people a few years back, and they
have a very valid argument for the current semantics
- "static inline" means "we have to have this function, if you use it
but don't inline it, then make a static version of it in this
compilation unit"
- "extern inline" means "I actually _have_ an extern for this function,
but if you want to inline it, here's the inline-version"
The only problem with "static inline" was some _really_ old gcc versions
that did the wrong thing and made a static version of the function in
_every_ compilation unit, whether it was needed or not. Those versions of
gcc do not work on the kernel anyway these days, so..
I think the current gcc semantics are (a) more powerful than the old one
and (b) have been in effect long enough that it's not painful for Linux
to just switch over to them. In short, we might actually want to start
taking advantage of them, and even if we don't we should just convert
all current users of "extern inline" to "static inline".
Linus
On Thu, Jul 26, 2001 at 08:28:32PM +0000, Petr Vandrovec wrote:
> Just adding '-finline-limit=150' fixes all of them (critical limit
> is somewhere between 120 and 150 on my kernel). As '-finline-limit'
> is documented as being 10000 by default, it looks like that someone
> changed default value to some really unreasonable value (probably 100).
Yes. The higher value resulted in much compile-time lossage on
heavily templated c++ code, as it proceeded to inline everything
in sight.
While we may not use 100 in the final 3.0.1, it will definitely
be much lower than 10000. More intelligent heuristics will have
to wait for 3.1 or something.
r~
In article <[email protected]>,
Richard Henderson <[email protected]> wrote:
>On Thu, Jul 26, 2001 at 08:28:32PM +0000, Petr Vandrovec wrote:
>> Just adding '-finline-limit=150' fixes all of them (critical limit
>> is somewhere between 120 and 150 on my kernel). As '-finline-limit'
>> is documented as being 10000 by default, it looks like that someone
>> changed default value to some really unreasonable value (probably 100).
>
>Yes. The higher value resulted in much compile-time lossage on
>heavily templated c++ code, as it proceeded to inline everything
>in sight.
Having seen the discussion on the gcc lists, I can only heartily
approve.
I did some repmacement of "extern" into "static" in 2.4.8-pre1, but I
don't have gcc-3.0.x on my machines (too many headaches, too little
time).
Linus
In LKML, you wrote:
> I did some repmacement of "extern" into "static" in 2.4.8-pre1, but
> I don't have gcc-3.0.x on my machines (too many headaches, too
> little time).
For gcc version 2.96 20000731 (Red Hat Linux 7.1 2.96-85), I had to
change the "static"s in include/linux/parport_pc.h back to "extern"s
to get drivers/parport/parport_pc.c to compile (as a module).
Cheers, Wayne
On Thu, 26 Jul 2001, Linus Torvalds wrote:
> - "static inline" means "we have to have this function, if you use it
> but don't inline it, then make a static version of it in this
> compilation unit"
>
> - "extern inline" means "I actually _have_ an extern for this function,
> but if you want to inline it, here's the inline-version"
>
[SNIP]
> ... we should just convert
> all current users of "extern inline" to "static inline".
>
Doesn't work for the ones in include/linux/parport_pc.h, which have
extern versions in drivers/parport/parport_pc.c. Gives build errors.
--
Niels Kristian Bech Jensen -- [email protected] -- http://www.image.dk/~nkbj/
----------->> Stop software piracy --- use free software! <<-----------
On Fri, Jul 27, 2001 at 08:55:52AM +0200, Niels Kristian Bech Jensen wrote:
> On Thu, 26 Jul 2001, Linus Torvalds wrote:
>
> > - "static inline" means "we have to have this function, if you use it
> > but don't inline it, then make a static version of it in this
> > compilation unit"
> >
> > - "extern inline" means "I actually _have_ an extern for this function,
> > but if you want to inline it, here's the inline-version"
> >
> [SNIP]
> > ... we should just convert
> > all current users of "extern inline" to "static inline".
> >
> Doesn't work for the ones in include/linux/parport_pc.h, which have
> extern versions in drivers/parport/parport_pc.c. Gives build errors.
Is there any reason against just removing these functions from
drivers/parport/parport_pc.c? I do not see one. After all, they are
just duplicates.
Robert
--
Robert Schiele mailto:[email protected]
Tel./Fax: +49-621-10059 http://webrum.uni-mannheim.de/math/rschiele/
"Petr Vandrovec" <[email protected]> writes:
> Just adding '-finline-limit=150' fixes all of them
This is not a fix, this is a workaround which is suitable for some
specific GCC release(s). The optimization decisions surrounding
inlining are likely to change again, so this will break almost
certainly in the future.
At least one GCC frontend has got a pragma called Always_Inline. This
seems the way to go. For the C frontend, you would use a function
attribute, of course.
--
Florian Weimer [email protected]
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898
On 27 Jul 01 at 18:03, Florian Weimer wrote:
> "Petr Vandrovec" <[email protected]> writes:
>
> > Just adding '-finline-limit=150' fixes all of them
>
> This is not a fix, this is a workaround which is suitable for some
> specific GCC release(s). The optimization decisions surrounding
> inlining are likely to change again, so this will break almost
> certainly in the future.
I found that main problem is in __constant_memcpy. This is very large
in internal representation (~70 'units'), so any kernel function which
contains two memcpy calls with constant length cannot be inlined with
current settings because of it contains 140+ internal operations - although
if compiler then generates static function (instead of inlined), it is ~10
i386 operations long, from which 4 are push %edi/%esi and pop %esi/%edi...
Best regards,
Petr Vandrovec
[email protected]