bitbox02-firmware icon indicating copy to clipboard operation
bitbox02-firmware copied to clipboard

[wip] Enable lto (should only be merged if needed, modifies bootloader)

Open NickeZ opened this issue 5 years ago • 4 comments

This PR enables LTO. We might want to think a minute or two before merging...

  • _binExec was a bit sloppy written. It didn't actually read the input variable and the asm block wasn't volatile. So the compiler happily optimized most of it away.
  • I have no freakin clue why __stack_chk_guard cannot be declared in the same way in firmware.c as in bootloader/startup.c. I get linker errors saying that the variable doesn't exist. I have carefully tried to get the link commands be as equal as possible, but no, the only way I could fix it was to change from local declaration to extern.

The benefits are that we get back about 10% of space.

NickeZ avatar Mar 09 '20 20:03 NickeZ

reduction by 10% is very nice! I'll take a look in about a week unless @benma beats me to it.

x1ddos avatar Jun 06 '20 11:06 x1ddos

reduction by 10% is very nice! I'll take a look in about a week unless @benma beats me to it.

if i remember correctly, this PR caused some hard to explain effects (hardfaults?). Can't quite recall, but we put it on ice since we do not need the 10% yet and the time to review and make sure everything is good was not worth it. Imho this is still the same.

Feel free to take a look anyway :P

benma avatar Jun 06 '20 15:06 benma

Enabling LTO will for sure make the compiler have more fun with any UB in the code :/ might not be worth it.

I think i didn't have any problems with it when I tried it the last time. But it was some time ago..

NickeZ avatar Jun 06 '20 18:06 NickeZ

I think i also solved the stack check guard in some other pr related to the device tests

NickeZ avatar Jun 06 '20 18:06 NickeZ