On Sam, 2008-03-01 at 14:16 +0100, Erik Tromp wrote:
> > Which hardware/CPU BTW?
> 
> CPU Type:	Mobile DualCore Intel Core 2 Duo T5600, 1833 MHz (11 x 167)

i686 or x86_64.
Either way, "usual hardware";-)

> gcc version 3.3.5 (Debian 1:3.3.5-13)
> 
> > That is very probably triggered by the "-finline-limit=50" + 
> > "-Winline".
> 
> You are right.
>
> > And
> > ----  snip  ----
> > #define LISTNODE2STRUCT(funcname, structname, listnodename) \ 
> > static inline structname * funcname (struct list_node *ptr)\ {\
> >   if (ptr) {\ 
> >     return((structname *) (((olsr_u8_t *) ptr) - 
> > offsetof(structname, listnodename))); \
> >   } \
> >   return(NULL); \
> > }
> > ----  snip  ----
> > apparently may generate more the "50" (whatever units gcc 
> > defines this to bet).
> > Though I can't really understand why (basically) "return ptr != NULL ?
> > ptr - constant : NULL" should generate that much code.
> > 
> > Solutions (in arbitrary order):
> > - Removing these options shouldn't hurt the generated binary. I put it
> >   in to catch too large inline functions.
> > - remove the "if (ptr)" safety net from the macro and push that
> >   responsibility to the caller.
> >   Personally I'm not a great fan of the "the code checks for NULL and
> >   fail gracefully silent" as it makes the code explode without cause.
> >   But I think I know why it's in there.
> > - Make the number "50" larger.
I forgot one:
- replace the inline function with a macro
But - given type checks and type safety on inline functions - inline
functions are better than macros.

> I did some tests.
> 
> The lowest value for inline-limit that worked for me was 180.
> 
> Then I replaced the explicit if-statement by a conditional expression, as
> follows:
> 
> #define LISTNODE2STRUCT(funcname, structname, listnodename) \
> static inline structname * funcname (struct list_node *ptr)\
> {\
>   return( \
>     ptr != 0 ? \
>       (structname *) (((olsr_u8_t *) ptr) - offsetof(structname, listnodename)) : \
>       NULL); \
> }
> 
> With that in place, the lowest value for inline-limit that worked was 80.
> 
> Maybe a bug in gcc ?

IMHO probably not really. IIRC about these options, the "50" (or "80")
is some gcc-internal metric. And it varies probably from version to
version.
It is not number of generated bytes or number of assembler commands or
any other outside visible unit. I also played around with that number at
that time to make warnings go away.

So the pragmatic solution is to either remove that warnings entirely or
- preferred by me - raise the number to 80 (or whatever works without
warning).
Why?
A quite small helper function like above can (and should) very well be
inlined. If the compiler can prove that the ptr can't be NULL, it can
optimize the check way. Otherwise it should leave it in.
And if that magic metric gives even 150 on that, so should it be.

The only reason to heav "-Winline" is to have gcc raise a warning so
that humans look into it and decide.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services



