[U-Boot] patch for gc-sections

[U-Boot] patch for gc-sections

[Haiying Wang]

Peter,

Do you have any idea on why your commit:
"
commit fbe53f59bd40b3b1ab66dc98859e26589d64d1b7
Author: Peter Tyser <ptyser@xes-inc.com>
Date:   Wed Sep 29 14:05:56 2010 -0500

    85xx: Use gc-sections to reduce image size

    
which made change to :
"
--- a/arch/powerpc/cpu/mpc85xx/config.mk
+++ b/arch/powerpc/cpu/mpc85xx/config.mk
@@ -25,6 +25,10 @@ PLATFORM_RELFLAGS += -fPIC -meabi
 
 PLATFORM_CPPFLAGS += -ffixed-r2 -Wa,-me500 -msoft-float -mno-string
 
+# Enable gc-sections to enable generation of smaller images.
+PLATFORM_LDFLAGS += --gc-sections
+PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
"

makes the u-boot-spl.bin, u-boot-spl-16k.bin to 0 bytes? At least I
tried to build MPC8536DS_NAND_config, MPC8569MDS_NAND_config,
P1021MDS_NAND_config, and got 0 bytes of those nand_spl binaries for
them all.

Thanks.

Haiying

[U-Boot] patch for gc-sections

[Peter Tyser]

On Wed, 2010-11-03 at 15:07 -0400, Haiying Wang wrote:
> Peter,
> 
> Do you have any idea on why your commit:
> "
> commit fbe53f59bd40b3b1ab66dc98859e26589d64d1b7
> Author: Peter Tyser <ptyser@xes-inc.com>
> Date:   Wed Sep 29 14:05:56 2010 -0500
> 
>     85xx: Use gc-sections to reduce image size

I'd guess none of the functions in the SPL binary are referenced in the
linker script or linker command line, so the linker thinks none of them
are necessary and removes them.

Can you try the following patch:
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
index 7d9cee9..53d33ee 100644
--- a/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
+++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand_spl.lds
@@ -54,7 +54,7 @@ SECTIONS
        __init_end = .;
 
        .resetvec ADDR(.text) + 0xffc : {
-               *(.resetvec)
+               KEEP(*(.resetvec))
        } = 0xffff
 
        __bss_start = .;

I did a quick compile test, and it seemed to work, as well as stripped
out a few unused functions.

Best,
Peter

[U-Boot] patch for gc-sections

[Haiying Wang]

On Wed, 2010-11-03 at 13:38 -0700, Peter Tyser wrote:
> I'd guess none of the functions in the SPL binary are referenced in
> the
> linker script or linker command line, so the linker thinks none of
> them
> are necessary and removes them.
> 
> Can you try the following patch:
> I did a quick compile test, and it seemed to work, as well as stripped
> out a few unused functions.

Thanks for your patch, it did work to generate the binary, however,
there are two problems: 
1. The u-boot size for nand_spl is not cut down as expected.
/* before apply your new patch */
 29292 2010-11-04 09:29 nand_spl/u-boot-spl
     0 2010-11-04 09:29 nand_spl/u-boot-spl-16k.bin
     0 2010-11-04 09:29 nand_spl/u-boot-spl.bin
 13931 2010-11-04 09:29 nand_spl/u-boot-spl.map

/* After apply your new patch */
    35636 2010-11-04 09:49 nand_spl/u-boot-spl
116912128 2010-11-04 09:49 nand_spl/u-boot-spl-16k.bin
     4096 2010-11-04 09:49 nand_spl/u-boot-spl.bin
    16381 2010-11-04 09:49 nand_spl/u-boot-spl.map

/* Remove your two patches(remove gc-section patch and this new patch */
    34094 2010-11-04 09:51 nand_spl/u-boot-spl
116912128 2010-11-04 09:51 nand_spl/u-boot-spl-16k.bin
     4096 2010-11-04 09:51 nand_spl/u-boot-spl.bin
    14097 2010-11-04 09:51 nand_spl/u-boot-spl.map

2.the u-boot-spl.bin is 4096 bytes, and u-boot-spl-16.bin which should
be padded to 4K bytes is 116912128 bytes. You can take a look at
nand_spl/board/freescale/mpc8536ds/Makefile to see how
u-boot-spl.bin/u-boot-spl-16k.bin is generated. I don't know which
patch(not your gc-section for sure)  cause this problem. I just reset my
git tree to 2010.06, the size is:
 33512 2010-11-04 10:07 nand_spl/u-boot-spl
 4096 2010-11-04 10:07 nand_spl/u-boot-spl-16k.bin
 4096 2010-11-04 10:07 nand_spl/u-boot-spl.bin
 14037 2010-11-04 10:07 nand_spl/u-boot-spl.map

So we need to find out the cause for the huge u-boot-spl-16k.bin.

Anyway, thanks for taking care of this.

Haiying

[U-Boot] patch for gc-sections

[Peter Tyser]

On Thu, 2010-11-04 at 10:19 -0400, Haiying Wang wrote:
> On Wed, 2010-11-03 at 13:38 -0700, Peter Tyser wrote:
> > I'd guess none of the functions in the SPL binary are referenced in
> > the
> > linker script or linker command line, so the linker thinks none of
> > them
> > are necessary and removes them.
> > 
> > Can you try the following patch:
> > I did a quick compile test, and it seemed to work, as well as stripped
> > out a few unused functions.
> 
> Thanks for your patch, it did work to generate the binary, however,
> there are two problems: 
> 1. The u-boot size for nand_spl is not cut down as expected.
> /* before apply your new patch */
>  29292 2010-11-04 09:29 nand_spl/u-boot-spl
>      0 2010-11-04 09:29 nand_spl/u-boot-spl-16k.bin
>      0 2010-11-04 09:29 nand_spl/u-boot-spl.bin
>  13931 2010-11-04 09:29 nand_spl/u-boot-spl.map
> 
> /* After apply your new patch */
>     35636 2010-11-04 09:49 nand_spl/u-boot-spl
> 116912128 2010-11-04 09:49 nand_spl/u-boot-spl-16k.bin
>      4096 2010-11-04 09:49 nand_spl/u-boot-spl.bin
>     16381 2010-11-04 09:49 nand_spl/u-boot-spl.map
> 
> /* Remove your two patches(remove gc-section patch and this new patch */
>     34094 2010-11-04 09:51 nand_spl/u-boot-spl
> 116912128 2010-11-04 09:51 nand_spl/u-boot-spl-16k.bin
>      4096 2010-11-04 09:51 nand_spl/u-boot-spl.bin
>     14097 2010-11-04 09:51 nand_spl/u-boot-spl.map

Can you explain what you mean?  The binary needs to be 4K, right?  So it
can't be trimmed down.  But there should be more available space in that
4K region, eg (all tests on MPC8536DS_NAND_config):

/* After apply my patch sent yesterday */
ptyser at petert u-boot $ size after/u-boot-spl
   text    data     bss     dec     hex filename
   3440     460       0    3900     f3c after/u-boot-spl

/* Remove my two patches(remove gc-section patch and this new patch) */
ptyser at petert u-boot $ size before/u-boot-spl
   text	   data	    bss	    dec	    hex	filename
   3620	    460	      0	   4080	    ff0	before/u-boot-spl

The above says 180 bytes were removed between
fbe53f59bd40b3b1ab66dc98859e26589d64d1b7 and the current HEAD.  I'm
assuming some of that savings is due to -gc-sections (I think 2-3
functions were stripped out via -gc-sections).

You can just look at filesizes of ELF and map files to determine
savings, you'll have to use size/objdump/readelf to determine how the
code size is really affected.

> 2.the u-boot-spl.bin is 4096 bytes, and u-boot-spl-16.bin which should
> be padded to 4K bytes is 116912128 bytes. You can take a look at
> nand_spl/board/freescale/mpc8536ds/Makefile to see how
> u-boot-spl.bin/u-boot-spl-16k.bin is generated. I don't know which
> patch(not your gc-section for sure)  cause this problem. I just reset my
> git tree to 2010.06, the size is:
>  33512 2010-11-04 10:07 nand_spl/u-boot-spl
>  4096 2010-11-04 10:07 nand_spl/u-boot-spl-16k.bin
>  4096 2010-11-04 10:07 nand_spl/u-boot-spl.bin
>  14037 2010-11-04 10:07 nand_spl/u-boot-spl.map

I batted an eye when I saw the 112M file to, but it looks like that is
unrelated to my change:

/* Remove my two patches(remove gc-section patch and this new patch) */
ptyser at petert u-boot $ ls -lh after/
-rwxr-xr-x 1 ptyser users  36K 2010-11-04 10:25 u-boot-spl
-rwxr-xr-x 1 ptyser users 112M 2010-11-04 10:25 u-boot-spl-16k.bin
-rwxr-xr-x 1 ptyser users 4.0K 2010-11-04 10:25 u-boot-spl.bin
-rw-r--r-- 1 ptyser users  16K 2010-11-04 10:25 u-boot-spl.map

/* Remove my two patches(remove gc-section patch and this new patch) */
ptyser at petert u-boot $ ls -lh before/
-rwxr-xr-x 1 ptyser users  35K 2010-11-04 10:26 u-boot-spl
-rwxr-xr-x 1 ptyser users 112M 2010-11-04 10:26 u-boot-spl-16k.bin
-rwxr-xr-x 1 ptyser users 4.0K 2010-11-04 10:26 u-boot-spl.bin
-rw-r--r-- 1 ptyser users  13K 2010-11-04 10:26 u-boot-spl.map

The PAD_TO address is set to 0xfff01000, and each of
include/configs/MPC8536DS.h, include/configs/MPC8569MDS.h,
include/configs/P1_P2_RDB.h set CONFIG_SYS_TEXT_BASE to 0xf8f82000.

0xff01000 - 0xff82000 = 0x6F7F000 = 116912128 (as seen above).

Are these boards mis-configured?  In any case, everything seems to be
working as "expected" from what I can tell after the example patch I
sent yesterday.  Let me know if I'm missing something.

Best,
Peter

[U-Boot] patch for gc-sections

[Haiying Wang]

On Thu, 2010-11-04 at 08:45 -0700, Peter Tyser wrote:
> Can you explain what you mean?  The binary needs to be 4K, right?  So
> it
> can't be trimmed down.  But there should be more available space in
> that
> 4K region, eg (all tests on MPC8536DS_NAND_config):
> 
> /* After apply my patch sent yesterday */
> ptyser at petert u-boot $ size after/u-boot-spl
>    text    data     bss     dec     hex filename
>    3440     460       0    3900     f3c after/u-boot-spl
> 
> /* Remove my two patches(remove gc-section patch and this new patch)
> */
> ptyser at petert u-boot $ size before/u-boot-spl
>    text    data     bss     dec     hex filename
>    3620     460       0    4080     ff0 before/u-boot-spl
Yes, you are right, the size is down. I only noticed the u-boot-spl size
by using ls -l, not via size. Now I get the same result as yours. 

It is very good to see this trim-down size because I know some board
developers are fighting with the 4k limitation of the nand-spl size. Are
you going to submit your new patch to upstream?

Btw, the filesize I see here is smaller than yours, I guess you are
using the most updated gcc version. I am using gcc-4.3.2.
 
> I batted an eye when I saw the 112M file to, but it looks like that is
> unrelated to my change:
It is not related to your change, I see it happens on 2010.09 which
doesn't have your change. Something between 2010.06 and 2010.09 makes
this happen.

Thanks.

Haiying

[U-Boot] patch for gc-sections

[Peter Tyser]

> Yes, you are right, the size is down. I only noticed the u-boot-spl size
> by using ls -l, not via size. Now I get the same result as yours. 
> 
> It is very good to see this trim-down size because I know some board
> developers are fighting with the 4k limitation of the nand-spl size. Are
> you going to submit your new patch to upstream?

Glad to hear.  I'll submit an official patch shortly.  Just to make
sure, have you tried running one of the nand-spl images after the patch
I sent yesterday?  It'd be good to get confirmation that the
-gc-sections doesn't have any accidental side effects as I wasn't able
to test it.

> Btw, the filesize I see here is smaller than yours, I guess you are
> using the most updated gcc version. I am using gcc-4.3.2.

I was using gcc 4.2.2 in my tests for what its worth.

> > I batted an eye when I saw the 112M file to, but it looks like that is
> > unrelated to my change:
> It is not related to your change, I see it happens on 2010.09 which
> doesn't have your change. Something between 2010.06 and 2010.09 makes
> this happen.

One more bug to track down:)

Best,
Peter

[U-Boot] patch for gc-sections

[Haiying Wang]

On Thu, 2010-11-04 at 09:36 -0700, Peter Tyser wrote:
> Glad to hear.  I'll submit an official patch shortly.  Just to make
> sure, have you tried running one of the nand-spl images after the
> patch
> I sent yesterday?  It'd be good to get confirmation that the
> -gc-sections doesn't have any accidental side effects as I wasn't able
> to test it.
Because of the second bug, I can not test it based on the top of the
uboot tree. I just use 2010.06 version plus your two patches, and run
the nand-spl image on MPC8569MDS board. Only "NAND boot... transfering
control" is printed out in the terminal. It means the final u-boot image
doesn't work.

Then I make changes to u-boot-nand.lds and u-boot-nand_spl.lds,
following the changes you made for u-boot.lds, I get the first u-boot
line "U-Boot 2010.06..." printed out which means the final u-boot image
can work a little bit. But nothing else is showed up in terminal.

Do you have any idea on how to make changes to u-boot-nand/_spl.lds?

Thanks.

Haiying

[U-Boot] patch for gc-sections

[Haiying Wang]

On Thu, 2010-11-04 at 14:22 -0400, Haiying Wang wrote:
> On Thu, 2010-11-04 at 09:36 -0700, Peter Tyser wrote:
> > Glad to hear.  I'll submit an official patch shortly.  Just to make
> > sure, have you tried running one of the nand-spl images after the
> > patch
> > I sent yesterday?  It'd be good to get confirmation that the
> > -gc-sections doesn't have any accidental side effects as I wasn't able
> > to test it.
> Because of the second bug, I can not test it based on the top of the
> uboot tree. I just use 2010.06 version plus your two patches, and run
> the nand-spl image on MPC8569MDS board. Only "NAND boot... transfering
> control" is printed out in the terminal. It means the final u-boot image
> doesn't work.
> 
> Then I make changes to u-boot-nand.lds and u-boot-nand_spl.lds,
> following the changes you made for u-boot.lds, I get the first u-boot
> line "U-Boot 2010.06..." printed out which means the final u-boot image
> can work a little bit. But nothing else is showed up in terminal.
> 
> Do you have any idea on how to make changes to u-boot-nand/_spl.lds?

I figured out the problem now. The bootpg was removed by --gc-section as
well, so I need to add KEEP for it in u-boot-nand.lds. 

Patch will come out, I tested on MPC8569MDS board against top of the
tree.

Haiying