From: Colin Ian King <[email protected]>
Don't populate the array ext_div on the stack but instead it
static const. Makes the object code smaller by 118 bytes:
Before:
text data bss dec hex filename
39449 17500 128 57077 def5 ./drivers/gpu/drm/bridge/tc358767.o
After:
text data bss dec hex filename
39235 17596 128 56959 de7f ./drivers/gpu/drm/bridge/tc358767.o
(gcc version 10.3.0)
Signed-off-by: Colin Ian King <[email protected]>
---
drivers/gpu/drm/bridge/tc358767.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 23a6f90b694b..599c23759400 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -468,7 +468,7 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
int div, best_div = 1;
int mul, best_mul = 1;
int delta, best_delta;
- int ext_div[] = {1, 2, 3, 5, 7};
+ static const int ext_div[] = {1, 2, 3, 5, 7};
int best_pixelclock = 0;
int vco_hi = 0;
u32 pxl_pllparam;
--
2.32.0
On Thu, 2021-08-19 at 14:38 +0100, Colin King wrote:
> From: Colin Ian King <[email protected]>
>
> Don't populate the array ext_div on the stack but instead it
> static const. Makes the object code smaller by 118 bytes:
>
> Before:
> ???text data bss dec hex filename
> ??39449 17500 128 57077 def5 ./drivers/gpu/drm/bridge/tc358767.o
>
> After:
> ???text data bss dec hex filename
> ??39235 17596 128 56959 de7f ./drivers/gpu/drm/bridge/tc358767.o
Why is text smaller and data larger with this change?
>
> (gcc version 10.3.0)
>
> Signed-off-by: Colin Ian King <[email protected]>
> ---
> ?drivers/gpu/drm/bridge/tc358767.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 23a6f90b694b..599c23759400 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -468,7 +468,7 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
> ? int div, best_div = 1;
> ? int mul, best_mul = 1;
> ? int delta, best_delta;
> - int ext_div[] = {1, 2, 3, 5, 7};
> + static const int ext_div[] = {1, 2, 3, 5, 7};
> ? int best_pixelclock = 0;
> ? int vco_hi = 0;
> ? u32 pxl_pllparam;
On 19/08/2021 14:51, Joe Perches wrote:
> On Thu, 2021-08-19 at 14:38 +0100, Colin King wrote:
>> From: Colin Ian King <[email protected]>
>>
>> Don't populate the array ext_div on the stack but instead it
>> static const. Makes the object code smaller by 118 bytes:
>>
>> Before:
>> text data bss dec hex filename
>> 39449 17500 128 57077 def5 ./drivers/gpu/drm/bridge/tc358767.o
>>
>> After:
>> text data bss dec hex filename
>> 39235 17596 128 56959 de7f ./drivers/gpu/drm/bridge/tc358767.o
>
> Why is text smaller and data larger with this change?
There are less instructions being used with the change since it's not
shoving the array data onto the stack at run time. Instead the array is
being stored in the data section and there is less object code required
to access the data.
Colin
>
>>
>> (gcc version 10.3.0)
>>
>> Signed-off-by: Colin Ian King <[email protected]>
>> ---
>> drivers/gpu/drm/bridge/tc358767.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index 23a6f90b694b..599c23759400 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -468,7 +468,7 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
>> int div, best_div = 1;
>> int mul, best_mul = 1;
>> int delta, best_delta;
>> - int ext_div[] = {1, 2, 3, 5, 7};
>> + static const int ext_div[] = {1, 2, 3, 5, 7};
>> int best_pixelclock = 0;
>> int vco_hi = 0;
>> u32 pxl_pllparam;
>
>
On Thu, 2021-08-19 at 14:54 +0100, Colin Ian King wrote:
> On 19/08/2021 14:51, Joe Perches wrote:
> > On Thu, 2021-08-19 at 14:38 +0100, Colin King wrote:
> > > From: Colin Ian King <[email protected]>
> > >
> > > Don't populate the array ext_div on the stack but instead it
> > > static const. Makes the object code smaller by 118 bytes:
> > >
> > > Before:
> > > ???text data bss dec hex filename
> > > ??39449 17500 128 57077 def5 ./drivers/gpu/drm/bridge/tc358767.o
> > >
> > > After:
> > > ???text data bss dec hex filename
> > > ??39235 17596 128 56959 de7f ./drivers/gpu/drm/bridge/tc358767.o
> >
> > Why is text smaller and data larger with this change?
>
> There are less instructions being used with the change since it's not
> shoving the array data onto the stack at run time. Instead the array is
> being stored in the data section and there is less object code required
> to access the data.
Ah. It's really because it's not a minimal compilation ala defconfig.
I think you should really stop making these size comparisons with
.config uses that are not based on a defconfig as a whole lot of other
things are going on.
Please notice that the object sizes are significantly smaller below:
So with an x86-64 defconfig and this compilation unit enabled with
CONFIG_OF enabled and CONFIG_DRM_TOSHIBA_TC358767=y, with gcc 10.3
and this change the object size actually increases a bit.
$ size drivers/gpu/drm/bridge/tc358767.o*
text data bss dec hex filename
13554 268 1 13823 35ff drivers/gpu/drm/bridge/tc358767.o.new
13548 268 1 13817 35f9 drivers/gpu/drm/bridge/tc358767.o.old
objdump -h shows these differences:
.old:
0 .text 00001e1f 0000000000000000 0000000000000000 00000040 2**4
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
[...]
14 .rodata 000005ae 0000000000000000 0000000000000000 000046e0 2**5
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
.new:
0 .text 00001e05 0000000000000000 0000000000000000 00000040 2**4
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
[...]
11 .rodata 000005ce 0000000000000000 0000000000000000 00004600 2**5
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
cheers, Joe
On 19/08/2021 15:40, Joe Perches wrote:
> On Thu, 2021-08-19 at 14:54 +0100, Colin Ian King wrote:
>> On 19/08/2021 14:51, Joe Perches wrote:
>>> On Thu, 2021-08-19 at 14:38 +0100, Colin King wrote:
>>>> From: Colin Ian King <[email protected]>
>>>>
>>>> Don't populate the array ext_div on the stack but instead it
>>>> static const. Makes the object code smaller by 118 bytes:
>>>>
>>>> Before:
>>>> text data bss dec hex filename
>>>> 39449 17500 128 57077 def5 ./drivers/gpu/drm/bridge/tc358767.o
>>>>
>>>> After:
>>>> text data bss dec hex filename
>>>> 39235 17596 128 56959 de7f ./drivers/gpu/drm/bridge/tc358767.o
>>>
>>> Why is text smaller and data larger with this change?
>>
>> There are less instructions being used with the change since it's not
>> shoving the array data onto the stack at run time. Instead the array is
>> being stored in the data section and there is less object code required
>> to access the data.
>
> Ah. It's really because it's not a minimal compilation ala defconfig >
> I think you should really stop making these size comparisons with
> .config uses that are not based on a defconfig as a whole lot of other
> things are going on.
I'm using allmodconfig, which I believe is a legitimate configuration,
especially since distros so build kernels with lots of modules.
I'll double check on this though in case I've made a mistake.
>
> Please notice that the object sizes are significantly smaller below:
>
> So with an x86-64 defconfig and this compilation unit enabled with
> CONFIG_OF enabled and CONFIG_DRM_TOSHIBA_TC358767=y, with gcc 10.3
> and this change the object size actually increases a bit.
>
> $ size drivers/gpu/drm/bridge/tc358767.o*
> text data bss dec hex filename
> 13554 268 1 13823 35ff drivers/gpu/drm/bridge/tc358767.o.new
> 13548 268 1 13817 35f9 drivers/gpu/drm/bridge/tc358767.o.old>
> objdump -h shows these differences:
>
> .old:
> 0 .text 00001e1f 0000000000000000 0000000000000000 00000040 2**4
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> [...]
> 14 .rodata 000005ae 0000000000000000 0000000000000000 000046e0 2**5
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
>
> .new:
> 0 .text 00001e05 0000000000000000 0000000000000000 00000040 2**4
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> [...]
> 11 .rodata 000005ce 0000000000000000 0000000000000000 00004600 2**5
> CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
ACK. Understood. Even so, it still makes sense for these kind of
janitorial changes as it makes sense to constify arrays when they are
read-only and making them static is sensible for const data.
>
> cheers, Joe
>
On Thu, 2021-08-19 at 15:51 +0100, Colin Ian King wrote:
> it still makes sense for these kind of
> janitorial changes as it makes sense to constify arrays when they are
> read-only and making them static is sensible for const data.
I'm not disagreeing. Marking unmodifiable arrays as const is generally
useful for readers. Decent compilers though can _mostly_ determine
whether or not an array is used as const and whether the array can be
placed in a readonly section and is not required to be in a writable one.
But the object sizes deltas you show with an allmodconfig are misleading.
At a minimum I think you should show the output sizes as allmodconfig.