build-system: enabling LTO builds on arm platforms #49553

dkalowsk · 2022-08-25T21:54:47Z

We've been spending a bit of time trying to get LTO to work for Zephyr on Arm and recently had some luck on it. I don't expect this PR to actually be merged, hence the Draft prefix, but is being provided to get feedback for what would hopefully be the correct set of changes needed to be merged in.

Locally on our system, we are seeing these changes provide about a 16% decrease in binary size for us, with the resulting binary bootable. Debugging is not so pretty, but if you're space constrained...

Additional changes need for a project to take advantage of these include:

$PROJECT/CMakeLists.txt:
  add_compile_options( -flto )
  add_link_options( -flto )
  # fix1: undefined reference to `memcpy'
  add_link_options( -fno-ipa-sra )

For some reason we also see undefined reference to __device_dts_ord_X'` which is resolved by adding:

$PROJECT/CMakeLists.txt:
  add_link_options( -ffunction-sections -fdata-sections )

We are still investigating why this is needed or what is happening here.

Likely this will all want to be added/wrapped with a Kconfig to enable/disable what is going on.

When building with LTO enabled, the GCC wrapper versions of ar and ranlib need to be used to get the correct LTO commands. Signed-off-by: Dan Kalowsky <dank@deadmime.org>

When building with LTO, one of the first things the process does is toss the irq_vector_table out the window making it hard to actually boot. Use the compiler attributes to mark the struct as used thus making the LTO process ignore it. Signed-off-by: Dan Kalowsky <dank@deadmime.org>

mbolivar-nordic · 2022-08-26T06:26:24Z

scripts/build/gen_isr_tables.py

@@ -153,7 +153,7 @@ def write_code_irq_vector_table(fp, vt, nv, syms):
    fp.write("}\n")

 def write_address_irq_vector_table(fp, vt, nv):
-    fp.write("uintptr_t __irq_vector_table _irq_vector_table[%d] = {\n" % nv)
+    fp.write("uintptr_t __irq_vector_table __attribute__((__used__)) _irq_vector_table[%d] = {\n" % nv)


You meant __used here, right?

Are you suggesting changing this to:

uintptr_t __irq_vector_table __used _irq_vector_table[%d] =

to make it more cross-compiler friendly?

yes, to make it toolchain agnostic.

arch/common/CMakeLists.txt

andyross · 2022-08-26T15:23:27Z

cmake/bintools/gnu/target.cmake

+find_program(CMAKE_RANLIB  ${CROSS_COMPILE}gcc-ranlib PATHS ${TOOLCHAIN_HOME} NO_DEFAULT_PATH)
+find_program(CMAKE_READELF ${CROSS_COMPILE}readelf    PATHS ${TOOLCHAIN_HOME} NO_DEFAULT_PATH)
+find_program(CMAKE_NM      ${CROSS_COMPILE}nm         PATHS ${TOOLCHAIN_HOME} NO_DEFAULT_PATH)
+find_program(CMAKE_STRIP   ${CROSS_COMPILE}strip      PATHS ${TOOLCHAIN_HOME} NO_DEFAULT_PATH)


FWIW: the use of static libraries internal to our build is almost all cargo-cult. There are a tiny handful of places where we're using it as a trick to prevent the linker from seeing static data (like init calls and iterable elements) it would otherwise try to include. All those should be correctly isolating the data via kconfig in one of the dozens of ways we allow and not doing it implicitly.

I see that it is cargo-cult. My concern here is that I don't know how this will impact those with non-Zephyr SDK build tools

Using LTO compile flags on some specific files causes the system to not be bootable again. Mark these files specifically as not allowing LTO flags to make sure the application can boot properly. Signed-off-by: Dan Kalowsky <dank@deadmime.org>

dkalowsk · 2022-08-31T17:07:16Z

@tejlmand and @stephanosio any input/feedback on the proposed changes here?

tejlmand · 2022-09-05T13:06:56Z

We've been spending a bit of time trying to get LTO to work for Zephyr on Arm and recently had some luck on it. I don't expect this PR to actually be merged, hence the Draft prefix, but is being provided to get feedback for what would hopefully be the correct set of changes needed to be merged in.

Thanks for taking a look into this, and to share your experiences.

But I believe there is a long way to go on this.
Zephyr build system relies on --whole-archive to ensure symbols are not dropped by the linker, and trying to add lto will be a challenge.
For some more details regarding the whole-archive challenge, feel free to refer to:
#6961
#8441
#8451

Zephyr generally handles the size problem by fine-grained feature support with Kconfig, so that things you don't need can be disabled and thus minimizing the need for lto.
This again mean that if you see a large size reduction for Zephyr code itself, then chances are high that something is not working.

As a quick test, I tried to build bluetooth/peripheral_hr following the approach with this PR, and the radio / bluetooth stack is never initialized.

A quick look at the elf revealed that bt_init() function is removed with lto, and together with that bt_conn_init.
So bluetooth is definitely not working.

It would be great to see lto support, cause I do believe that when using 3rd party libraries with Zephyr or in application code, then there can be great value in lto, as such code may not have same fine-grained control as Zephyr itself.

But I would like to see some more thoughts on how to address the --whole-archive challenge, and not just adding -lto and see that a basic sample boots.

Locally on our system, we are seeing these changes provide about a 16% decrease in binary size for us, with the resulting binary bootable. Debugging is not so pretty, but if you're space constrained...

A great start, although there is a long walk from bootable to fully functioning, but keep up the good work 👍
Could you share more info on which sample and on which board you got something booting ?
Would be nice to reproduce your results.

tejlmand

DNM, one thing is to get something simple to boot, another thing is actually to ensure Zephyr in various configuration setup is working.

atm I mostly consider this as an early proff-of-concept and not ready for merge.

github-actions · 2022-11-05T00:33:24Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

eHammarstrom · 2022-11-15T09:52:32Z

I also had to add the used attribute to libc's __chk_fail

github-actions · 2023-01-16T00:30:07Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

dkalowsk added 2 commits August 25, 2022 14:37

cmake/bintools/gnu: use the gcc-ar and gcc-ranlib

43fe5c4

When building with LTO enabled, the GCC wrapper versions of ar and ranlib need to be used to get the correct LTO commands. Signed-off-by: Dan Kalowsky <dank@deadmime.org>

dkalowsk requested review from tejlmand, nashif, ioannisg and andyross as code owners August 25, 2022 21:54

zephyrbot added area: Architectures area: Build System area: Toolchains Toolchains labels Aug 25, 2022

zephyrbot requested review from dcpleung, jeremybettis and stephanosio August 25, 2022 21:55

zephyrbot assigned tejlmand Aug 25, 2022

mbolivar-nordic reviewed Aug 26, 2022

View reviewed changes

andyross reviewed Aug 26, 2022

View reviewed changes

cmakelists: disable LTO on specific files

a2d9f50

Using LTO compile flags on some specific files causes the system to not be bootable again. Mark these files specifically as not allowing LTO flags to make sure the application can boot properly. Signed-off-by: Dan Kalowsky <dank@deadmime.org>

dkalowsk force-pushed the dkalowsky/enable_arm_lto branch from 7e091f2 to a2d9f50 Compare August 26, 2022 16:01

dkalowsk changed the title ~~Draft: build-system: enabling LTO builds on arm platforms~~ build-system: enabling LTO builds on arm platforms Aug 31, 2022

tejlmand requested changes Sep 5, 2022

View reviewed changes

tejlmand added the DNM This PR should not be merged (Do Not Merge) label Sep 5, 2022

github-actions bot added the Stale label Nov 5, 2022

github-actions bot removed the Stale label Nov 16, 2022

github-actions bot added the Stale label Jan 16, 2023

github-actions bot closed this Jan 31, 2023

rakons mentioned this pull request Dec 12, 2023

LTO-compatible ISRs #66392

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build-system: enabling LTO builds on arm platforms #49553

build-system: enabling LTO builds on arm platforms #49553

dkalowsk commented Aug 25, 2022

mbolivar-nordic Aug 26, 2022

dkalowsk Aug 26, 2022

stephanosio Aug 31, 2022

andyross Aug 26, 2022

dkalowsk Aug 31, 2022

dkalowsk commented Aug 31, 2022

tejlmand commented Sep 5, 2022

tejlmand left a comment

github-actions bot commented Nov 5, 2022

eHammarstrom commented Nov 15, 2022

github-actions bot commented Jan 16, 2023

build-system: enabling LTO builds on arm platforms #49553

build-system: enabling LTO builds on arm platforms #49553

Conversation

dkalowsk commented Aug 25, 2022

mbolivar-nordic Aug 26, 2022

Choose a reason for hiding this comment

dkalowsk Aug 26, 2022

Choose a reason for hiding this comment

stephanosio Aug 31, 2022

Choose a reason for hiding this comment

andyross Aug 26, 2022

Choose a reason for hiding this comment

dkalowsk Aug 31, 2022

Choose a reason for hiding this comment

dkalowsk commented Aug 31, 2022

tejlmand commented Sep 5, 2022

tejlmand left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 5, 2022

eHammarstrom commented Nov 15, 2022

github-actions bot commented Jan 16, 2023