Re: [RFC PATCH] tg3: Convert chip type macros to inline functions

[Joe Perches <joe@perches.com>]

On Sun, 2013-02-10 at 19:39 -0500, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Wed, 06 Feb 2013 15:44:58 -0800
> 
> > To me the negative to these conversions is at
> > least for gcc 4.7.2, the overall code size
> > increases
> > 
> > $ size drivers/net/ethernet/broadcom/tg3.o*
> >    text          data     bss     dec     hex filename
> >  203426         13446   55744  272616   428e8 drivers/net/ethernet/broadcom/tg3.o.new
> >  202135         13446   55144  270725   42185 drivers/net/ethernet/broadcom/tg3.o.old
> > 
> > I'm not sure why gcc doesn't do the optimization
> > and code generation the same way.
> 
> Out of curiosity I looked at the assembler difference on sparc
> and one thing stood out.
> 
> You're changing how the values are evaluated, specifically you
> now are forcing the compiler to work with unsigned ID numbers
> whereas before it was working with signed values.
> 
> So, looking at one example, the ASIC revision test in __tg3_set_mac_addr():
> 
>         if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5703 ||
>             GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5704) {
>         ...
>         }
> 
> 
> The compiler previously turned the two tests into one:
> 
>         asic_id -= 1;
>         if (asic_id <= 1) {
>         ...
>         }
> 
> But this construct isn't valid for unsigned quantities, so now with
> your patch this expands to two tests:
> 
>         if (asic_id == 1 ||
>             asic_id == 2) {
>         ...
>         }
> 
> The assembler is hard to compare manually, because the inlining
> changes how hard registers are allocated, and issues like the above
> will change all of the branch and file offsets as well. :-/

Well maybe.

If the inlines return int instead of u32, x86 objects
do change trivially (using 2 jg vs ja) , but the size
is unchanged from u32 to int (still bigger).

Perhaps the gcc optimizer could be improved.