make: add -ffunction-sections -fdata-sections to LINKFLAGS if LTO=1

[kaspar030]

Contribution description

#14754 broke LTO as it created significantly larger binaries (instead of smaller) with LTO enabled. Our micro benchmarks showed increased performance, so LTO still worked partially.
I tracked this down to -ffunction-sections -fdata-sections missing for the linker invocation.

This PR fixes that by adding those flags to LINKFLAGS if LTO is enabled.

binary (text) sizes for examples/hello-world on nrf52840dk:

	LTO=0	LTO=1
master	8688	10188
PR	8688	7792

Testing procedure

non-LTO binaries should be identical. lto binaries should be smaller and still work.

I did not enable "run tests" as no LTO tests are run on CI.

Issues/PRs references

#14754

fixes #16202

[kaspar030]

@fjmolinas could you do a test run with LTO=1?

[fjmolinas]

@fjmolinas could you do a test run with LTO=1?

What do you want me to check? that it builds? Or the sizes?

[kaspar030]

What do you want me to check? that it builds? Or the sizes?

no, whether the LTO=1 binaries are working. Murdock can't (easily) do that.

[benpicco]

Murdock can't (easily) do that.

we could enable LTO by default and run tests

[kaspar030]

we could enable LTO by default and run tests

yeah, not that difficult, done.

[benpicco]

-- executing tests for tests/bench_xtimer_load on samr21-xpro (compiled with gnu toolchain):
…
socat - open:/dev/ttyACM0,b115200,echo=0,raw 
*** RIOT kernel panic:
DUMMY HANDLER

*** halted.

Inside isr 9
Stack pointer corrupted, reset to top of stack
Misc
EXC_RET: 0xfffffff1

😢

[kaspar030]

*** RIOT kernel panic:
DUMMY HANDLER

weird. I tried some of the tests, they work locally and when sending a locally compiled image to the test pi.

[benpicco]

Maybe the GCC version on CI is just too old?

[kaspar030]

Maybe the GCC version on CI is just too old?

seems like, with building in docker I can reproduce.

[fjmolinas]

Is the ci required update addressed now @kaspar030?

[kaspar030]

Is the ci required update addressed now @kaspar030?

no, this needs a toolchain upgrade.

[fjmolinas]

Unrelated failure, addressed in #17641

[fjmolinas]

Didn't see there was a REMOVEME commit, I removed the label to not re-build but everything passed except for the unrelated test.

[fjmolinas]

I ran some tests on a subset of hardware with LTO=1.

### nrf52dk/failuresummary

Failures during compilation:
- [tests/rust_minimal](nrf52dk/tests/rust_minimal/compilation.failed)

Failures during test:
- [examples/micropython](nrf52dk/examples/micropython/test.failed)
- [tests/periph_timer_short_relative_set](nrf52dk/tests/periph_timer_short_relative_set/test.failed)
- [tests/pkg_libfixmath](nrf52dk/tests/pkg_libfixmath/test.failed)

### nucleo-l152re/failuresummary

Failures during compilation:
- [tests/rust_minimal](nucleo-l152re/tests/rust_minimal/compilation.failed)

Failures during test:
- [examples/micropython](nucleo-l152re/examples/micropython/test.failed)
- [tests/malloc](nucleo-l152re/tests/malloc/test.failed)
- [tests/periph_timer_short_relative_set](nucleo-l152re/tests/periph_timer_short_relative_set/test.failed)

### samr21-xpro/failuresummary

Failures during compilation:
- [tests/pkg_c25519](samr21-xpro/tests/pkg_c25519/compilation.failed)
- [tests/rust_minimal](samr21-xpro/tests/rust_minimal/compilation.failed)

Failures during test:
- [examples/micropython](samr21-xpro/examples/micropython/test.failed)
- [tests/malloc](samr21-xpro/tests/malloc/test.failed)
- [tests/periph_timer_short_relative_set](samr21-xpro/tests/periph_timer_short_relative_set/test.failed)

### p-nucleo-wb55/failuresummary

Failures during compilation:
- [tests/pkg_flatbuffers](p-nucleo-wb55/tests/pkg_flatbuffers/compilation.failed)
- [tests/rust_minimal](p-nucleo-wb55/tests/rust_minimal/compilation.failed)

Failures during test:
- [examples/micropython](p-nucleo-wb55/examples/micropython/test.failed)

### z1/failuresummary

Failures during test:
- [tests/congure_test](z1/tests/congure_test/test.failed)
- [tests/pbkdf2](z1/tests/pbkdf2/test.failed)
- [tests/periph_flashpage_unittest](z1/tests/periph_flashpage_unittest/test.failed)
- [tests/periph_timer_short_relative_set](z1/tests/periph_timer_short_relative_set/test.failed)
- [tests/sched_change_priority](z1/tests/sched_change_priority/test.failed)
- [tests/sema](z1/tests/sema/test.failed)
- [tests/thread_float](z1/tests/thread_float/test.failed)
- [tests/turo](z1/tests/turo/test.failed)

Micropython and rust are a setup issue, z1 failures are expected and documented. Malloc failures are because of corrupted stdout, but the test ran fine. Since in any case LTO is never tested we really don't have a compare point against it, what I see is most tests passing and the hardfault is no longer present.

For detailes results see https://ci.inria.fr/ci-riot-tribe/job/build-pipeline.jk/313/artifact/

[fjmolinas]

ACK

[chrysn]

There are also other failures than the reported one (which is probably related to permissions in /data/riotbuild/.cargo and how that is set up and cached outside docker), at least when trying to build locally here, but that appears to be a failure of the Rust side, and to be fixed there independently of this issue.

Great to see this go ahead! (Tempted to already push the merge button as all is green, but not being familiar with the history I'll leave that to you).

[chrysn]

... and even the failures I've seen here were purely local errors, and after the adequate adjustments (as in: use the right nightly and remove erroneous bin content), the Rust test and applications with this and LTO=1 worked (on native and microbit-v2), and also built for nrf52dk (which I can't test).

[kaspar030]

Thanks everyone!