Le Sun, 04 Dec 2011 12:22:32 -0500,
Boris Gjenero <boris.gjenero_at_gmail.com> a écrit :

> On 03/12/2011 6:22 PM, Rafaël Carré wrote:
> > It fixes binsize increase after 93b6ca91696109be904448d23763f41b64bd7b8c^W r31127
> >
> > That flag is used already for android, bootbox, codecs, chessbox, goban,
> > imageviewer, lua, pictureflow, rockboy, zxbox, overlays
> >
> > Tested on Clip Zip
> > ---
> > tools/configure | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/tools/configure b/tools/configure
> > index 80c6380..6117f4f 100755
> > --- a/tools/configure
> > +++ b/tools/configure
> > @@ -9,7 +9,7 @@
> > #
> >
> > # global CC options for all platforms
> > -CCOPTS="-W -Wall -Wundef -O -nostdlib -ffreestanding -Wstrict-prototypes -pipe -std=gnu99"
> > +CCOPTS="-W -Wall -Wundef -O -nostdlib -ffreestanding -Wstrict-prototypes -pipe -std=gnu99 -Wl,--gc-sections"
> >
> > # global LD options for all platforms
> > GLOBAL_LDOPTS=""
>
> Wouldn't this only help with the the r31127 binsize increase when
> -ffunction-sections is defined?

Hum yes, I only verified that there was a binsize decrease.

For function removal I had this testcase:

% cat Makefile
CFLAGS += -O3 -Wl,--gc-sections

OBJS=a.o b.o

all: a.out
        objdump -d -j .rodata $<
        # You should only see DCBA == 44 43 42 41
        # 41 42 43 44 is not used and should be removed

a.out: $(OBJS)
        $(CC) $(CFLAGS) $(OBJS) -o $@
% cat a.c
void f(void) { puts("DCBA"); }
int main() {f();}
% cat b.c
__attribute__((weak)) void f(void)
{
    puts("ABCD");
}
%

> Also, it can remove needed stuff, and
> that needs to be prevented by putting KEEP() around that stuff in the
> linker script.

I can see some linker scripts have something like that already:
KEEP(*(.vectors));// otherwise there are no references to it and the linker strip it

> For example, if I add -Wl,--gc-sections to GCCOPTS in the r31138 5G iPod
> Makefile, size of .text of lcd-16bit.o remains the same, but vectors are
> discaded, creating a non-working binary. If I fix the vectors issue by
> adding KEEP( ) around *(.vectors) in ram.link, then there is only a 144
> byte binary size decrease from 3 small discards listed in rockbox.map.
> Without -ffunction-sections, the linker can only discard all of .text
> from lcd-16bit.o, and that won't happen because other functions are used.

Ok

> Also, I think -Wl,--gc-sections probably belongs in GLOBAL_LDOPTS.

Probably right. I hesitated to put it here because GLOBAL_LDOPTS was
empty for me.

tools/configure is also quite a mess, it uses sometimes CCOPTS,
sometimes GCCOPTS, etc

> I'm in favor of adding necessary KEEP() to linker scripts and using
> -ffunction-sections and -Wl,--gc-sections. It's a very easy way to get a
> binsize decrease, and I don't notice a linking time increase. I'm not
> suggesting -fdata-sections because added alignment padding is larger
> than the binsize decrease from removing unused data. Yes, preprocessor
> conditionals could take care of this stuff, but that could get messy,
> and I'm not sure if anyone wants to do all that work. For example,
> here's the list of functions which are unused or only used inline on the
> Archos targets: http://www.rockbox.org/tracker/task/12378?getfile=24330
>
> There is one catch here: this can resolve what would otherwise be
> linking errors. For example, suppose a non-static function a() is
> unused, and it calls another non-static function b() which is removed
> via preprocessor conditionals. Normally, that's a linking error, because
> the linker puts a() in the output, resulting in an undefined reference
> to b(). However, with -ffunction-sections and --Wl,--gc-sections, the
> compiler puts a() in its own section, and the linker discards that
> section because there are no references to it. So, there is no undefined
> reference to b(), and linking succeeds. Various bootloaders already fail
> to build without --gc-sections due to this.

If the build wouldn't have used the function I think making the link
fail isn't very helpful.

It avoids putting the call to b() under #ifdef

Btw the reason for the lcd_blit_yuv() commit was to avoid some #ifdef
hell (to remove the default C function from the sources when there is
an optimized version)

> Regards,
>
> Boris
>
> PS Thanks to Torne for telling me about -ffunction-sections,
> -fdata-sections and --gc-sections in a comment at FS#12378

Also Buschel tells me about FS#12427 : weak attribute doesn't seem to
work with mingw32 compiler

-- 
Rafaël Carré