Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
coreinfo: Add support for link time optimization
This introduces a Kconfig option for compiling coreinfo with LTO. This option can be used independently of LTO in libpayload, though will benefit most if that is enabled as well. If both are enabled, the final size of coreinfo.elf is reduced from 125K to 122K.
Change-Id: I6feacdb911b52b946869bff369e03dcf72897c9f Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/coreinfo/Kconfig M payloads/coreinfo/Makefile 2 files changed, 12 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/38293/1
diff --git a/payloads/coreinfo/Kconfig b/payloads/coreinfo/Kconfig index fd4c1b4..5fb17bb 100644 --- a/payloads/coreinfo/Kconfig +++ b/payloads/coreinfo/Kconfig @@ -56,6 +56,13 @@ help The version number of this payload.
+config LTO + bool "Use link time optimization" + default n + help + Compile with link time optimization. This can often decrease the + final binary size, but may increase compilation time. + endmenu
menu "Modules" diff --git a/payloads/coreinfo/Makefile b/payloads/coreinfo/Makefile index 34c45d9..6452c00 100644 --- a/payloads/coreinfo/Makefile +++ b/payloads/coreinfo/Makefile @@ -90,9 +90,13 @@ include $(src)/.config real-all: $(TARGET)
+ifeq ($(CONFIG_LTO),y) +CFLAGS += -flto=$(CPUS) -fuse-linker-plugin -fno-fat-lto-objects +endif + $(TARGET): $(src)/.config $(coreinfo_obj)/config.h $(OBJS) libpayload printf " LPCC $(subst $(CURDIR)/,,$(@)) (LINK)\n" - $(LPCC) -o $@ $(OBJS) + $(LPCC) $(CFLAGS) -o $@ $(OBJS) $(OBJCOPY) --only-keep-debug $@ $(TARGET).debug $(OBJCOPY) --strip-debug $@ $(OBJCOPY) --add-gnu-debuglink=$(TARGET).debug $@
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 2: Code-Review+1
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 2:
(2 comments)
Did you also test with Clang?
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig File payloads/coreinfo/Kconfig:
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig@6... PS2, Line 61: default n I’d default to yes, as less size is more important for firmware purposes.
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig@6... PS2, Line 65: Please mention, if libpayload is then also indirectly built with LTO.
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38293
to look at the new patch set (#3).
Change subject: coreinfo: Add support for link time optimization ......................................................................
coreinfo: Add support for link time optimization
This introduces a Kconfig option for compiling coreinfo with LTO. This option can be used independently of LTO in libpayload, though will benefit most if that is enabled as well. If both are enabled, the final size of coreinfo.elf is reduced from 95 KiB to 92 KiB.
Change-Id: I6feacdb911b52b946869bff369e03dcf72897c9f Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/coreinfo/Kconfig M payloads/coreinfo/Makefile 2 files changed, 12 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/38293/3
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 3: Code-Review+2
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 3: -Code-Review
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38293/3/payloads/coreinfo/Kconfig File payloads/coreinfo/Kconfig:
https://review.coreboot.org/c/coreboot/+/38293/3/payloads/coreinfo/Kconfig@6... PS3, Line 60: bool "Use link time optimization" Isn't it experimental?
Hello David Hendricks, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38293
to look at the new patch set (#4).
Change subject: coreinfo: Add support for link time optimization ......................................................................
coreinfo: Add support for link time optimization
This introduces a Kconfig option for compiling coreinfo with LTO. This option can be used independently of LTO in libpayload, though will benefit most if that is enabled as well. If both are enabled, the final size of coreinfo.elf is reduced from 95 KiB to 92 KiB.
Change-Id: I6feacdb911b52b946869bff369e03dcf72897c9f Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/coreinfo/Kconfig M payloads/coreinfo/Makefile 2 files changed, 14 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/38293/4
Hello David Hendricks, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38293
to look at the new patch set (#5).
Change subject: coreinfo: Add support for link time optimization ......................................................................
coreinfo: Add support for link time optimization
This introduces a Kconfig option for compiling coreinfo with LTO. This option can be used independently of LTO in libpayload, though will benefit most if that is enabled as well. If both are enabled, the final size of coreinfo.elf is reduced from 95 KiB to 92 KiB.
Tested in QEMU and on Thinkpad T500.
Change-Id: I6feacdb911b52b946869bff369e03dcf72897c9f Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/coreinfo/Kconfig M payloads/coreinfo/Makefile 2 files changed, 14 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/38293/5
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 6:
Since coreinfo had libpayload around, including its config, maybe use LTO if libpayload was built with LTO as well?
Hello build bot (Jenkins), David Hendricks, Paul Menzel,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38293
to look at the new patch set (#9).
Change subject: coreinfo: Add support for link time optimization ......................................................................
coreinfo: Add support for link time optimization
This introduces a Kconfig option for compiling coreinfo with LTO. This option can be used independently of LTO in libpayload, though will benefit most if that is enabled as well. If both are enabled, the final size of coreinfo.elf is reduced from 95 KiB to 92 KiB.
Tested in QEMU and on Thinkpad T500.
Change-Id: I6feacdb911b52b946869bff369e03dcf72897c9f Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/coreinfo/Kconfig M payloads/coreinfo/Makefile 2 files changed, 14 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/38293/9
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 9: Code-Review+1
Hello build bot (Jenkins), Paul Menzel, David Hendricks,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38293
to look at the new patch set (#10).
Change subject: coreinfo: Add support for link time optimization ......................................................................
coreinfo: Add support for link time optimization
This introduces a Kconfig option for compiling coreinfo with LTO. This option can be used independently of LTO in libpayload, though will benefit most if that is enabled as well. If both are enabled, the final size of coreinfo.elf is reduced from 95 KiB to 92 KiB.
Tested in QEMU and on Thinkpad T500.
Change-Id: I6feacdb911b52b946869bff369e03dcf72897c9f Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/coreinfo/Kconfig M payloads/coreinfo/Makefile 2 files changed, 14 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/38293/10
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 11:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig File payloads/coreinfo/Kconfig:
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig@6... PS2, Line 61: default n
I’d default to yes, as less size is more important for firmware purposes.
For now I think no is a safe default - it can enabled in a custom config if the space is really needed.
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig@6... PS2, Line 65:
Please mention, if libpayload is then also indirectly built with LTO.
No, this will only change it for coreinfo, libpayload needs to be enabled separately.
https://review.coreboot.org/c/coreboot/+/38293/3/payloads/coreinfo/Kconfig File payloads/coreinfo/Kconfig:
https://review.coreboot.org/c/coreboot/+/38293/3/payloads/coreinfo/Kconfig@6... PS3, Line 60: bool "Use link time optimization"
Isn't it experimental?
I tested this in QEMU and hardware, so it should be reasonably safe (it's also turned off by default, so anyone who feels like the extra risk can enable it themselves).
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig File payloads/coreinfo/Kconfig:
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig@6... PS2, Line 61: default n
For now I think no is a safe default - it can enabled in a custom config if the space is really need […]
Ack
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 12: Code-Review+1
tested successfully for system76/lemp9. builds fine, boots fine
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 12: Code-Review+2
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
Patch Set 13:
(2 comments)
Conversation has died down, marking as resolved.
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig File payloads/coreinfo/Kconfig:
https://review.coreboot.org/c/coreboot/+/38293/2/payloads/coreinfo/Kconfig@6... PS2, Line 65:
No, this will only change it for coreinfo, libpayload needs to be enabled separately.
Done
https://review.coreboot.org/c/coreboot/+/38293/3/payloads/coreinfo/Kconfig File payloads/coreinfo/Kconfig:
https://review.coreboot.org/c/coreboot/+/38293/3/payloads/coreinfo/Kconfig@6... PS3, Line 60: bool "Use link time optimization"
I tested this in QEMU and hardware, so it should be reasonably safe (it's also turned off by default […]
Done
Stefan Reinauer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38293 )
Change subject: coreinfo: Add support for link time optimization ......................................................................
coreinfo: Add support for link time optimization
This introduces a Kconfig option for compiling coreinfo with LTO. This option can be used independently of LTO in libpayload, though will benefit most if that is enabled as well. If both are enabled, the final size of coreinfo.elf is reduced from 95 KiB to 92 KiB.
Tested in QEMU and on Thinkpad T500.
Change-Id: I6feacdb911b52b946869bff369e03dcf72897c9f Signed-off-by: Jacob Garber jgarber1@ualberta.ca Reviewed-on: https://review.coreboot.org/c/coreboot/+/38293 Reviewed-by: Michael Niewöhner foss@mniewoehner.de Reviewed-by: Martin Roth martinroth@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M payloads/coreinfo/Kconfig M payloads/coreinfo/Makefile 2 files changed, 14 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Michael Niewöhner: Looks good to me, but someone else must approve
diff --git a/payloads/coreinfo/Kconfig b/payloads/coreinfo/Kconfig index eafb879..2c1f91c 100644 --- a/payloads/coreinfo/Kconfig +++ b/payloads/coreinfo/Kconfig @@ -42,6 +42,15 @@ help The version number of this payload.
+config LTO + bool "Use link time optimization (LTO)" + default n + help + Compile with link time optimization. This can often decrease the + final binary size, but may increase compilation time. This option + is most effective when LTO is also enabled in libpayload, which + is done separately. + endmenu
menu "Modules" diff --git a/payloads/coreinfo/Makefile b/payloads/coreinfo/Makefile index d842b46..cd58f39 100644 --- a/payloads/coreinfo/Makefile +++ b/payloads/coreinfo/Makefile @@ -76,9 +76,13 @@ include $(src)/.config real-all: $(TARGET)
+ifeq ($(CONFIG_LTO),y) +CFLAGS += -flto +endif + $(TARGET): $(src)/.config $(coreinfo_obj)/config.h $(OBJS) libpayload printf " LPCC $(subst $(CURDIR)/,,$(@)) (LINK)\n" - $(LPCC) -o $@ $(OBJS) + $(LPCC) $(CFLAGS) -o $@ $(OBJS) $(OBJCOPY) --only-keep-debug $@ $(TARGET).debug $(OBJCOPY) --strip-debug $@ $(OBJCOPY) --add-gnu-debuglink=$(TARGET).debug $@