[PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Colin King]

From: Colin Ian King 

Don't populate const array ac_to_fifo on the stack in an inlined
function, instead make it static.  Makes the object code smaller
by over 800 bytes:

   textdata bss dec hex filename
 159029   331541216  193399   2f377 4965-mac.o

   textdata bss dec hex filename
 158122   332501216  192588   2f04c 4965-mac.o

(gcc version 7.2.0 x86_64)

Signed-off-by: Colin Ian King 
---
 drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c 
b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
index de9b6522c43f..65eba2c24292 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
@@ -1480,7 +1480,7 @@ il4965_get_ac_from_tid(u16 tid)
 static inline int
 il4965_get_fifo_from_tid(u16 tid)
 {
-   const u8 ac_to_fifo[] = {
+   static const u8 ac_to_fifo[] = {
IL_TX_FIFO_VO,
IL_TX_FIFO_VI,
IL_TX_FIFO_BE,
-- 
2.14.1

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Johannes Berg]

On Thu, 2017-09-21 at 23:56 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Don't populate const array ac_to_fifo on the stack in an inlined
> function, instead make it static.  Makes the object code smaller
> by over 800 bytes:
> 
>    text      data bss dec hex 
> filename
>  159029     33154    1216  193399   2f377 
> 4965-mac.o
> 
>    text      data bss dec hex 
> filename
>  158122     33250    1216  192588   2f04c 
> 4965-mac.o
> 
> (gcc version 7.2.0 x86_64)

I'm curious - how did you find this?

johannes

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Julia Lawall]

On Thu, 21 Sep 2017, Colin King wrote:

> From: Colin Ian King 
>
> Don't populate const array ac_to_fifo on the stack in an inlined
> function, instead make it static.  Makes the object code smaller
> by over 800 bytes:
>
>text  data bss dec hex filename
>  159029 331541216  193399   2f377 4965-mac.o
>
>text  data bss dec hex filename
>  158122 332501216  192588   2f04c 4965-mac.o
>
> (gcc version 7.2.0 x86_64)

Gcc 7 must be much more aggressive about inlining than gcc 4.  Here is the
change that I got on this file:

 text  data bss dec hex filename
-   76346  1494 152   77992   130a8 
drivers/net/wireless/intel/iwlegacy/4965-mac.o
+   76298  1494 152   77944   13078 
drivers/net/wireless/intel/iwlegacy/4965-mac.o
decrease of 48

julia

>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c 
> b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> index de9b6522c43f..65eba2c24292 100644
> --- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> @@ -1480,7 +1480,7 @@ il4965_get_ac_from_tid(u16 tid)
>  static inline int
>  il4965_get_fifo_from_tid(u16 tid)
>  {
> - const u8 ac_to_fifo[] = {
> + static const u8 ac_to_fifo[] = {
>   IL_TX_FIFO_VO,
>   IL_TX_FIFO_VI,
>   IL_TX_FIFO_BE,
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Stanislaw Gruszka]

On Thu, Sep 21, 2017 at 11:56:30PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Don't populate const array ac_to_fifo on the stack in an inlined
> function, instead make it static.  Makes the object code smaller
> by over 800 bytes:
> 
>text  data bss dec hex filename
>  159029 331541216  193399   2f377 4965-mac.o
> 
>text  data bss dec hex filename
>  158122 332501216  192588   2f04c 4965-mac.o
> 
> (gcc version 7.2.0 x86_64)
> 
> Signed-off-by: Colin Ian King 

Content type information was added at the end of the topic, but
I think Kalle can fix that when he will be committing the patch.

Acked-by: Stanislaw Gruszka 

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Joe Perches]

On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> 
> On Thu, 21 Sep 2017, Colin King wrote:
> 
> > From: Colin Ian King 
> > 
> > Don't populate const array ac_to_fifo on the stack in an inlined
> > function, instead make it static.  Makes the object code smaller
> > by over 800 bytes:
> > 
> >textdata bss dec hex filename
> >  159029   331541216  193399   2f377 4965-mac.o
> > 
> >textdata bss dec hex filename
> >  158122   332501216  192588   2f04c 4965-mac.o
> > 
> > (gcc version 7.2.0 x86_64)
> 
> Gcc 7 must be much more aggressive about inlining than gcc 4.  Here is the
> change that I got on this file:
> 
>  text  data bss dec hex filename
> -   76346  1494 152   77992   130a8 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o
> +   76298  1494 152   77944   13078 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o
> decrease of 48

More likely different CONFIG options.

e.g. allyesconfig vs defconfig with wireless

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Colin Ian King]

On 22/09/17 11:03, Joe Perches wrote:
> On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
>>
>> On Thu, 21 Sep 2017, Colin King wrote:
>>
>>> From: Colin Ian King 
>>>
>>> Don't populate const array ac_to_fifo on the stack in an inlined
>>> function, instead make it static.  Makes the object code smaller
>>> by over 800 bytes:
>>>
>>>textdata bss dec hex filename
>>>  159029   331541216  193399   2f377 4965-mac.o
>>>
>>>textdata bss dec hex filename
>>>  158122   332501216  192588   2f04c 4965-mac.o
>>>
>>> (gcc version 7.2.0 x86_64)
>>
>> Gcc 7 must be much more aggressive about inlining than gcc 4.  Here is the
>> change that I got on this file:
>>
>>  text  data bss dec hex filename
>> -   76346  1494 152   77992   130a8 
>> drivers/net/wireless/intel/iwlegacy/4965-mac.o
>> +   76298  1494 152   77944   13078 
>> drivers/net/wireless/intel/iwlegacy/4965-mac.o
>> decrease of 48
> 
> More likely different CONFIG options.
> 
> e.g. allyesconfig vs defconfig with wireless
> 
yup, I used allyesconfig

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Julia Lawall]

On Fri, 22 Sep 2017, Colin Ian King wrote:

> On 22/09/17 11:03, Joe Perches wrote:
> > On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> >>
> >> On Thu, 21 Sep 2017, Colin King wrote:
> >>
> >>> From: Colin Ian King 
> >>>
> >>> Don't populate const array ac_to_fifo on the stack in an inlined
> >>> function, instead make it static.  Makes the object code smaller
> >>> by over 800 bytes:
> >>>
> >>>text  data bss dec hex filename
> >>>  159029 331541216  193399   2f377 4965-mac.o
> >>>
> >>>text  data bss dec hex filename
> >>>  158122 332501216  192588   2f04c 4965-mac.o
> >>>
> >>> (gcc version 7.2.0 x86_64)
> >>
> >> Gcc 7 must be much more aggressive about inlining than gcc 4.  Here is the
> >> change that I got on this file:
> >>
> >>  text  data bss dec hex filename
> >> -   76346  1494 152   77992   130a8 
> >> drivers/net/wireless/intel/iwlegacy/4965-mac.o
> >> +   76298  1494 152   77944   13078 
> >> drivers/net/wireless/intel/iwlegacy/4965-mac.o
> >> decrease of 48
> >
> > More likely different CONFIG options.
> >
> > e.g. allyesconfig vs defconfig with wireless
> >
> yup, I used allyesconfig

Mine is also allyesconfig.

julia

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

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Kalle Valo]

Stanislaw Gruszka  writes:

> On Thu, Sep 21, 2017 at 11:56:30PM +0100, Colin King wrote:
>> From: Colin Ian King 
>> 
>> Don't populate const array ac_to_fifo on the stack in an inlined
>> function, instead make it static.  Makes the object code smaller
>> by over 800 bytes:
>> 
>>text data bss dec hex filename
>>  159029331541216  193399   2f377 4965-mac.o
>> 
>>text data bss dec hex filename
>>  158122332501216  192588   2f04c 4965-mac.o
>> 
>> (gcc version 7.2.0 x86_64)
>> 
>> Signed-off-by: Colin Ian King 
>
> Content type information was added at the end of the topic, but
> I think Kalle can fix that when he will be committing the patch.

Yeah, I'll fix that when I commit this. But very good that you pointed
it out, I might miss stuff like this.

I'll also remove the "wireless:" prefix from the title.

-- 
Kalle Valo

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Joe Perches]

On Fri, 2017-09-22 at 12:06 +0200, Julia Lawall wrote:
> 
> On Fri, 22 Sep 2017, Colin Ian King wrote:
> 
> > On 22/09/17 11:03, Joe Perches wrote:
> > > On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> > > > 
> > > > On Thu, 21 Sep 2017, Colin King wrote:
> > > > 
> > > > > From: Colin Ian King 
> > > > > 
> > > > > Don't populate const array ac_to_fifo on the stack in an inlined
> > > > > function, instead make it static.  Makes the object code smaller
> > > > > by over 800 bytes:
> > > > > 
> > > > >text  data bss dec hex filename
> > > > >  159029 331541216  193399   2f377 4965-mac.o
> > > > > 
> > > > >text  data bss dec hex filename
> > > > >  158122 332501216  192588   2f04c 4965-mac.o
> > > > > 
> > > > > (gcc version 7.2.0 x86_64)
> > > > 
> > > > Gcc 7 must be much more aggressive about inlining than gcc 4.  Here is 
> > > > the
> > > > change that I got on this file:
> > > > 
> > > >  text  data bss dec hex filename
> > > > -   76346  1494 152   77992   130a8 
> > > > drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > +   76298  1494 152   77944   13078 
> > > > drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > decrease of 48
> > > 
> > > More likely different CONFIG options.
> > > 
> > > e.g. allyesconfig vs defconfig with wireless
> > > 
> > 
> > yup, I used allyesconfig
> 
> Mine is also allyesconfig.

Odd.

Here is what I get: (Sorry, no gcc-7)

gcc4: gcc-4.9 (Ubuntu 4.9.4-2ubuntu1) 4.9.4
gcc5: gcc-5 (Ubuntu 5.4.1-8ubuntu1) 5.4.1 20170304
gcc6: gcc-6 (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406

$ size drivers/net/wireless/intel/iwlegacy/4965-mac.o*
   text    data bss dec hex filename
 118559    1766 152  120477   1d69d 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.new
 118623    1766 152  120541   1d6dd 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.old
  51595    1156   0   52751    ce0f 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.new
  51659    1156   0   52815    ce4f 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.old
 141956   29790    1216  172962   2a3a2 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.new
 142671   29702    1216  173589   2a615 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.old
  51733    1156   0   52889    ce99 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.new
  51813    1156   0   52969    cee9 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.old
 152991   29790    1216  183997   2cebd 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.new
 153722   29702    1216  184640   2d140 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.old
  51813    1156   0   52969    cee9 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.new
  51893    1156   0   53049    cf39 
drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.old

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Julia Lawall]

On Fri, 22 Sep 2017, Joe Perches wrote:

> On Fri, 2017-09-22 at 12:06 +0200, Julia Lawall wrote:
> >
> > On Fri, 22 Sep 2017, Colin Ian King wrote:
> >
> > > On 22/09/17 11:03, Joe Perches wrote:
> > > > On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> > > > >
> > > > > On Thu, 21 Sep 2017, Colin King wrote:
> > > > >
> > > > > > From: Colin Ian King 
> > > > > >
> > > > > > Don't populate const array ac_to_fifo on the stack in an inlined
> > > > > > function, instead make it static.  Makes the object code smaller
> > > > > > by over 800 bytes:
> > > > > >
> > > > > >textdata bss dec hex filename
> > > > > >  159029   331541216  193399   2f377 4965-mac.o
> > > > > >
> > > > > >textdata bss dec hex filename
> > > > > >  158122   332501216  192588   2f04c 4965-mac.o
> > > > > >
> > > > > > (gcc version 7.2.0 x86_64)
> > > > >
> > > > > Gcc 7 must be much more aggressive about inlining than gcc 4.  Here 
> > > > > is the
> > > > > change that I got on this file:
> > > > >
> > > > >  text  data bss dec hex filename
> > > > > -   76346  1494 152   77992   130a8 
> > > > > drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > > +   76298  1494 152   77944   13078 
> > > > > drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > > decrease of 48
> > > >
> > > > More likely different CONFIG options.
> > > >
> > > > e.g. allyesconfig vs defconfig with wireless
> > > >
> > >
> > > yup, I used allyesconfig
> >
> > Mine is also allyesconfig.
>
> Odd.
>
> Here is what I get: (Sorry, no gcc-7)
>
> gcc4: gcc-4.9 (Ubuntu 4.9.4-2ubuntu1) 4.9.4
> gcc5: gcc-5 (Ubuntu 5.4.1-8ubuntu1) 5.4.1 20170304
> gcc6: gcc-6 (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406
>
> $ size drivers/net/wireless/intel/iwlegacy/4965-mac.o*
>    text      data bss dec hex filename
>  118559      1766 152  120477   1d69d 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.new
>  118623      1766 152  120541   1d6dd 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.old

My gcc is 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.3)

You get a savings of 64 and I got a savings of 48, but it's not that big a
difference, compared to what the later versions give.

julia

>   51595      1156   0   52751    ce0f 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.new
>   51659      1156   0   52815    ce4f 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.old
>  141956     29790    1216  172962   2a3a2 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.new
>  142671     29702    1216  173589   2a615 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.old
>   51733      1156   0   52889    ce99 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.new
>   51813      1156   0   52969    cee9 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.old
>  152991     29790    1216  183997   2cebd 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.new
>  153722     29702    1216  184640   2d140 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.old
>   51813      1156   0   52969    cee9 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.new
>   51893      1156   0   53049    cf39 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.old
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>