linker: sort sections by alignment

[dcpleung]

This turns on the linker flag to sort sections by alignment in decreasing size of symbols. This helps to minimize padding between symbols.

This also adds the linker options to sort common symbols by alignment in descending to minimize the need for padding.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@017446b). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #14183   +/-   ##
=========================================
  Coverage          ?   51.99%           
=========================================
  Files             ?      308           
  Lines             ?    45569           
  Branches          ?    10553           
=========================================
  Hits              ?    23693           
  Misses            ?    17069           
  Partials          ?     4807

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 017446b...93e5b5e. Read the comment docs.

[SebastianBoe]

Very nice, would you mind looking into if -Wl,--sort-section=alignment could be used instead?

Then the optimization will be applied even more broadly, and we don't need to remember to invoke SORT_BY_ALIGNMENT for new sections.

I tried to get this in a while back, but encountered a runtime issue in xtensa that I was not able to debug.

#4296

If there are any issues with defaulting to sorting wildcard sections (there shouldn't in a well-designed system), then the proposed patchset is still better than what we are doing today.

[galak]

Adding @ioannisg, need to make sure we are keep various partitions together to not break userspace. @ioannisg knows the details.

[ioannisg]

This looks ok to me; AFAIU, the patch only sorts-by-alignment the kernel SRAM sections (noint, bss, etc.) and the kernel objects.

For ARM, changing the order of kernel SRAM sections does not break User Mode/MPU etc., as long as the sections still form a consecutive memory area, together, also, with the APP MEM partitions.

So looks safe enough - in the mean time, I am running the kernel test suite to see if anything is broken. Will report here.

[ioannisg]

Approving this one. I ran the tests on all Cortex-M & MPU arches and things look good.

[SebastianBoe]

-Wl,--sort-section=alignment should be used if possible

[dcpleung]

-Wl,--sort-section=alignment should be used if possible

Test tests/application_development/gen_inc_file failed with qemu_nios2 with *(SORT_BY_ALIGNMENT(".rodata.*")). According to the map file, the linker moved the whole section two bytes forward, and the test never booted (AFAICT).

Also, the device init section (.init_*) requires strict ordering by name, and also IRQ vector sections. And I am not totally sure the linker would leave those sections sorted by name only without reshuffling them.

I am a bit hesitant to enable it for everything, for now.

[andrewboie]

-Wl,--sort-section=alignment

Does this override the SORT_BY_NAME() invocations we have in the linker scripts?
Also it's not clear to me that we need to change this many lines in the linker scripts. I would imagine, that for most text/data, everything is already 4 byte aligned. The kernel object sections are simply arrays of kernel objects.

Can we identify what areas we are enforcing something other than 4 byte alignment, and just sort by alignment there?

[andrewboie]

Adding @ioannisg, need to make sure we are keep various partitions together to not break userspace.

This patch is orthogonal to userspace's application shared memory feature.

[dcpleung]

Updated the PR and removed the patches for kernel objects. As @andrewboie said, those objects are of fixed size already so there is no gain. Also went through numerous sanitycheck where sorting text sections has no benefits except ARM. On ARM, sorting text sections shows saving of around 30 bytes for rom_size, so keeping those. Sorting by alignment for data, noinit, rodata and bss showed improvements for both ROM and RAM size.

[SebastianBoe]

Does this override the SORT_BY_NAME() invocations we have in the linker scripts?

Fortunately, it behaves as expected, and does not override SORT_BY_NAME.

https://sourceware.org/binutils/docs/ld/Input-Section-Wildcards.html#Input-Section-Wildcards

"When both command line section sorting option and linker script section sorting command are used, section sorting command always takes precedence over the command line option."

[SebastianBoe]

Also, the device init section (.init_*) requires strict ordering by name, and also IRQ vector sections. And I am not totally sure the linker would leave those sections sorted by name only without reshuffling them.

These sections can and should explicitly use SORT_BY_NAME, it will leave them sorted by name.

[SebastianBoe]

Test tests/application_development/gen_inc_file failed with qemu_nios2 with (SORT_BY_ALIGNMENT(".rodata.")). According to the map file, the linker moved the whole section two bytes forward, and the test never booted (AFAICT).

This sounds like a bug in our linker scripts that should be fixed independently of whether wildcards (without explicit sorting commands) are sorted by alignment or by the order that the linker sees the section.

[dcpleung]

-Wl,--sort-section=alignment should be used if possible

Updated the PR to do this.

[galak]

Is there a reason we shouldn't just do this alway? Why make it a Kconfig option?

[dcpleung]

Is there a reason we shouldn't just do this alway? Why make it a Kconfig option?

1. Given that it broke Nios2 without changing the linker script, having a kconfig option provides a quick way to exclude a board or platform to un-break the build while a solution is being worked on.
2. The kconfig also provides flexibility in case someone wants to optimize their own placement, without having to revert this patch on their tree.

[galak]

• Given that it broke Nios2 without changing the linker script, having a kconfig option provides a quick way to exclude a board or platform to un-break the build while a solution is being worked on.
• The kconfig also provides flexibility in case someone wants to optimize their own placement, without having to revert this patch on their tree.

I'd rather we always do this an once someone want's that flexibility they can ask or submit the change. Seems like a Kconfig option that we'll have around and never change.

[dcpleung]

Removed the kconfig option.