Re: [PATCH net-next] net: ethernet: ti: cpsw: replace unnecessarily macroses on functions
From: Ivan Khoronzhuk <hidden>
Date: 2018-07-27 20:23:58
Also in:
linux-omap, lkml
On Fri, Jul 27, 2018 at 01:04:22PM -0700, Joe Perches wrote:
On Fri, 2018-07-27 at 22:36 +0300, Ivan Khoronzhuk wrote:quoted
On Fri, Jul 27, 2018 at 12:21:07PM -0700, Joe Perches wrote:quoted
On Fri, 2018-07-27 at 22:13 +0300, Ivan Khoronzhuk wrote:quoted
Replace ugly macroses on functions.Careful, see below.quoted
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c[]quoted
@@ -565,40 +565,40 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = { (func)(slave++, ##arg); \ } while (0) -#define cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb) \ - do { \ - if (!cpsw->data.dual_emac) \ - break; \ - if (CPDMA_RX_SOURCE_PORT(status) == 1) { \ - ndev = cpsw->slaves[0].ndev; \ - skb->dev = ndev; \ - } else if (CPDMA_RX_SOURCE_PORT(status) == 2) { \ - ndev = cpsw->slaves[1].ndev; \ - skb->dev = ndev; \ - } \ - } while (0) -#define cpsw_add_mcast(cpsw, priv, addr) \ - do { \ - if (cpsw->data.dual_emac) { \ - struct cpsw_slave *slave = cpsw->slaves + \ - priv->emac_port; \ - int slave_port = cpsw_get_slave_port( \ - slave->slave_num); \ - cpsw_ale_add_mcast(cpsw->ale, addr, \ - 1 << slave_port | ALE_PORT_HOST, \ - ALE_VLAN, slave->port_vlan, 0); \ - } else { \ - cpsw_ale_add_mcast(cpsw->ale, addr, \ - ALE_ALL_PORTS, \ - 0, 0, 0); \ - } \ - } while (0) - static inline int cpsw_get_slave_port(u32 slave_num) { return slave_num + 1; } +static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status, + struct sk_buff *skb) +{ + if (!cpsw->data.dual_emac) + return; + + if (CPDMA_RX_SOURCE_PORT(status) == 1) + skb->dev = cpsw->slaves[0].ndev; + else if (CPDMA_RX_SOURCE_PORT(status) == 2) + skb->dev = cpsw->slaves[1].ndev; +}perhaps better as a switch/casenot better, it's shorter.True for the source code but it compiles to more object code. $ cat foo.c struct cpsw_common { struct { int dual_emac; } data; struct { int ndev; } slaves[2]; }; struct sk_buff { int dev; }; #define CPDMA_RX_SOURCE_PORT(__status__) ((__status__ >> 16) & 0x7) #if defined SWITCH void foo(struct cpsw_common *cpsw, int status, struct sk_buff *skb) { if (!cpsw->data.dual_emac) return; switch (CPDMA_RX_SOURCE_PORT(status)) { case 1: skb->dev = cpsw->slaves[0].ndev; break; case 2: skb->dev = cpsw->slaves[1].ndev; break; } } #else void foo(struct cpsw_common *cpsw, int status, struct sk_buff *skb) { if (!cpsw->data.dual_emac) return; if (CPDMA_RX_SOURCE_PORT(status) == 1) skb->dev = cpsw->slaves[0].ndev; else if (CPDMA_RX_SOURCE_PORT(status) == 2) skb->dev = cpsw->slaves[1].ndev; } #endif $ gcc -c -O2 -DSWITCH foo.c $ size foo.o text data bss dec hex filename 94 0 0 94 5e foo.o $ objdump -d foo.o foo.o: file format elf64-x86-64 Disassembly of section .text: 0000000000000000 <foo>: 0: 8b 07 mov (%rdi),%eax 2: 85 c0 test %eax,%eax 4: 74 15 je 1b <foo+0x1b> 6: c1 fe 10 sar $0x10,%esi 9: 83 e6 07 and $0x7,%esi c: 83 fe 01 cmp $0x1,%esi f: 74 17 je 28 <foo+0x28> 11: 83 fe 02 cmp $0x2,%esi 14: 75 0a jne 20 <foo+0x20> 16: 8b 47 08 mov 0x8(%rdi),%eax 19: 89 02 mov %eax,(%rdx) 1b: f3 c3 repz retq 1d: 0f 1f 00 nopl (%rax) 20: f3 c3 repz retq 22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) 28: 8b 47 04 mov 0x4(%rdi),%eax 2b: 89 02 mov %eax,(%rdx) 2d: c3 retq $ gcc -c -O2 foo.c $ size foo.o text data bss dec hex filename 102 0 0 102 66 foo.o $ objdump -d foo.o foo.o: file format elf64-x86-64 Disassembly of section .text: 0000000000000000 <foo>: 0: 8b 07 mov (%rdi),%eax 2: 85 c0 test %eax,%eax 4: 74 10 je 16 <foo+0x16> 6: c1 fe 10 sar $0x10,%esi 9: 83 e6 07 and $0x7,%esi c: 83 fe 01 cmp $0x1,%esi f: 74 0f je 20 <foo+0x20> 11: 83 fe 02 cmp $0x2,%esi 14: 74 1a je 30 <foo+0x30> 16: f3 c3 repz retq 18: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1) 1f: 00 20: 8b 47 04 mov 0x4(%rdi),%eax 23: 89 02 mov %eax,(%rdx) 25: c3 retq 26: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 2d: 00 00 00 30: 8b 47 08 mov 0x8(%rdi),%eax 33: 89 02 mov %eax,(%rdx) 35: c3 retq
This driver, mainly used for ARM
For ARM, situation a little bit different:
$ arm-linux-gnueabihf-gcc -c -O2 -DSWITCH foo.c
$ size foo.o
text data bss dec hex filename
32 0 0 32 20 foo.o
$ arm-linux-gnueabihf-objdump -d foo.o
foo.o: file format elf32-littlearm
Disassembly of section .text:
00000000 <foo>:
0: 6803 ldr r3, [r0, #0]
2: b143 cbz r3, 16 <foo+0x16>
4: f3c1 4102 ubfx r1, r1, #16, #3
8: 2901 cmp r1, #1
a: d005 beq.n 18 <foo+0x18>
c: 2902 cmp r1, #2
e: d000 beq.n 12 <foo+0x12>
10: 4770 bx lr
12: 6883 ldr r3, [r0, #8]
14: 6013 str r3, [r2, #0]
16: 4770 bx lr
18: 6843 ldr r3, [r0, #4]
1a: 6013 str r3, [r2, #0]
1c: 4770 bx lr
1e: bf00 nop
$ arm-linux-gnueabihf-gcc -c -O2 foo.c
$ size foo.o
text data bss dec hex filename
28 0 0 28 1c foo.o
$ arm-linux-gnueabihf-objdump -d foo.o
foo.o: file format elf32-littlearm
Disassembly of section .text:
00000000 <foo>:
0: 6803 ldr r3, [r0, #0]
2: b13b cbz r3, 14 <foo+0x14>
4: f3c1 4102 ubfx r1, r1, #16, #3
8: 2901 cmp r1, #1
a: d004 beq.n 16 <foo+0x16>
c: 2902 cmp r1, #2
e: bf04 itt eq
10: 6883 ldreq r3, [r0, #8]
12: 6013 streq r3, [r2, #0]
14: 4770 bx lr
16: 6843 ldr r3, [r0, #4]
18: 6013 str r3, [r2, #0]
1a: 4770 bx lr
So, it's shorter.
--
Regards,
Ivan Khoronzhuk