Skip to content

cpu/cortexm_common: enable LTO by default#14795

Closed
benpicco wants to merge 2 commits into
RIOT-OS:masterfrom
benpicco:lto_default
Closed

cpu/cortexm_common: enable LTO by default#14795
benpicco wants to merge 2 commits into
RIOT-OS:masterfrom
benpicco:lto_default

Conversation

@benpicco

Copy link
Copy Markdown
Contributor

Contribution description

LTO support in GCC should be pretty mature by now, to a point where Fedora enables it by default for it's packages.

Let's follow suit by enabling LTO for our Cortex-M target.
This is the most mature target and is well-tested regularly.
So if any issues are caused by this, they should pop up fairly quick.

Testing procedure

Usually one would expect interrupt-related things to break if there are LTO related bugs.
Testing everything on every platform is not practicable.
But if no errors pop up during release-spec testing, this is probably fine.

Issues/PRs references

The feature has been in place for 4 years now, let's remove the warning.
@benpicco benpicco added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … labels Aug 19, 2020
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 19, 2020
@maribu

maribu commented Aug 20, 2020

Copy link
Copy Markdown
Member

There is still one issue to overcome: We currently use gcc for linking even when LLVM is used as toolchain. This does not work with LTO.

LTO support in GCC should be pretty mature by now, to a point where
Fedora enables it by default for all packages[0].

Let's follow suit by enabling LTO for our Cortex-M target.
This is the most mature target and is well-tested regularly.
So if any issues are caused by this, they should pop up fairly quick.

[0] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/CQEI3UGK5HSHZ7JHE7NRTQHBWQYWA5HM/
@benpicco

Copy link
Copy Markdown
Contributor Author

Looks like tests/devfs is indeed broken with LTO.

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 10, 2020
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 26, 2020
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 17, 2020
@maribu

maribu commented Dec 18, 2020

Copy link
Copy Markdown
Member

@kaspar030: You were recently asking for examples of LTO increasing the ROM size. The output of Murdock has some :-/

-- running on worker mobi6.inet.haw-hamburg.de thread 1, build number 28617.
make: Entering directory '/tmp/dwq.0.8873865931215175/c6e60fae10124650099702b3fa584256/bootloaders/riotboot'
Building application "riotboot" for "cc2538dk" with MCU "cc2538".

/opt/gcc-arm-none-eabi-9-2019-q4-major/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld: /tmp/dwq.0.8873865931215175/c6e60fae10124650099702b3fa584256/build/riotboot.elf section `.text' will not fit in region `rom'
/opt/gcc-arm-none-eabi-9-2019-q4-major/bin/../lib/gcc/arm-none-eabi/9.2.1/../../../../arm-none-eabi/bin/ld: region `rom' overflowed by 856 bytes
collect2: error: ld returned 1 exit status
/tmp/dwq.0.8873865931215175/c6e60fae10124650099702b3fa584256/bootloaders/riotboot/../..//Makefile.include:610: recipe for target '/tmp/dwq.0.8873865931215175/c6e60fae10124650099702b3fa584256/build/riotboot.elf' failed
make: *** [/tmp/dwq.0.8873865931215175/c6e60fae10124650099702b3fa584256/build/riotboot.elf] Error 1
make: Leaving directory '/tmp/dwq.0.8873865931215175/c6e60fae10124650099702b3fa584256/bootloaders/riotboot'
cat: /tmp/dwq.0.8873865931215175/c6e60fae10124650099702b3fa584256/build/test-input-hash.sha1: No such file or directory
-- build directory size: 1000K

I don't know if the increase of ROM size only affects riotboot, or if the increase in ROM size for other apps just does not result in link failures.

I can reproduce the increase also with my GCC toolchain (+552B .text, +4B .bss, +-0B .data). Previously my experience with LTO were that I consistently got smaller binaries, but sometimes a missing __attribute__((used)) resulted in IRQ handlers being garbage collected or similar issues.

@kaspar030

Copy link
Copy Markdown
Contributor

I don't know if the increase of ROM size only affects riotboot, or if the increase in ROM size for other apps just does not result in link failures.

tests/minimal also gets significantly larger:

   text    data     bss     dec     hex filename
   3668       8     988    4664    1238 /home/kaspar/src/riot.tmp/tests/minimal/bin/samr21-xpro/tests_minimal.elf
   text    data     bss     dec     hex filename
   2312       8     988    3308     cec /home/kaspar/src/riot.tmp/tests/minimal/bin/samr21-xpro/tests_minimal.elf

@kaspar030

Copy link
Copy Markdown
Contributor

I wonder if we graph this, if there's a constant overhead, pointing to something not properly LTO'ed.

@kaspar030

Copy link
Copy Markdown
Contributor

hello-world also gets larger on samr21-xpro, and that definitely wasn't the case before.

@kaspar030

Copy link
Copy Markdown
Contributor

I bisected this, 81cb769 adds almost 2k to hello-world on samr21-xpro compiled with LTO.

@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 14, 2021
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 17, 2022
@stale

stale Bot commented Nov 2, 2022

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale Bot added the State: stale State: The issue / PR has no activity for >185 days label Nov 2, 2022
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 2, 2022
@crasbe

crasbe commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Superseded by #22338.

@mguetschow

Copy link
Copy Markdown
Contributor

I don't know if the increase of ROM size only affects riotboot, or if the increase in ROM size for other apps just does not result in link failures.

tests/minimal also gets significantly larger:

   text    data     bss     dec     hex filename
   3668       8     988    4664    1238 /home/kaspar/src/riot.tmp/tests/minimal/bin/samr21-xpro/tests_minimal.elf
   text    data     bss     dec     hex filename
   2312       8     988    3308     cec /home/kaspar/src/riot.tmp/tests/minimal/bin/samr21-xpro/tests_minimal.elf

For reference, this is not true anymore in 2026:

$ BUILD_IN_DOCKER=1 make -C tests/minimal BOARD=samr21-xpro all -j
   text    data     bss     dec     hex filename
   2296       8     988    3292     cdc /data/riotbuild/riotbase/tests/minimal/bin/samr21-xpro/tests_minimal.elf
$ BUILD_IN_DOCKER=1 LTO=1 make -C tests/minimal BOARD=samr21-xpro all -j
   text    data     bss     dec     hex filename
   1736       0     988    2724     aa4 /data/riotbuild/riotbase/tests/minimal/bin/samr21-xpro/tests_minimal.elf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: stale State: The issue / PR has no activity for >185 days Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants