Re: [PATCH v2 1/1] sandbox: eliminate unused functions from binaries
Hi, On Mon, 23 Oct 2023 at 10:24, Tom Rini wrote: > > On Mon, Oct 23, 2023 at 10:13:55AM -0700, Simon Glass wrote: > > Hi, > > > > On Mon, 23 Oct 2023 at 10:03, Tom Rini wrote: > > > > > > On Mon, Oct 23, 2023 at 01:37:27AM +0200, Heinrich Schuchardt wrote: > > > > The sandbox should closely mimic other architectures. > > > > > > > > Place each function or data in a separate section and let the linker > > > > eliminate unused ones. This will reduce the binary size. > > > > > > > > In the linker script mark that u_boot_sandbox_getopt are to be kept. > > > > > > > > Signed-off-by: Heinrich Schuchardt > > > > --- > > > > v2: > > > > mark _u_boot_sandbox_getopt secitons a KEEP() > > > > --- > > > > arch/sandbox/config.mk | 4 +++- > > > > arch/sandbox/cpu/u-boot.lds | 6 +++--- > > > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk > > > > index 2d184c5f65..c02f0229c2 100644 > > > > --- a/arch/sandbox/config.mk > > > > +++ b/arch/sandbox/config.mk > > > > @@ -2,7 +2,7 @@ > > > > # Copyright (c) 2011 The Chromium OS Authors. > > > > > > > > PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE > > > > -PLATFORM_CPPFLAGS += -fPIC > > > > +PLATFORM_CPPFLAGS += -fPIC -ffunction-sections -fdata-sections > > > > PLATFORM_LIBS += -lrt > > > > SDL_CONFIG ?= sdl2-config > > > > > > > > @@ -26,6 +26,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds > > > > $(u-boot-init) \ > > > > $(KBUILD_LDFLAGS:%=-Wl,%) \ > > > > $(SANITIZERS) \ > > > > $(LTO_FINAL_LDFLAGS) \ > > > > + -Wl,--gc-section \ > > > > -Wl,--whole-archive \ > > > > $(u-boot-main) \ > > > > $(u-boot-keep-syms-lto) \ > > > > @@ -37,6 +38,7 @@ cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) > > > > -Wl,-T u-boot-spl.lds \ > > > > $(SANITIZERS) \ > > > > $(LTO_FINAL_LDFLAGS) \ > > > > $(patsubst $(obj)/%,%,$(u-boot-spl-init)) \ > > > > + -Wl,--gc-section \ > > > > -Wl,--whole-archive \ > > > > $(patsubst $(obj)/%,%,$(u-boot-spl-main)) \ > > > > $(patsubst $(obj)/%,%,$(u-boot-spl-platdata)) \ > > > > > > There's already a call to --gc-sections for u-boot-spl on sandbox (in > > > the last line of the link command). We should probably move the main > > > u-boot call there too, for consistency. > > > > For the moment I really don't want this in sandbox. I think I explain > > that there is very little benefit in terms of size and it takes us > > away from how native apps are normally built. It also seems to mask > > problems in your CMDLINE series > > This fixes problems with sandbox not acting like a proper U-Boot target. > It is not at all about size reduction, it's about being able to use > normal conventions in U-Boot. I've said everything I can say here. I would prefer not to go down this path, but we can always try it and see what happens. Regards, Simon
Re: [PATCH v2 1/1] sandbox: eliminate unused functions from binaries
On Mon, Oct 23, 2023 at 10:13:55AM -0700, Simon Glass wrote: > Hi, > > On Mon, 23 Oct 2023 at 10:03, Tom Rini wrote: > > > > On Mon, Oct 23, 2023 at 01:37:27AM +0200, Heinrich Schuchardt wrote: > > > The sandbox should closely mimic other architectures. > > > > > > Place each function or data in a separate section and let the linker > > > eliminate unused ones. This will reduce the binary size. > > > > > > In the linker script mark that u_boot_sandbox_getopt are to be kept. > > > > > > Signed-off-by: Heinrich Schuchardt > > > --- > > > v2: > > > mark _u_boot_sandbox_getopt secitons a KEEP() > > > --- > > > arch/sandbox/config.mk | 4 +++- > > > arch/sandbox/cpu/u-boot.lds | 6 +++--- > > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk > > > index 2d184c5f65..c02f0229c2 100644 > > > --- a/arch/sandbox/config.mk > > > +++ b/arch/sandbox/config.mk > > > @@ -2,7 +2,7 @@ > > > # Copyright (c) 2011 The Chromium OS Authors. > > > > > > PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE > > > -PLATFORM_CPPFLAGS += -fPIC > > > +PLATFORM_CPPFLAGS += -fPIC -ffunction-sections -fdata-sections > > > PLATFORM_LIBS += -lrt > > > SDL_CONFIG ?= sdl2-config > > > > > > @@ -26,6 +26,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds > > > $(u-boot-init) \ > > > $(KBUILD_LDFLAGS:%=-Wl,%) \ > > > $(SANITIZERS) \ > > > $(LTO_FINAL_LDFLAGS) \ > > > + -Wl,--gc-section \ > > > -Wl,--whole-archive \ > > > $(u-boot-main) \ > > > $(u-boot-keep-syms-lto) \ > > > @@ -37,6 +38,7 @@ cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) > > > -Wl,-T u-boot-spl.lds \ > > > $(SANITIZERS) \ > > > $(LTO_FINAL_LDFLAGS) \ > > > $(patsubst $(obj)/%,%,$(u-boot-spl-init)) \ > > > + -Wl,--gc-section \ > > > -Wl,--whole-archive \ > > > $(patsubst $(obj)/%,%,$(u-boot-spl-main)) \ > > > $(patsubst $(obj)/%,%,$(u-boot-spl-platdata)) \ > > > > There's already a call to --gc-sections for u-boot-spl on sandbox (in > > the last line of the link command). We should probably move the main > > u-boot call there too, for consistency. > > For the moment I really don't want this in sandbox. I think I explain > that there is very little benefit in terms of size and it takes us > away from how native apps are normally built. It also seems to mask > problems in your CMDLINE series This fixes problems with sandbox not acting like a proper U-Boot target. It is not at all about size reduction, it's about being able to use normal conventions in U-Boot. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/1] sandbox: eliminate unused functions from binaries
Hi, On Mon, 23 Oct 2023 at 10:03, Tom Rini wrote: > > On Mon, Oct 23, 2023 at 01:37:27AM +0200, Heinrich Schuchardt wrote: > > The sandbox should closely mimic other architectures. > > > > Place each function or data in a separate section and let the linker > > eliminate unused ones. This will reduce the binary size. > > > > In the linker script mark that u_boot_sandbox_getopt are to be kept. > > > > Signed-off-by: Heinrich Schuchardt > > --- > > v2: > > mark _u_boot_sandbox_getopt secitons a KEEP() > > --- > > arch/sandbox/config.mk | 4 +++- > > arch/sandbox/cpu/u-boot.lds | 6 +++--- > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk > > index 2d184c5f65..c02f0229c2 100644 > > --- a/arch/sandbox/config.mk > > +++ b/arch/sandbox/config.mk > > @@ -2,7 +2,7 @@ > > # Copyright (c) 2011 The Chromium OS Authors. > > > > PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE > > -PLATFORM_CPPFLAGS += -fPIC > > +PLATFORM_CPPFLAGS += -fPIC -ffunction-sections -fdata-sections > > PLATFORM_LIBS += -lrt > > SDL_CONFIG ?= sdl2-config > > > > @@ -26,6 +26,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds > > $(u-boot-init) \ > > $(KBUILD_LDFLAGS:%=-Wl,%) \ > > $(SANITIZERS) \ > > $(LTO_FINAL_LDFLAGS) \ > > + -Wl,--gc-section \ > > -Wl,--whole-archive \ > > $(u-boot-main) \ > > $(u-boot-keep-syms-lto) \ > > @@ -37,6 +38,7 @@ cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T > > u-boot-spl.lds \ > > $(SANITIZERS) \ > > $(LTO_FINAL_LDFLAGS) \ > > $(patsubst $(obj)/%,%,$(u-boot-spl-init)) \ > > + -Wl,--gc-section \ > > -Wl,--whole-archive \ > > $(patsubst $(obj)/%,%,$(u-boot-spl-main)) \ > > $(patsubst $(obj)/%,%,$(u-boot-spl-platdata)) \ > > There's already a call to --gc-sections for u-boot-spl on sandbox (in > the last line of the link command). We should probably move the main > u-boot call there too, for consistency. > For the moment I really don't want this in sandbox. I think I explain that there is very little benefit in terms of size and it takes us away from how native apps are normally built. It also seems to mask problems in your CMDLINE series Regards, Simon
Re: [PATCH v2 1/1] sandbox: eliminate unused functions from binaries
On Mon, Oct 23, 2023 at 01:37:27AM +0200, Heinrich Schuchardt wrote: > The sandbox should closely mimic other architectures. > > Place each function or data in a separate section and let the linker > eliminate unused ones. This will reduce the binary size. > > In the linker script mark that u_boot_sandbox_getopt are to be kept. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > mark _u_boot_sandbox_getopt secitons a KEEP() > --- > arch/sandbox/config.mk | 4 +++- > arch/sandbox/cpu/u-boot.lds | 6 +++--- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk > index 2d184c5f65..c02f0229c2 100644 > --- a/arch/sandbox/config.mk > +++ b/arch/sandbox/config.mk > @@ -2,7 +2,7 @@ > # Copyright (c) 2011 The Chromium OS Authors. > > PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE > -PLATFORM_CPPFLAGS += -fPIC > +PLATFORM_CPPFLAGS += -fPIC -ffunction-sections -fdata-sections > PLATFORM_LIBS += -lrt > SDL_CONFIG ?= sdl2-config > > @@ -26,6 +26,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds $(u-boot-init) > \ > $(KBUILD_LDFLAGS:%=-Wl,%) \ > $(SANITIZERS) \ > $(LTO_FINAL_LDFLAGS) \ > + -Wl,--gc-section \ > -Wl,--whole-archive \ > $(u-boot-main) \ > $(u-boot-keep-syms-lto) \ > @@ -37,6 +38,7 @@ cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T > u-boot-spl.lds \ > $(SANITIZERS) \ > $(LTO_FINAL_LDFLAGS) \ > $(patsubst $(obj)/%,%,$(u-boot-spl-init)) \ > + -Wl,--gc-section \ > -Wl,--whole-archive \ > $(patsubst $(obj)/%,%,$(u-boot-spl-main)) \ > $(patsubst $(obj)/%,%,$(u-boot-spl-platdata)) \ There's already a call to --gc-sections for u-boot-spl on sandbox (in the last line of the link command). We should probably move the main u-boot call there too, for consistency. -- Tom signature.asc Description: PGP signature
