Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Petr Vandrovec]

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
                                                vandrove@vc.cvut.cz
                                                

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Richard Henderson]

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~

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Linus Torvalds]

In article <20010726175735.A20320@twiddle.net>,
Richard Henderson  <rth@twiddle.net> 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

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Wayne Whitney]

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

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Florian Weimer]

"Petr Vandrovec" <VANDROVE@vc.cvut.cz> 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 	                  Florian.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart           http://cert.uni-stuttgart.de/
RUS-CERT                          +49-711-685-5973/fax +49-711-685-5898

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Petr Vandrovec]

On 27 Jul 01 at 18:03, Florian Weimer wrote:
> "Petr Vandrovec" <VANDROVE@vc.cvut.cz> 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
                                                    vandrove@vc.cvut.cz
                                                    

[PATCH] gcc-3.0.1 and 2.4.7-ac1

[Petr Vandrovec]

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
						vandrove@vc.cvut.cz


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;

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Alan Cox]

>   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

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Linus Torvalds]

In article <E15PpJg-0004D5-00@the-village.bc.nu>,
Alan Cox  <alan@lxorguk.ukuu.org.uk> 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

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Niels Kristian Bech Jensen]

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 -- nkbj@image.dk -- http://www.image.dk/~nkbj/

----------->>  Stop software piracy --- use free software!  <<-----------

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Robert Schiele]

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

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:rschiele@uni-mannheim.de
Tel./Fax: +49-621-10059		http://webrum.uni-mannheim.de/math/rschiele/

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

Re: [PATCH] gcc-3.0.1 and 2.4.7-ac1

[Alan Cox]

> 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