Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

NickeZ
Copy link
Collaborator

@NickeZ NickeZ commented Mar 9, 2020

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 NickeZ force-pushed the enable-lto branch 4 times, most recently from 1a62795 to 713b6b3 Compare March 12, 2020 16:59
@NickeZ NickeZ changed the title Enable lto [wip] Enable lto (should only be merged if needed, modifies bootloader) Mar 12, 2020
@x1ddos
Copy link
Contributor

x1ddos commented Jun 6, 2020

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

@x1ddos x1ddos self-requested a review June 6, 2020 11:29
@benma
Copy link
Collaborator

benma commented Jun 6, 2020

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

@NickeZ
Copy link
Collaborator Author

NickeZ commented Jun 6, 2020

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
Copy link
Collaborator Author

NickeZ commented Jun 6, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants