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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1360546445.2028.8.camel@joe-AO722>
Date:	Sun, 10 Feb 2013 17:34:05 -0800
From:	Joe Perches <joe@...ches.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, hauke@...ke-m.de, mcarlson@...adcom.com,
	mchan@...adcom.com, nsujir@...adcom.com
Subject: Re: [RFC PATCH] tg3: Convert chip type macros to inline functions

On Sun, 2013-02-10 at 19:39 -0500, David Miller wrote:
> From: Joe Perches <joe@...ches.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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html