Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
libpayload: Add support for link time optimization
Link time optimization is a technique for whole-program optimization. Instead of doing code generation during compilation, the compiler saves its intermediate representation to the object files. During the final linking step, it will then merge all the object files together and perform optimizations on the entire program. This can often reduce the final binary size, but also may increase the total compilation time.
This patch introduces a Kconfig option for enabling link time optimization in libpayload. Since libpayload does no linking of its own, its LTO archive files will contain only IR and no generated code. Downstream projects will need to use LTO-aware tools when manipulating the archives (eg. gcc-ar and gcc-nm), but otherwise do not need to use LTO themselves -- the compiler will recognize which files are LTO and which are not, so enabling this option should mostly be "drop in".
For example, when building coreinfo.elf using tinycurses libpayload:
binary size compilation time default 149K 11.49s LTO 125K 10.36s
In this case the total compilation time was actually shorter -- despite the final linking step taking longer, this was offset by the shorter compilation times for each individual file (since there is no code gen until the very end).
Change-Id: I048f2ff6298ed0d891098942e1e8b29d35487b91 Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/libpayload/Kconfig M payloads/libpayload/Makefile.inc 2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/38291/1
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index f7501e3..9312f19 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -80,6 +80,13 @@
endchoice
+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. + config REMOTEGDB bool "Remote GDB stub" default n diff --git a/payloads/libpayload/Makefile.inc b/payloads/libpayload/Makefile.inc index 1b7986c..ecf7937 100644 --- a/payloads/libpayload/Makefile.inc +++ b/payloads/libpayload/Makefile.inc @@ -65,6 +65,10 @@ CFLAGS += -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wimplicit-fallthrough CFLAGS += -Wstrict-aliasing -Wshadow -Werror
+ifeq ($(CONFIG_LP_LTO),y) +CFLAGS += -flto=$(CPUS) -fuse-linker-plugin -fno-fat-lto-objects +endif + $(obj)/libpayload-config.h: $(KCONFIG_AUTOHEADER) cmp $@ $< 2>/dev/null || cp $< $@
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38291
to look at the new patch set (#3).
Change subject: libpayload: Add support for link time optimization ......................................................................
libpayload: Add support for link time optimization
Link time optimization is a technique for whole-program optimization. Instead of doing code generation during compilation, the compiler saves its intermediate representation to the object files. During the final linking step, it will then merge all the object files together and perform optimizations on the entire program. This can often reduce the final binary size, but also may increase the total compilation time.
This patch introduces a Kconfig option for enabling link time optimization in libpayload. Since libpayload does no linking of its own, its LTO archive files will contain only IR and no generated code. Downstream projects will need to use LTO-aware tools when manipulating the archives (eg. gcc-ar and gcc-nm), but otherwise do not need to use LTO themselves -- the compiler will recognize which files are LTO and which are not, so enabling this option should mostly be "drop in".
For example, when building coreinfo.elf using tinycurses libpayload:
binary size compilation time default 114 KiB 11.49s LTO 95 KiB 10.36s
In this case the total compilation time was actually shorter -- despite the final linking step taking longer, this was offset by the shorter compilation times for each individual file (since there is no code gen until the very end).
Change-Id: I048f2ff6298ed0d891098942e1e8b29d35487b91 Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/libpayload/Kconfig M payloads/libpayload/Makefile.inc 2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/38291/3
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... PS3, Line 69: $(CPUS) Was there any reason in particular to use $(CPUS) instead of auto? AFAICT they will be identical in this case since auto should take whatever was passed in via -j.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... PS3, Line 69: $(CPUS)
Was there any reason in particular to use $(CPUS) instead of auto? AFAICT they will be identical in […]
Might be important if not all CPU cores are to be used when building this.
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... PS3, Line 69: $(CPUS)
Might be important if not all CPU cores are to be used when building this.
In GCC 10 LTO linking will automatically detect the number of available cores and do the linking in parallel (regardless of what was set using -j). This just copies that behaviour since it will become the default anyway. (I'm also not sure what the auto flag is. Were you thinking of jobserver?)
https://www.phoronix.com/scan.php?page=news_item&px=GCC-10-LTO-flto-Avai...
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... PS3, Line 69: $(CPUS)
In GCC 10 LTO linking will automatically detect the number of available cores and do the linking in […]
Regarding -flto=auto, the man page for gcc 9.2.1 (which I happen to have installed) says: "Use -flto=auto to use GNU make's job server, if available, or otherwise fall back to autodetection of the number of CPU threads present in your system."
I guess we may as well be explicit about what we want, i.e. what CPUS= is set to on the command-line). Whether or not GCC overrides that is another matter.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 3:
@Chromium-team, will your CI system catch possible regressions in depthcharge due to this change? I am in favor of getting this into master by default, as the next release is still some time away.
Jacob, it’d be great, if you sent a request for testing to the mailing list to make people aware of this, and test it, and report possible regressions, which should then be fixed later on.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... PS3, Line 69: $(CPUS)
Regarding -flto=auto, the man page for gcc 9.2. […]
I'm confused. The GCC manual states
If you specify the optional N, the optimization and code generation done at link time is executed in parallel using N parallel jobs by utilizing an installed 'make' program.
But isn't this `Makefile.inc` only about compile-time? Does it have an effect here?
Hmmm, I also can't find anything about `auto`. Am I looking at the wrong option?
Hello ron minnich, Julius Werner, David Hendricks, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38291
to look at the new patch set (#4).
Change subject: libpayload: Add support for link time optimization ......................................................................
libpayload: Add support for link time optimization
Link time optimization is a technique for whole-program optimization. Instead of doing code generation during compilation, the compiler saves its intermediate representation to the object files. During the final linking step, it will then merge all the object files together and perform optimizations on the entire program. This can often reduce the final binary size, but also may increase the total compilation time.
This patch introduces a Kconfig option for enabling link time optimization in libpayload. Since libpayload does no linking of its own, its LTO archive files will contain only IR and no generated code. Downstream projects will need to use LTO-aware tools when manipulating the archives (eg. gcc-ar and gcc-nm), but otherwise do not need to use LTO themselves -- the compiler will recognize which files are LTO and which are not, so enabling this option should mostly be "drop in".
For example, when building coreinfo.elf using tinycurses libpayload:
binary size compilation time default 114 KiB 11.49s LTO 95 KiB 10.36s
In this case the total compilation time was actually shorter -- despite the final linking step taking longer, this was offset by the shorter compilation times for each individual file (since there is no code gen until the very end).
Change-Id: I048f2ff6298ed0d891098942e1e8b29d35487b91 Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/libpayload/Kconfig M payloads/libpayload/Makefile.inc 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/38291/4
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/3/payloads/libpayload/Makefil... PS3, Line 69: $(CPUS)
I'm confused. The GCC manual states […]
Hmmm, now I'm confused too. I think I'll just leave this option off for now, since its behaviour will be changing eventually anyway and AFAICT it doesn't change the build time. (I also can't find anything about auto in my man page.)
Hello ron minnich, Julius Werner, David Hendricks, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38291
to look at the new patch set (#5).
Change subject: libpayload: Add support for link time optimization ......................................................................
libpayload: Add support for link time optimization
Link time optimization is a technique for whole-program optimization. Instead of doing code generation during compilation, the compiler saves its intermediate representation to the object files. During the final linking step, it will then merge all the object files together and perform optimizations on the entire program. This can often reduce the final binary size, but also may increase the total compilation time.
This patch introduces a Kconfig option for enabling link time optimization in libpayload. Since libpayload does no linking of its own, its LTO archive files will contain only IR and no generated code. Downstream projects will need to use LTO-aware tools when manipulating the archives (eg. gcc-ar and gcc-nm), but otherwise do not need to use LTO themselves -- the compiler will recognize which files are LTO and which are not, so enabling this option should mostly be "drop in".
For example, when building coreinfo.elf using tinycurses libpayload:
binary size compilation time default 114 KiB 11.49s LTO 95 KiB 10.36s
In this case the total compilation time was actually shorter -- despite the final linking step taking longer, this was offset by the shorter compilation times for each individual file (since there is no code gen until the very end).
Change-Id: I048f2ff6298ed0d891098942e1e8b29d35487b91 Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/libpayload/Kconfig M payloads/libpayload/Makefile.inc 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/38291/5
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 5: Code-Review+1
I still think this should be enabled by default.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... PS6, Line 69: -fuse-linker-plugin -fno-fat-lto-objects Can you add a comment here or in the commit message, why the last two switches are needed?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... PS6, Line 69: -fuse-linker-plugin -fno-fat-lto-objects
Can you add a comment here or in the commit message, why the last two switches are needed?
use-linker-plugin tells the compiler to tell the linker to use plugins, among them, the linker's LTO support. I don't think it's necessary everywhere, but some configurations are better off with it.
no-fat-lto-objects: "fat object files" are those that contain both the in-compiler data representation used for LTO and object code like "traditional" objects. I'd rather go for enforcing fat-lto-objects though: Experience on Chrome EC has shown that the compiler skips a few steps (those relevant for assembler generation) in LTO mode and with them, a bunch of warnings. See https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1648924 where we force enabled fat objects there.
Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Paul Menzel, David Hendricks, Julius Werner, ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38291
to look at the new patch set (#9).
Change subject: libpayload: Add support for link time optimization ......................................................................
libpayload: Add support for link time optimization
Link time optimization is a technique for whole-program optimization. Instead of doing code generation during compilation, the compiler saves its intermediate representation to the object files. During the final linking step, it will then merge all the object files together and perform optimizations on the entire program. This can often reduce the final binary size, but also may increase the total compilation time.
This patch introduces a Kconfig option for enabling link time optimization in libpayload. Since libpayload does no linking of its own, its LTO archive files will contain only IR and no generated code. Downstream projects will need to use LTO-aware tools when manipulating the archives (eg. gcc-ar and gcc-nm), but otherwise do not need to use LTO themselves -- the compiler will recognize which files are LTO and which are not, so enabling this option should mostly be "drop in".
For example, when building coreinfo.elf using tinycurses libpayload:
binary size compilation time default 114 KiB 11.49s LTO 95 KiB 10.36s
In this case the total compilation time was actually shorter -- despite the final linking step taking longer, this was offset by the shorter compilation times for each individual file (since there is no code gen until the very end).
Change-Id: I048f2ff6298ed0d891098942e1e8b29d35487b91 Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/libpayload/Kconfig M payloads/libpayload/Makefile.inc 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/38291/9
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 9: Code-Review+1
Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Paul Menzel, David Hendricks, Julius Werner, ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38291
to look at the new patch set (#10).
Change subject: libpayload: Add support for link time optimization ......................................................................
libpayload: Add support for link time optimization
Link time optimization is a technique for whole-program optimization. Instead of doing code generation during compilation, the compiler saves its intermediate representation to the object files. During the final linking step, it will then merge all the object files together and perform optimizations on the entire program. This can often reduce the final binary size, but also may increase the total compilation time.
This patch introduces a Kconfig option for enabling link time optimization in libpayload. Since libpayload does no linking of its own, its LTO archive files will contain only IR and no generated code. Downstream projects will need to use LTO-aware tools when manipulating the archives (eg. gcc-ar and gcc-nm), but otherwise do not need to use LTO themselves -- the compiler will recognize which files are LTO and which are not, so enabling this option should mostly be "drop in".
For example, when building coreinfo.elf using tinycurses libpayload:
binary size compilation time default 114 KiB 11.49s LTO 95 KiB 10.36s
In this case the total compilation time was actually shorter -- despite the final linking step taking longer, this was offset by the shorter compilation times for each individual file (since there is no code gen until the very end).
Change-Id: I048f2ff6298ed0d891098942e1e8b29d35487b91 Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M payloads/libpayload/Kconfig M payloads/libpayload/Makefile.inc 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/38291/10
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... PS6, Line 69: -fuse-linker-plugin -fno-fat-lto-objects
use-linker-plugin tells the compiler to tell the linker to use plugins, among them, the linker's LTO […]
For fat-lto-objects, I think it's more likely that the inclusion of the compiler data rather than the object code is triggering the extra warnings (since normal builds also include the object code but don't generate the warnings). It didn't generate any extra warnings when enabled on the coreboot tree so I think it should be safe to stick with the defaults.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: 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/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 13:
(1 comment)
Conversation has died down, marking as resolved (speak now if you disagree!)
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38291/6/payloads/libpayload/Makefil... PS6, Line 69: -fuse-linker-plugin -fno-fat-lto-objects
For fat-lto-objects, I think it's more likely that the inclusion of the compiler data rather than th […]
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 13: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38291/13//COMMIT_MSG@33 PS13, Line 33: until the very end). This is potentially out of date now because fat objects are allowed?
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38291/13//COMMIT_MSG@33 PS13, Line 33: until the very end).
This is potentially out of date now because fat objects are allowed?
Resolving, because compile/link time estimates are not all too relevant in this case anyways.
Stefan Reinauer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
libpayload: Add support for link time optimization
Link time optimization is a technique for whole-program optimization. Instead of doing code generation during compilation, the compiler saves its intermediate representation to the object files. During the final linking step, it will then merge all the object files together and perform optimizations on the entire program. This can often reduce the final binary size, but also may increase the total compilation time.
This patch introduces a Kconfig option for enabling link time optimization in libpayload. Since libpayload does no linking of its own, its LTO archive files will contain only IR and no generated code. Downstream projects will need to use LTO-aware tools when manipulating the archives (eg. gcc-ar and gcc-nm), but otherwise do not need to use LTO themselves -- the compiler will recognize which files are LTO and which are not, so enabling this option should mostly be "drop in".
For example, when building coreinfo.elf using tinycurses libpayload:
binary size compilation time default 114 KiB 11.49s LTO 95 KiB 10.36s
In this case the total compilation time was actually shorter -- despite the final linking step taking longer, this was offset by the shorter compilation times for each individual file (since there is no code gen until the very end).
Change-Id: I048f2ff6298ed0d891098942e1e8b29d35487b91 Signed-off-by: Jacob Garber jgarber1@ualberta.ca Reviewed-on: https://review.coreboot.org/c/coreboot/+/38291 Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Martin Roth martinroth@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M payloads/libpayload/Kconfig M payloads/libpayload/Makefile.inc 2 files changed, 12 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Nico Huber: Looks good to me, approved
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index b5dc9a3..f5b81a9 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -79,6 +79,14 @@
endchoice
+config LTO + bool "Use link time optimization (LTO)" + default n + depends on COMPILER_GCC + help + Compile with link time optimization. This can often decrease the + final binary size, but may increase compilation time. + config REMOTEGDB bool "Remote GDB stub" default n diff --git a/payloads/libpayload/Makefile.inc b/payloads/libpayload/Makefile.inc index 2acf226..1b2a883 100644 --- a/payloads/libpayload/Makefile.inc +++ b/payloads/libpayload/Makefile.inc @@ -64,6 +64,10 @@ CFLAGS += -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wimplicit-fallthrough CFLAGS += -Wstrict-aliasing -Wshadow -Werror
+ifeq ($(CONFIG_LP_LTO),y) +CFLAGS += -flto +endif + $(obj)/libpayload-config.h: $(KCONFIG_AUTOHEADER) cmp $@ $< 2>/dev/null || cp $< $@
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38291 )
Change subject: libpayload: Add support for link time optimization ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38291/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38291/13//COMMIT_MSG@33 PS13, Line 33: until the very end).
Resolving, because compile/link time estimates are not all too relevant in this case anyways.
Fat objects were only enabled to see if they raised any more warnings (which they didn't), so this is still accurate.