netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Nithin Nayak Sujir <nsujir@broadcom.com>,
	Michael Chan <mchan@broadcom.com>
Cc: Veaceslav Falico <vfalico@gmail.com>,
	David Laight <David.Laight@aculab.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Jay Vosburgh <j.vosburgh@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	Veaceslav Falico <vfalico@redhat.com>
Subject: [PATCH] tg3: Use static inlines not macros
Date: Wed, 14 May 2014 17:04:11 -0700	[thread overview]
Message-ID: <1400112251.12666.14.camel@joe-AO725> (raw)
In-Reply-To: <CAADnVQKvrL+eKNbV=OWP3h2qPmHDm5ixt_4qNDtw-DZeR8US-w@mail.gmail.com>

Newer versions of gcc produce better code
so convert some macros to static inlines.

$ gcc --version
gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

(x86/defconfig)

$ size drivers/net/ethernet/broadcom/tg3.o.*
   text	   data	    bss	    dec	    hex	filename
 134282	    963	      0	 135245	  2104d	drivers/net/ethernet/broadcom/tg3.o.new
 134613	    963	      0	 135576	  21198	drivers/net/ethernet/broadcom/tg3.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---
On Wed, 2014-05-14 at 16:37 -0700, Alexei Starovoitov wrote:
> On Wed, May 14, 2014 at 3:03 PM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2014-05-14 at 14:52 -0700, Alexei Starovoitov wrote:
> >> I cannot imagine the case where macro would be faster than static inline
> >> unless it wasn't inlined.
> >
> > For an example, look at commit 4153577a8d
> > ("tg3: Use different macros for pci_chip_rev_id accesses")
> >
> > Converting these macros to static inline produces
> > larger/slower code.  (at least with gcc 4.7.3)
> >
> > +#define tg3_chip_rev_id(tp)                                    \
> > +       ((tp)->pci_chip_rev_id)
> > +#define tg3_asic_rev(tp)                                       \
> > +       ((tp)->pci_chip_rev_id >> 12)
> > +#define tg3_chip_rev(tp)                                       \
> > +       ((tp)->pci_chip_rev_id >> 8)
> >
> 
> hmm. interesting.
> Using gcc 4.7.2 object file size is larger with 'static inline'
> 2893016 vs 2868112
> but that's due to larger debug info.
> .text is actually smaller 000207c4 vs 00020824
> and these three calls were inlined (even without __always_inline),
> so I suspect it's better optimized..
> though better optimized can very well mean slower.

Compiler optimizers change with every version too.

 drivers/net/ethernet/broadcom/tg3.h | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 461acca..3d0cf6b 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -3416,11 +3416,19 @@ struct tg3 {
  *     Using statement expression macros to check tp with
  *     typecheck(struct tg3 *, tp) also creates larger objects.
  */
-#define tg3_chip_rev_id(tp)					\
-	((tp)->pci_chip_rev_id)
-#define tg3_asic_rev(tp)					\
-	((tp)->pci_chip_rev_id >> 12)
-#define tg3_chip_rev(tp)					\
-	((tp)->pci_chip_rev_id >> 8)
+static inline u32 tg3_chip_rev_id(const struct tg3 *tp)
+{
+	return tp->pci_chip_rev_id;
+}
+
+static inline u32 tg3_asic_rev(const struct tg3 *tp)
+{
+	return tp->pci_chip_rev_id >> 12;
+}
+
+static inline u32 tg3_chip_rev(const struct tg3 *tp)
+{
+	return tp->pci_chip_rev_id >> 8;
+}
 
 #endif /* !(_T3_H) */

  reply	other threads:[~2014-05-15  0:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-14 12:54 [PATCH net-next 0/5] bonding: simple macro cleanup Veaceslav Falico
2014-05-14 12:54 ` [PATCH net-next 1/5] bonding: use macro instead of bond_is_lb() Veaceslav Falico
2014-05-14 12:54 ` [PATCH net-next 2/5] bonding: rename {,BOND}_TX_QUEUE_OVERRIDE and make it accept bond struct Veaceslav Falico
2014-05-14 12:54 ` [PATCH net-next 3/5] bonding: rename {BOND_NO,MODE_NOT}_USES_ARP to better reflect its meaning Veaceslav Falico
2014-05-14 12:54 ` [PATCH net-next 4/5] bonding: rename {,MODE_}USES_PRIMARY " Veaceslav Falico
2014-05-14 12:54 ` [PATCH net-next 5/5] bonding: create a macro for bond mode and use it Veaceslav Falico
2014-05-14 13:08 ` [PATCH net-next 0/5] bonding: simple macro cleanup David Laight
2014-05-14 13:29   ` Veaceslav Falico
2014-05-14 16:10     ` Alexei Starovoitov
2014-05-14 16:29       ` Joe Perches
2014-05-14 21:52         ` Alexei Starovoitov
2014-05-14 22:03           ` Joe Perches
2014-05-14 23:37             ` Alexei Starovoitov
2014-05-15  0:04               ` Joe Perches [this message]
2014-05-15  5:24                 ` [PATCH] tg3: Use static inlines not macros Alexei Starovoitov
2014-05-15  9:04         ` [PATCH net-next 0/5] bonding: simple macro cleanup David Laight
2014-05-15  6:34       ` Veaceslav Falico

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1400112251.12666.14.camel@joe-AO725 \
    --to=joe@perches.com \
    --cc=David.Laight@aculab.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andy@greyhouse.net \
    --cc=j.vosburgh@gmail.com \
    --cc=mchan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=nsujir@broadcom.com \
    --cc=vfalico@gmail.com \
    --cc=vfalico@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).