Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
[TEST] Add support for link time optimization
- Code generation is not done until after linking, so the compiler must be invoked at all linking stages instead of the linker. As a consequence all linker arguments must be prefixed with -Wl. - Partial linking is not supported. Instead, object files are collected into thin archives that are linked instead. - The dead_code() macro causes linking errors, since dead functions aren't optimized out until after linking has begun. This macro could be replaced with the preprocessor if necessary, or just disabled for LTO builds. - Wrapping libgcc functions causes a symbol mismatch when using LTO. Wrapping these functions was originally done to support alternate regparam values, though AFAICT this isn't used anywhere.
Using LTO leads to a ~10% decrease in stage size for QEMU and ~18% for the Thinkpad T500, and both targets boot successfully.
Change-Id: I48c31ea8b1b57276125cffdac44c7c16642547ac Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M src/Kconfig M src/arch/x86/Makefile.inc M src/cpu/x86/smm/Makefile.inc M src/include/assert.h M src/lib/Makefile.inc M src/lib/gcc.c M toolchain.inc M util/xcompile/xcompile 9 files changed, 42 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38989/1
diff --git a/Makefile.inc b/Makefile.inc index 1f18726..b85c11b 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -434,6 +434,10 @@ endif endif
+ifeq ($(CONFIG_LTO),y) +CFLAGS_common += -flto -fuse-linker-plugin -fno-fat-lto-objects +endif + ADAFLAGS_common += -gnatp ADAFLAGS_common += -Wuninitialized -Wall -Werror ADAFLAGS_common += -pipe -g -nostdinc @@ -478,7 +482,7 @@ # Disable style checks for now ADAFLAGS_common += -gnatyN
-LDFLAGS_common := --gc-sections -nostdlib -nostartfiles -static --emit-relocs +LDFLAGS_common := -nostdlib -nostartfiles -static -Wl,--emit-relocs,--gc-sections
ifeq ($(CONFIG_WARNINGS_ARE_ERRORS),y) CFLAGS_common += -Werror diff --git a/src/Kconfig b/src/Kconfig index f75f942..4b81818 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -116,6 +116,15 @@
For details see https://ccache.samba.org.
+config LTO + bool "Use link time optimization" + # Enable now for testing + default y + depends on COMPILER_GCC + help + Compile with link time optimization. This can often decrease the + final binary size, but may increase compilation time. + config FMD_GENPARSER bool "Generate flashmap descriptor parser using flex and bison" default n diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 534f2ce..75f96d2 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -80,7 +80,7 @@
$$(objcbfs)/$(1).debug: $$$$($(1)-libs) $$$$($(1)-objs) @printf " LINK $$(subst $$(obj)/,,$$(@))\n" - $$(LD_$(1)) $$(LDFLAGS_$(1)) -o $$@ -L$$(obj) $$(COMPILER_RT_FLAGS_$(1)) --whole-archive --start-group $$(filter-out %.ld,$$($(1)-objs)) $$($(1)-libs) --no-whole-archive $$(COMPILER_RT_$(1)) --end-group -T $(call src-to-obj,$(1),$(dir)/memlayout.ld) --oformat $(2) + $$(LD_$(1)) $$(LDFLAGS_$(1)) -o $$@ -L$$(obj) $$(COMPILER_RT_FLAGS_$(1)) -Wl,--whole-archive,--start-group $$(filter-out %.ld,$$($(1)-objs)) $$($(1)-libs) -Wl,--no-whole-archive $$(COMPILER_RT_$(1)) -Wl,--end-group -T $(call src-to-obj,$(1),$(dir)/memlayout.ld) -Wl,--oformat=$(2) -LANG=C LC_ALL= $$(OBJCOPY_$(1)) --only-section .illegal_globals $$(@) $$(objcbfs)/$(1)_null.offenders >/dev/null 2>&1 if [ -z "$$$$($$(NM_$(1)) $$(objcbfs)/$(1)_null.offenders 2>&1 | grep 'no symbols')" ];then \ echo "Forbidden global variables in $(1):"; \ @@ -212,11 +212,11 @@ postcar-y += postcar.c postcar-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
-LDFLAGS_postcar += -Map $(objcbfs)/postcar.map +LDFLAGS_postcar += -Wl,-Map,$(objcbfs)/postcar.map
$(objcbfs)/postcar.debug: $$(postcar-objs) @printf " LINK $(subst $(obj)/,,$(@))\n" - $(LD_postcar) $(LDFLAGS_postcar) -o $@ -L$(obj) $(COMPILER_RT_FLAGS_postcar) --whole-archive --start-group $(filter-out %.ld,$^) --no-whole-archive $(COMPILER_RT_postcar) --end-group -T $(call src-to-obj,postcar,src/arch/x86/memlayout.ld) + $(LD_postcar) $(LDFLAGS_postcar) -o $@ -L$(obj) $(COMPILER_RT_FLAGS_postcar) -Wl,--whole-archive,--start-group $(filter-out %.ld,$^) -Wl,--no-whole-archive $(COMPILER_RT_postcar) -Wl,--end-group -T $(call src-to-obj,postcar,src/arch/x86/memlayout.ld)
$(objcbfs)/postcar.elf: $(objcbfs)/postcar.debug.rmod cp $< $@ @@ -309,18 +309,20 @@
endif
-$(objcbfs)/ramstage.debug: $(objgenerated)/ramstage.o $(call src-to-obj,ramstage,src/arch/x86/memlayout.ld) - @printf " CC $(subst $(obj)/,,$(@))\n" - $(LD_ramstage) $(CPPFLAGS) $(LDFLAGS_ramstage) -o $@ -L$(obj) $< -T $(call src-to-obj,ramstage,src/arch/x86/memlayout.ld) - -$(objgenerated)/ramstage.o: $$(ramstage-objs) $(COMPILER_RT_ramstage) $$(ramstage-libs) - @printf " CC $(subst $(obj)/,,$(@))\n" ifeq ($(CONFIG_ARCH_RAMSTAGE_X86_32),y) - $(LD_ramstage) -m elf_i386 -r -o $@ $(COMPILER_RT_FLAGS_ramstage) --whole-archive --start-group $(filter-out %.ld,$(ramstage-objs)) $(ramstage-libs) --no-whole-archive $(COMPILER_RT_ramstage) --end-group +LDFLAGS_ramstage += -Wl,-m,elf_i386 else - $(LD_ramstage) -m elf_x86_64 -r -o $@ $(COMPILER_RT_FLAGS_ramstage) --whole-archive --start-group $(filter-out %.ld,$(ramstage-objs)) $(ramstage-libs) --no-whole-archive $(COMPILER_RT_ramstage) --end-group +LDFLAGS_ramstage += -Wl,-m,elf_x86_64 endif
+$(objcbfs)/ramstage.debug: $(objgenerated)/ramstage.a $(call src-to-obj,ramstage,src/arch/x86/memlayout.ld) + @printf " CC $(subst $(obj)/,,$(@))\n" + $(LD_ramstage) $(CPPFLAGS) $(LDFLAGS_ramstage) $(COMPILER_RT_FLAGS_ramstage) -o $@ -L$(obj) -Wl,--whole-archive,--start-group $< -Wl,--no-whole-archive $(COMPILER_RT_ramstage) -Wl,--end-group -T $(call src-to-obj,ramstage,src/arch/x86/memlayout.ld) + +$(objgenerated)/ramstage.a: $$(ramstage-objs) $$(ramstage-libs) + @printf " AR $(subst $(obj)/,,$(@))\n" + $(AR_ramstage) rcT $@ $(filter-out %.ld,$(ramstage-objs)) $(ramstage-libs) + endif # CONFIG_ARCH_RAMSTAGE_X86_32 / CONFIG_ARCH_RAMSTAGE_X86_64
smm-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c diff --git a/src/cpu/x86/smm/Makefile.inc b/src/cpu/x86/smm/Makefile.inc index 11a4e67..f582a31 100644 --- a/src/cpu/x86/smm/Makefile.inc +++ b/src/cpu/x86/smm/Makefile.inc @@ -25,8 +25,8 @@ smm-generic-ccopts += -D__SMM__ smm-c-deps:=$$(OPTION_TABLE_H)
-$(obj)/smm/smm.o: $$(smm-objs) $(COMPILER_RT_smm) - $(LD_smm) -nostdlib -r -o $@ $(COMPILER_RT_FLAGS_smm) --whole-archive --start-group $(smm-objs) --no-whole-archive $(COMPILER_RT_smm) --end-group +$(obj)/smm/smm.a: $$(smm-objs) + $(AR_smm) rcT $@ $^
# change to the target path because objcopy will use the path name in its # ELF symbol names. @@ -53,7 +53,7 @@ # SMM Stub Module. The stub is used as a trampoline for relocation and normal # SMM handling. $(obj)/smmstub/smmstub.o: $$(smmstub-objs) $(COMPILER_RT_smmstub) - $(LD_smmstub) -nostdlib -r -o $@ $(COMPILER_RT_FLAGS_smmstub) --whole-archive --start-group $(smmstub-objs) --no-whole-archive $(COMPILER_RT_smmstub) --end-group + $(LD_smmstub) -nostdlib -r -o $@ $(COMPILER_RT_FLAGS_smmstub) -Wl,--whole-archive,--start-group $(smmstub-objs) -Wl,--no-whole-archive $(COMPILER_RT_smmstub) -Wl,--end-group
# Link the SMM stub module with a 0-byte heap. ifeq ($(CONFIG_ARCH_RAMSTAGE_X86_32),y) @@ -72,9 +72,9 @@ # C-based SMM handler.
ifeq ($(CONFIG_ARCH_RAMSTAGE_X86_32),y) -$(eval $(call rmodule_link,$(obj)/smm/smm.elf, $(obj)/smm/smm.o, $(CONFIG_SMM_MODULE_HEAP_SIZE),x86_32)) +$(eval $(call rmodule_link,$(obj)/smm/smm.elf, $(obj)/smm/smm.a, $(CONFIG_SMM_MODULE_HEAP_SIZE),x86_32)) else -$(eval $(call rmodule_link,$(obj)/smm/smm.elf, $(obj)/smm/smm.o, $(CONFIG_SMM_MODULE_HEAP_SIZE),x86_64)) +$(eval $(call rmodule_link,$(obj)/smm/smm.elf, $(obj)/smm/smm.a, $(CONFIG_SMM_MODULE_HEAP_SIZE),x86_64)) endif
$(obj)/smm/smm: $(obj)/smm/smm.elf.rmod @@ -82,8 +82,8 @@
else # CONFIG_SMM_TSEG
-$(obj)/smm/smm: $(obj)/smm/smm.o $(src)/cpu/x86/smm/smm.ld - $(LD_smm) $(LDFLAGS_smm) -o $(obj)/smm/smm.elf -T $(src)/cpu/x86/smm/smm.ld $(obj)/smm/smm.o +$(obj)/smm/smm: $(obj)/smm/smm.a $(src)/cpu/x86/smm/smm.ld + $(LD_smm) $(LDFLAGS_smm) -o $(obj)/smm/smm.elf -T $(src)/cpu/x86/smm/smm.ld -Wl,--whole-archive,--start-group $(obj)/smm/smm.a -Wl,--no-whole-archive $(COMPILER_RT_smm) --end-group $(NM_smm) -n $(obj)/smm/smm.elf | sort > $(obj)/smm/smm.map $(OBJCOPY_smm) -O binary $(obj)/smm/smm.elf $@
diff --git a/src/include/assert.h b/src/include/assert.h index e0db0bc..6901953 100644 --- a/src/include/assert.h +++ b/src/include/assert.h @@ -61,7 +61,8 @@ dead_code_assertion_failed_at_line_##line(); \ } while (0) #define _dead_code(line) __dead_code(line) -#define dead_code() _dead_code(__LINE__) +//#define dead_code() _dead_code(__LINE__) +#define dead_code()
/* This can be used in the context of an expression of type 'type'. */ #define dead_code_t(type) ({ \ diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 2333f64..ac6d624 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -291,7 +291,7 @@ # rmdoule is named $(1).rmod define rmodule_link $(strip $(1)): $(strip $(2)) $$(COMPILER_RT_rmodules_$(4)) $(call src-to-obj,rmodules_$(4),src/lib/rmodule.ld) | $$(RMODTOOL) - $$(LD_rmodules_$(4)) $$(LDFLAGS_rmodules_$(4)) $(RMODULE_LDFLAGS) -T $(call src-to-obj,rmodules_$(4),src/lib/rmodule.ld) --defsym=__heap_size=$(strip $(3)) -o $$@ --whole-archive --start-group $(filter-out %.ld,$(2)) --end-group + $$(LD_rmodules_$(4)) $$(LDFLAGS_rmodules_$(4)) $(RMODULE_LDFLAGS) -T $(call src-to-obj,rmodules_$(4),src/lib/rmodule.ld) -Wl,--defsym=__heap_size=$(strip $(3)) -o $$@ -Wl,--whole-archive,--start-group $(filter-out %.ld,$(2)) -Wl,--no-whole-archive $$(COMPILER_RT_rmodules_$(4)) -Wl,--end-group $$(NM_rmodules_$(4)) -n $$@ > $$(basename $$@).map endef
diff --git a/src/lib/gcc.c b/src/lib/gcc.c index 5a93f45..66a7788 100644 --- a/src/lib/gcc.c +++ b/src/lib/gcc.c @@ -24,6 +24,7 @@ /* TODO: maybe this code should move to arch/x86 as architecture * specific implementations may vary */ +#if 0 #define WRAP_LIBGCC_CALL(type, name) \ asmlinkage type __real_##name(type a, type b); \ type __wrap_##name(type a, type b); \ @@ -33,3 +34,4 @@ WRAP_LIBGCC_CALL(unsigned long long, __udivdi3) WRAP_LIBGCC_CALL(long long, __moddi3) WRAP_LIBGCC_CALL(unsigned long long, __umoddi3) +#endif diff --git a/toolchain.inc b/toolchain.inc index 865227b..f52a518 100644 --- a/toolchain.inc +++ b/toolchain.inc @@ -117,7 +117,7 @@ $(error Check your .config file for CONFIG_ARCH_$(1)_* settings)) CC_$(1) := $(CC_$(2)) GCC_$(1) := $(GCC_CC_$(2)) -LD_$(1) := $(LD_$(2)) +LD_$(1) := $(CC_$(2)) NM_$(1) := $(NM_$(2)) AR_$(1) := $(AR_$(2)) GNATBIND_$(1) := $(GNATBIND_$(2)) @@ -130,7 +130,7 @@ CPPFLAGS_$(1) = $$(CPPFLAGS_common) $$(CPPFLAGS_$(2)) -D__ARCH_$(2)__ COMPILER_RT_$(1) := $$(COMPILER_RT_$(2)) COMPILER_RT_FLAGS_$(1) := $$(COMPILER_RT_FLAGS_$(2)) -LDFLAGS_$(1) = $$(LDFLAGS_common) $$(LDFLAGS_$(2)) +LDFLAGS_$(1) = $$(CFLAGS_$(1)) $$(LDFLAGS_common) $$(LDFLAGS_$(2)) endef
# define_class: Allows defining any program as dynamic class and compiler tool diff --git a/util/xcompile/xcompile b/util/xcompile/xcompile index 3203d71..8515813 100755 --- a/util/xcompile/xcompile +++ b/util/xcompile/xcompile @@ -363,7 +363,7 @@ TCLIST="i386 x86_64" TWIDTH="32" TABI="elf" - CC_RT_EXTRA_GCC="--wrap __divdi3 --wrap __udivdi3 --wrap __moddi3 --wrap __umoddi3" + #CC_RT_EXTRA_GCC="-Wl,--wrap=__divdi3,--wrap=__udivdi3,--wrap=__moddi3,--wrap=__umoddi3" }
arch_config_ppc64() {
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 3:
(2 comments)
(How do you guys manage to comment on this without clearing the WIP flag? Gerrit always forces me to click "START REVIEW" when I'm trying to reply...)
https://review.coreboot.org/c/coreboot/+/38989/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38989/3//COMMIT_MSG@14 PS3, Line 14: - The dead_code() macro causes linking errors, since dead functions What exactly fails when you enable this? I took your patch, reverted the assert.h change and built GOOGLE_SCARLET (which uses that macro a bunch), and I didn't get any errors.
I like that macro a lot and would like to keep it alive. It serves a function that isn't easily duplicated with other means (you cannot do the same thing with just the preprocessor without making a giant mess).
https://review.coreboot.org/c/coreboot/+/38989/2/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/38989/2/src/Kconfig@123 PS2, Line 123: depends on COMPILER_GCC
Assuming the board builds and boots with Clang first, we would need to use the llvm-* versions of bi […]
Isn't clang not working anyway (and I believe efforts to fix it have pretty much stopped)? I think we've decided to support GCC only for the foreseeable future.
Hello ron minnich, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38989
to look at the new patch set (#4).
Change subject: [TEST] Add support for link time optimization ......................................................................
[TEST] Add support for link time optimization
- Code generation is not done until after linking, so the compiler must be invoked at all linking stages instead of the linker. As a consequence all linker arguments must be prefixed with -Wl. - Partial linking is not supported. Instead, object files are collected into thin archives that are linked instead. - Wrapping libgcc functions causes a symbol mismatch when using LTO. Wrapping these functions was originally done to support alternate regparam values, though AFAICT this isn't used anywhere.
Using LTO leads to a ~10% decrease in stage size for QEMU and ~18% for the Thinkpad T500, and both targets boot successfully.
Change-Id: I48c31ea8b1b57276125cffdac44c7c16642547ac Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M src/Kconfig M src/arch/arm/Makefile.inc M src/arch/arm64/Makefile.inc M src/arch/ppc64/Makefile.inc M src/arch/riscv/Makefile.inc M src/arch/x86/Makefile.inc M src/cpu/x86/smm/Makefile.inc M src/lib/Makefile.inc M src/lib/gcc.c M toolchain.inc M util/xcompile/xcompile 12 files changed, 61 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38989/4
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38989/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38989/3//COMMIT_MSG@14 PS3, Line 14: - The dead_code() macro causes linking errors, since dead functions
What exactly fails when you enable this? I took your patch, reverted the assert. […]
I recall having some problems with it early on, but I can't reproduce them anymore. (They might have been triggered by the partial linking, which caused all sorts of strange errors.) I've reverted it for now and Jenkins seems happy, so it looks like it's not an issue.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 4:
Patch Set 3:
(2 comments)
(How do you guys manage to comment on this without clearing the WIP flag? Gerrit always forces me to click "START REVIEW" when I'm trying to reply...)
There is a great *SAVE* field on the bottom left.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38989/2/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/38989/2/src/Kconfig@123 PS2, Line 123: depends on COMPILER_GCC
Isn't clang not working anyway (and I believe efforts to fix it have pretty much stopped)? I think w […]
I am able to successfully build and boot QEMU i440fx with LLVM/Clang.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 4:
http://dpaste.com/00PM90M the board boots with current patch
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 4: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/38989/4/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/38989/4/src/Kconfig@120 PS4, Line 120: bool "Use link time optimization" Also add *(LTO)* at the end.
https://review.coreboot.org/c/coreboot/+/38989/4/src/Kconfig@122 PS4, Line 122: default y Could you split `default y` out to a separate commit please on top. That way the rest could be submitted to avoid bitrotting, and people could test more easily just turning this on in Kconfig.
Hello build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth, Paul Menzel, Julius Werner, ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38989
to look at the new patch set (#5).
Change subject: [TEST] Add support for link time optimization ......................................................................
[TEST] Add support for link time optimization
- Code generation is not done until after linking, so the compiler must be invoked at all linking stages instead of the linker. As a consequence all linker arguments must be prefixed with -Wl. - Partial linking is not supported. Instead, object files are collected into thin archives that are linked instead.
Using LTO leads to a ~10% decrease in stage size for QEMU and ~18% for the Thinkpad T500, and both targets boot successfully.
Change-Id: I48c31ea8b1b57276125cffdac44c7c16642547ac Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M src/Kconfig M src/arch/arm/Makefile.inc M src/arch/arm64/Makefile.inc M src/arch/ppc64/Makefile.inc M src/arch/riscv/Makefile.inc M src/arch/x86/Makefile.inc M src/cpu/x86/smm/Makefile.inc M src/lib/Makefile.inc M toolchain.inc M util/xcompile/xcompile 11 files changed, 59 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38989/5
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 6:
Any idea how to fix the error below for AGESA boards like ASRock E350M1?
``` LINK cbfs/fallback/romstage.debug /opt/xgcc/lib/gcc/i386-elf/8.3.0/../../../../i386-elf/bin/ld.bfd: section .illegal_globals VMA wraps around address space collect2: error: ld returned 1 exit status ```
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 6:
I worked around that error with the hack below.
``` $ git diff diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 1615e75c9a..d818205ab7 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -78,11 +78,7 @@ $(1)-S-ccopts += -I. $$(objcbfs)/$(1).debug: $$$$($(1)-libs) $$$$($(1)-objs) @printf " LINK $$(subst $$(obj)/,,$$(@))\n" $$(LD_$(1)) $$(LDFLAGS_$(1)) -o $$@ -L$$(obj) $$(COMPILER_RT_FLAGS_$(1)) -Wl,--whole-archive,--start-group $$(filter-out %.ld,$$($(1)-objs)) $$($(1)-libs) -Wl,--no-whole-archive $$(COMPILER_RT_$(1)) -Wl,--end-group -T $(call src-to-obj,$(1),$(dir)/memlayout.ld) -Wl,--oformat=$(2) - -LANG=C LC_ALL= $$(OBJCOPY_$(1)) --only-section .illegal_globals $$(@) $$(objcbfs)/$(1)_null.offenders >/dev/null 2>&1 - if [ -z "$$$$($$(NM_$(1)) $$(objcbfs)/$(1)_null.offenders 2>&1 | grep 'no symbols')" ];then \ - echo "Forbidden global variables in $(1):"; \ - $$(NM_$(1)) $$(objcbfs)/$(1)_null.offenders; false; \ - fi + -LANG=C LC_ALL= $$(OBJCOPY_$(1)) --only-section $$(@) $$(objcbfs)/$(1)_null.offenders >/dev/null 2>&1 endef
############################################################################### diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld index 92b26a0877..05e317dd29 100644 --- a/src/arch/x86/car.ld +++ b/src/arch/x86/car.ld @@ -83,10 +83,6 @@ */
. = 0xffffff00; -.illegal_globals . : { - *(EXCLUDE_FILE ("*/libagesa.*.a:" "*/romstage*/buildOpts.o" "*/romstage*/agesawrapper.o" "*/vendorcode/amd/agesa/*" "*/vendorcode/amd/cimx/*") .data) - *(EXCLUDE_FILE ("*/libagesa.*.a:" "*/romstage*/buildOpts.o" "*/romstage*/agesawrapper.o" "*/vendorcode/amd/agesa/*" "*/vendorcode/amd/cimx/*") .data.*) -}
_bogus = ASSERT((CONFIG_DCACHE_RAM_SIZE == 0) || (SIZEOF(.car.data) <= CONFIG_DCACHE_RAM_SIZE), "Cache as RAM area is too full"); #if CONFIG(PAGING_IN_CACHE_AS_RAM) ```
romstage is 9.3 % smaller, and ramstage around 11 %.
``` $ build/cbfstool build/coreboot.rom print FMAP REGION: COREBOOT Name Offset Type Size Comp cbfs master header 0x0 cbfs header 32 none fallback/romstage 0x80 stage 170260 none fallback/ramstage 0x29a00 stage 105668 none […] ```
No LTO:
``` fallback/romstage 0x80 stage 185012 none fallback/ramstage 0x2d3c0 stage 119027 none ```
With that, I reach the payload phase, but the drive connected to the SATA port is not detected by the payload (GRUB, SeaBIOS) and GNU/Linux (5.7-rc1 and 5.5) – also not with warm reboot. I cannot spot a difference in the coreboot log [1]. Booting from USB works.
[1]: https://paste.flashrom.org/view.php?id=3306
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 6:
Patch Set 6:
I worked around that error with the hack below.
Yes, this is a current limitation of LTO right now. Because the object files are all lumped together into a single unit, all information about where the symbols came from is lost, so EXCLUDE_FILE is unable of excluding the AGESA objects from the illegal_globals check. Tracing where a symbol came from has been implemented in LLVM [0], but I'm not sure if it's on the roadmap for GCC. For now it's probably best to disable LTO when compiling AGESA.
[0]: https://llvm.org/devmtg/2017-10/slides/LTOLinkerScriptsEdlerVonKoch.pdf
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 6:
Keith, as you also seem to care about image size, could you please test, if this still works on your Intel 440BX boards?
Size reduction for the asus/p2b (P2B-LS) seems also ten to 13 percent:
1. Without LTO:
``` fallback/romstage 0x80 stage 16412 none cpu_microcode_blob.bin 0x4100 microcode 83968 none fallback/ramstage 0x18980 stage 50347 none ```
2. With LTO:
``` fallback/romstage 0x80 stage 14212 none cpu_microcode_blob.bin 0x3880 microcode 83968 none fallback/ramstage 0x18100 stage 44882 none ```
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 6:
The build artifacts on the builders are gone, could you please rebase, so it’s tested again?
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 6:
Keith, as you also seem to care about image size, could you please test, if this still works on your Intel 440BX boards?
Size reduction for the asus/p2b (P2B-LS) seems also ten to 13 percent:
Without LTO:
fallback/romstage 0x80 stage 16412 none cpu_microcode_blob.bin 0x4100 microcode 83968 none fallback/ramstage 0x18980 stage 50347 noneWith LTO:
fallback/romstage 0x80 stage 14212 none cpu_microcode_blob.bin 0x3880 microcode 83968 none fallback/ramstage 0x18100 stage 44882 none
Here's my reduction results (in bytes) Bootblock: -936 (-23.54%) (effective code size, minus reset vector and build signature) romstage: -2176 (-13.38%) ramstage: -5343 (-10.84%) postcar: -908 (-9.07%)
Now I can't directly make use of space saved in bootblock, but it gives us more room before we have to go to 16KiB bootblock.
I'll now boot test it.
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: [TEST] Add support for link time optimization ......................................................................
Patch Set 6:
Patch Set 6:
Keith, as you also seem to care about image size, could you please test, if this still works on your Intel 440BX boards?
I'll now boot test it.
I booted this patch on p2b-ls with no noticeable differences in functionalities.
However I'm seeing corruption in the cbmem console output from earlier boots. Unsure what the cause is, as the test setup is bare with no physical support of any kind.
Hello Keith Hui, build bot (Jenkins), Michał Żygowski, Philipp Hug, Patrick Georgi, Martin Roth, Paul Menzel, Julius Werner, ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38989
to look at the new patch set (#7).
Change subject: [TEST] Add support for link time optimization ......................................................................
[TEST] Add support for link time optimization
Using LTO leads to a ~10% decrease in stage size for QEMU and ~18% for the Thinkpad T500, and both targets boot successfully.
GCC currently does not track symbol origins when using LTO, so it must be disabled for AGESA so it can be excluded from the illegal_globals check.
Change-Id: I48c31ea8b1b57276125cffdac44c7c16642547ac Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M src/Kconfig M src/vendorcode/amd/agesa/Makefile.inc 3 files changed, 19 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38989/7
Hello Keith Hui, build bot (Jenkins), Michał Żygowski, Philipp Hug, Patrick Georgi, Martin Roth, Paul Menzel, Julius Werner, ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38989
to look at the new patch set (#8).
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Kconfig,Makefile.inc: 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 coreboot. Due to the potential for breakage, this option is disabled by default, and would only be enabled per-board once it has been tested on hardware. Not all targets compile now, but early results are encouraging: using LTO leads to a ~13% decrease in stage size for QEMU i440fx and ~10% for the Thinkpad T500, and both targets boot successfully. Compile times for both are also about 10% faster.
Change-Id: I48c31ea8b1b57276125cffdac44c7c16642547ac Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M src/Kconfig 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38989/8
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 8:
(5 comments)
https://review.coreboot.org/c/coreboot/+/38989/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38989/2//COMMIT_MSG@23 PS2, Line 23: the Thinkpad T500, and both targets boot successfully.
For the T500 it's ~8ms less, but otherwise about the same. […]
Done
https://review.coreboot.org/c/coreboot/+/38989/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38989/3//COMMIT_MSG@14 PS3, Line 14: - The dead_code() macro causes linking errors, since dead functions
I recall having some problems with it early on, but I can't reproduce them anymore. […]
Done
https://review.coreboot.org/c/coreboot/+/38989/2/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/38989/2/src/Kconfig@123 PS2, Line 123: depends on COMPILER_GCC
I am able to successfully build and boot QEMU i440fx with LLVM/Clang.
Done
https://review.coreboot.org/c/coreboot/+/38989/4/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/38989/4/src/Kconfig@120 PS4, Line 120: bool "Use link time optimization"
Also add *(LTO)* at the end.
Done
https://review.coreboot.org/c/coreboot/+/38989/4/src/Kconfig@122 PS4, Line 122: default y
Could you split `default y` out to a separate commit please on top. […]
Done
Hello Keith Hui, build bot (Jenkins), Michał Żygowski, Philipp Hug, Patrick Georgi, Martin Roth, Paul Menzel, Julius Werner, ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38989
to look at the new patch set (#9).
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Kconfig,Makefile.inc: 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 coreboot. Due to the potential for breakage, this option is disabled by default, and would only be enabled per-board once it has been tested on hardware. Not all targets compile now, but early results are encouraging: using LTO leads to a ~13% decrease in stage size for QEMU i440fx and ~10% for the Thinkpad T500, and both targets boot successfully. Compile times for both are also about 10% faster.
Change-Id: I48c31ea8b1b57276125cffdac44c7c16642547ac Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M src/Kconfig 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/38989/9
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 10:
Jacob, maybe we need to rebase 42251 on current patch ? Please feel free if you want to do so....
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 11:
it doesn't work here, what am I doing wrong? `build/bootblock/lib/libgcc.o: plugin needed to handle lto object`
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 11:
Patch Set 11:
it doesn't work here, what am I doing wrong? `build/bootblock/lib/libgcc.o: plugin needed to handle lto object`
What board do you build for?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 11:
Patch Set 11:
Patch Set 11:
it doesn't work here, what am I doing wrong? `build/bootblock/lib/libgcc.o: plugin needed to handle lto object`
What board do you build for?
Qemu i440fx
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 11:
Patch Set 11:
it doesn't work here, what am I doing wrong? `build/bootblock/lib/libgcc.o: plugin needed to handle lto object`
Are you doing a regular build? Some of the tools need to be changed around to be LTO-aware (such as CB:40811), though it should work if all the patches in this train are applied.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 11: Code-Review+1
tested successfully for system76/lemp9. builds fine, boots fine
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 11:
Patch Set 11: Code-Review+1
tested successfully for system76/lemp9. builds fine, boots fine
That's good to hear, I'm glad there isn't any breakage. Did the binary size decrease at all?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 11:
Patch Set 11:
Patch Set 11: Code-Review+1
tested successfully for system76/lemp9. builds fine, boots fine
That's good to hear, I'm glad there isn't any breakage. Did the binary size decrease at all?
LTO disabled: fallback/romstage 0x80 stage 47188 none fallback/ramstage 0x3b1c0 stage 106256 none fallback/postcar 0x116680 stage 20252 none fallback/payload 0x11b600 simple elf 461468 none bootblock 0xba3dc0 bootblock 49152 none
LTO enabled: fallback/romstage 0x80 stage 42876 none - 9.13 % fallback/ramstage 0x3a100 stage 94121 none -11.42 % fallback/postcar 0x112680 stage 18348 none - 9.40 % fallback/payload 0x116e80 simple elf 461651 none + 0.04 % bootblock 0xba3dc0 bootblock 49152 none 0.00 %
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 12:
One thing that worked before for me, doesn't work anymore. I'm not sure if there's any solution for this.
When testing tiny changes, I never did a make clean but just ran `make -j5` to rebuild, where Make just rebuilds the changed source and resolved dependencies automagically. With the LTO patches, this does not seem to work anymore. The final linking step fails: https://paste.xinu.at/gOLYHtjrF4SO68Vam0HfecWreiN4ZJyP8k8GwRk4HTRSfV25v0Cd4u...
Any ideas?
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 12:
Patch Set 12:
One thing that worked before for me, doesn't work anymore. I'm not sure if there's any solution for this.
When testing tiny changes, I never did a make clean but just ran `make -j5` to rebuild, where Make just rebuilds the changed source and resolved dependencies automagically. With the LTO patches, this does not seem to work anymore. The final linking step fails: https://paste.xinu.at/gOLYHtjrF4SO68Vam0HfecWreiN4ZJyP8k8GwRk4HTRSfV25v0Cd4u...
Any ideas?
Good catch. The problem is the previous commit that replaces partial linking with thin archives. Trying to find a fix now.
Attention is currently required from: Jacob Garber. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 13:
(1 comment)
Patchset:
PS13: ramstage is now 2 MB in size.
``` CBFS fallback/romstage E: Input file size (2135728) greater than page size (261632). E: 'fallback/romstage' can't fit in CBFS for page-size 0, align 0x40. E: Failed while operating on 'COREBOOT' region! E: The image will be left unmodified. make[1]: *** [Makefile.inc:1101: /cb-build/coreboot-gerrit.0/default/EMULATION_QEMU_X86_I440FX_X86_64/coreboot.pre] Error 1 make[1]: Leaving directory '/home/coreboot/node-root/workspace/coreboot-gerrit' ```
Attention is currently required from: Jacob Garber. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 15: Code-Review+1
(1 comment)
Patchset:
PS15:
Patch Set 12:
Patch Set 12:
One thing that worked before for me, doesn't work anymore. I'm not sure if there's any solution for this.
When testing tiny changes, I never did a make clean but just ran `make -j5` to rebuild, where Make just rebuilds the changed source and resolved dependencies automagically. With the LTO patches, this does not seem to work anymore. The final linking step fails: https://paste.xinu.at/gOLYHtjrF4SO68Vam0HfecWreiN4ZJyP8k8GwRk4HTRSfV25v0Cd4u...
Any ideas?
Good catch. The problem is the previous commit that replaces partial linking with thin archives. Trying to find a fix now.
Have you ever found a solution for this?
Attention is currently required from: Michael Niewöhner. Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 15:
(1 comment)
Patchset:
PS15:
Patch Set 12: […]
Oh yes I did. I forget the exact problem but it was something with the archives not being regenerated in one particular case, it should be fixed now.
Attention is currently required from: Jacob Garber, Michael Niewöhner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38989 )
Change subject: Kconfig,Makefile.inc: Add support for link time optimization ......................................................................
Patch Set 15: Code-Review+1
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/38989/comment/6ed47765_7096aadf PS15, Line 19: Not all targets compile now Could we add a config file to build-test at least one of the targets that works?
https://review.coreboot.org/c/coreboot/+/38989/comment/351c9ff7_89f7b5f4 PS15, Line 21: successfully. nit: move to next line
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Jacob Garber (Code Review)
-
Julius Werner (Code Review)
-
Keith Hui (Code Review)
-
Michael Niewöhner (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)