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

David Miller
2013-02-11T00:39:31Z

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. :-/