abcminiuser / lufa

LUFA - the Lightweight USB Framework for AVRs.
http://www.lufa-lib.org
1.04k stars 325 forks source link

Fix high memory usage #129

Closed fornellas closed 3 years ago

fornellas commented 6 years ago

Same fix as described here https://github.com/abcminiuser/dmbs/pull/33.

abcminiuser commented 6 years ago

This is potentially a little dangerous - it removes any data section symbols which aren't explicitly referenced. That's fine most of the time, and can lead to some good savings if you have unused tables, but can also remove important bootloader jump tables and other data which isn't explicitly referenced by the application but is nevertheless required.

Have you compared the LSS listing files with/without this to determine which symbols are actually removed?

fornellas commented 6 years ago

I'm not sure about the dangerous cases. In my scenario, there were tons of functions not being used, that referred tons of unused static variables.

So, arguably, safer, broke by use case. Do you have examples where things break? Maybe it is the case of selectively using the flag, rather than doing it globally.

On Sun 17 Jun 2018, 07:05 Dean Camera, notifications@github.com wrote:

This is potentially a little dangerous - it removes any data section symbols which aren't explicitly referenced. That's fine most of the time, and can lead to some good savings if you have unused tables, but can also remove important bootloader jump tables and other data which isn't explicitly referenced by the application but is nevertheless required.

Have you compared the LSS listing files with/without this to determine which symbols are actually removed?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/abcminiuser/lufa/pull/129#issuecomment-397855055, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUm-HwI36i2Bht4pLKp7I7wx_uZO0AQks5t9eOXgaJpZM4UM7da .

NicoHood commented 6 years ago

Can you please compare with the lufa provided examples? I would then also test it myself, I am not sure if it really helps that much.

Have you also tried to use LTO?

abcminiuser commented 6 years ago

I'm not sure about the dangerous cases. In my scenario, there were tons of functions not being used, that referred tons of unused static variables.

Having a special table in RAM that isn't directly referenced by the application (for example, to provide metadata for the bootloader) would result in the table being removed with -fdata-sections if the symbol wasn't explicitly kept by a linker override:

static const char* BootloaderMetadata = "[[BOOTLOADER-META]]Version 1.0.2";

This is the same issue encountered when making bootloader API functions, when -ffunction-sections is enabled (I had to mark them with -Wl,--undefined=.... in the makefile) although that's probably less common. I'm not sure if having unused static tables is common enough to turn on -fdata-sections by default or not.

NicoHood commented 6 years ago

I might missunderstand, but what about __attribute__((used))? Wouldnt this keep the data, even if its not used?

abcminiuser commented 6 years ago

__attribute__((used)) only affects the compiler's optimizer - that directive isn't passed down to the linker when it strips out unused sections. The -ffunction-sections and -fdata-sections compiler flags tells the compiler to place every function/static variable in a uniquely-named section inside the compiled binary, while -gc-sections tells the linker to discard sections whose contents are entirely unused.

The only way that I know of to force the symbol to survive the linker garbage collection pass is to either explicitly make the symbol used by referencing it from somewhere else that is also not eligible for garbage collection, or to explicitly mark it as required in the final binary via a KEEP() directive in a custom linker script, or the --undefined linker option on the command line.

fornellas commented 3 years ago

I'm closing this PR as it seems there's no agreement it should proceed

NicoHood commented 3 years ago

@fornellas did you try LTO?

fornellas commented 3 years ago

@NicoHood I'm not familiar with that, and I've been manually adding -fdata-sections to all my projects without issues (I don't think I have anything that falls into the corner cases explained here). TBH, I'm not knowledgeable enough in this layer to discuss more here :-D

NicoHood commented 3 years ago

That does not sound smart, for given reasons. LTO is documented like all other options, you can enable it with LTO = Y in the makefile.