[patch][v2] remove inline functions in fam10 source

[Segher Boessenkool]

> Remove inline from FAM10 CPU initialization functions. This should 
> save some ROM space.
> -static inline u32 get_initial_apicid(void)
> +static u32 get_initial_apicid(void)
>  {
>    return ((cpuid_ebx(1) >> 24) & 0xff);
>  }
> -static inline void _WRMSR(u32 addr, u32 lo, u32 hi)
> +static void _WRMSR(u32 addr, u32 lo, u32 hi)
>  {
>    __asm__ volatile (
>    	"wrmsr"
> -static inline u32 bsf(u32 x)
> +static u32 bsf(u32 x)
>  {
>    u8 i;
>    u32 ret = 32;
> -static inline u32 read_cr4(void)
> +static u32 read_cr4(void)
>  {
>    u32 cr4;
>    __asm__ volatile ("movl %%cr4, %0" : "=r" (cr4));

Things like these (not exhaustive) are usually *smaller* when inlined;
they compile to a single machine instruction (or two or so), and don't
have the space overhead for setting up a call (saving stuff to stack,
etc.)

But don't take my word for it, just measure it after every change :-)

Rest looks good.

Segher

[Segher Boessenkool]

> Things like these (not exhaustive) are usually *smaller* when inlined;
> they compile to a single machine instruction (or two or so), and

when inlined
> don't
> have the space overhead for setting up a call (saving stuff to stack,
> etc.)

[Marc Jones]

Segher Boessenkool wrote:
> > Remove inline from FAM10 CPU initialization functions. This should 
> > save some ROM space.

....
> > -static inline u32 read_cr4(void)
> > +static u32 read_cr4(void)
> >  {
> >      u32 cr4;
> >      __asm__ volatile ("movl %%cr4, %0" : "=r" (cr4));
>
> Things like these (not exhaustive) are usually *smaller* when inlined;
> they compile to a single machine instruction (or two or so), and don't
> have the space overhead for setting up a call (saving stuff to stack,
> etc.)
>
> But don't take my word for it, just measure it after every change :-)

Segher,

Thanks for the comments. I think you are right about  _asm_ inline 
instructions should be smaller than a function call. It turns out that 
none of these changes resulted in a ROM size decrease with my tools. My 
guess is the GCC inlined them anyway.

gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)
GNU ld (GNU Binutils for Ubuntu) 2.18

Here is a revised version of patch.

[Segher Boessenkool]

> Thanks for the comments. I think you are right about  _asm_ inline 
> instructions should be smaller than a function call. It turns out that 
> none of these changes resulted in a ROM size decrease with my tools. 
> My guess is the GCC inlined them anyway.

You can try  -finline-limit=5  or similar, or for finer control
--param max-inline-insns-auto=3  etc.

The standard GCC tuning seems not to always be very good for C code
at -Os, please file a bug report if you have some hard numbers on a
non-trivial piece of code (like this :-) ).

Segher

[Carl-Daniel Hailfinger]

On 25.04.2008 22:05, Marc Jones wrote:
> Segher Boessenkool wrote:
> > > Remove inline from FAM10 CPU initialization functions. This should
> > > save some ROM space.
>
> ....
> > > -static inline u32 read_cr4(void)
> > > +static u32 read_cr4(void)
> > >  {
> > >      u32 cr4;
> > >      __asm__ volatile ("movl %%cr4, %0" : "=r" (cr4));
> >
> > Things like these (not exhaustive) are usually *smaller* when inlined;
> > they compile to a single machine instruction (or two or so), and don't
> > have the space overhead for setting up a call (saving stuff to stack,
> > etc.)
> >
> > But don't take my word for it, just measure it after every change :-)
>
> Segher,
>
> Thanks for the comments. I think you are right about  _asm_ inline
> instructions should be smaller than a function call. It turns out that
> none of these changes resulted in a ROM size decrease with my tools.
> My guess is the GCC inlined them anyway.
>
> gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)
> GNU ld (GNU Binutils for Ubuntu) 2.18
>
> Here is a revised version of patch.

Thanks for doing this work!
> Remove inline from FAM10 CPU initialization functions.
> This doesn't save any space for me but it is the right thing to allow GCC to optimize.
>
> Signed-off-by: Marc Jones

Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[Marc Jones]

Carl-Daniel Hailfinger wrote:
> On 25.04.2008 22:05, Marc Jones wrote:
> > Segher Boessenkool wrote:
> > > > Remove inline from FAM10 CPU initialization functions. This should
> > > > save some ROM space.
> >
> > ....
> > > > -static inline u32 read_cr4(void)
> > > > +static u32 read_cr4(void)
> > > >  {
> > > >      u32 cr4;
> > > >      __asm__ volatile ("movl %%cr4, %0" : "=r" (cr4));
> > >
> > > Things like these (not exhaustive) are usually *smaller* when inlined;
> > > they compile to a single machine instruction (or two or so), and don't
> > > have the space overhead for setting up a call (saving stuff to stack,
> > > etc.)
> > >
> > > But don't take my word for it, just measure it after every change :-)
> >
> > Segher,
> >
> > Thanks for the comments. I think you are right about  _asm_ inline
> > instructions should be smaller than a function call. It turns out that
> > none of these changes resulted in a ROM size decrease with my tools.
> > My guess is the GCC inlined them anyway.
> >
> > gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)
> > GNU ld (GNU Binutils for Ubuntu) 2.18
> >
> > Here is a revised version of patch.
>
> Thanks for doing this work!
> > Remove inline from FAM10 CPU initialization functions.
> > This doesn't save any space for me but it is the right thing to allow GCC to optimize.
> >
> > Signed-off-by: Marc Jones
>
> Acked-by: Carl-Daniel Hailfinger
>
> Regards,
> Carl-Daniel

r3266

Thanks,
Marc