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

Enable link time optimisation in builds #139

Closed
4 tasks
mithro opened this issue Dec 17, 2018 · 11 comments
Closed
4 tasks

Enable link time optimisation in builds #139

mithro opened this issue Dec 17, 2018 · 11 comments

Comments

@mithro
Copy link
Collaborator

mithro commented Dec 17, 2018

Link time optimisation seems to be resulting in much smaller binaries as any unused elements can be totally removed.

There are however some issues to make this work;

  • Have to use a modern gcc
  • Use GCC for linking
  • Have to mark a couple of functions like the ISR as actually used.
  • Have to link against libgcc
@mithro
Copy link
Collaborator Author

mithro commented Dec 17, 2018

@mithro
Copy link
Collaborator Author

mithro commented Dec 17, 2018

It is unclear why we have to link against libgcc when the symbols should be provided by libcompiler_rt. It seems to be because GCC is generating function calls itself (rather than being in the code) which cause lto to not understand that the symbols are used.

@enjoy-digital
Copy link
Owner

This has been integrated with #401. For now it's enabled with for all processors except LM32. If this is causing issue with others configurations, we'll add a parameter be able to disable LTO when needed.

@enjoy-digital
Copy link
Owner

LTO has been reverted in 979f98e since seems to cause subtle issues on some configurations. We'll have to test it more before enabling it.

@enjoy-digital
Copy link
Owner

cc @mithro @mateusz-holenko.

@ewenmcneill
Copy link
Contributor

In case it helps, the link issue I had (in #417) with litex-buildenv was with the "stub firmware" in litex-buildenv, which uses the auto-generated litex compiler/linker/flags/etc, but has its own Makefile. So (copying from my comment on #417 (comment)), " updating the definition of $(LD) in common.mak instead of changing it locally in just the BIOS Makefile" might make these changes more robust (/consistent) for anything else that uses the litex generated compiler/linker/flags, etc. (It looks like that Makefile suffered from trying to use ld with flags intended for gcc, via $(CPUFLAGS), on lm32....)

Also (as I said in the same comment) maybe having a build time option to "force LTO off" would help anyone thinking their target is suffering from bad LTO handling.

In general I think it'd be a great idea to have LTO available as a build option, and probably even on by default.

Ewen

/cc @mithro @mateusz-holenko

@mithro
Copy link
Collaborator Author

mithro commented Mar 12, 2020

Having LTO on by default with a way to easily turn it off seems like a good plan. If LTO is off by default, it will just end up bit rotting until it no longer works.

@enjoy-digital
Copy link
Owner

I'm also ok with that, we just need to work a bit more on it before integrating it.

@DurandA
Copy link
Contributor

DurandA commented Nov 3, 2020

I just found this issue, should I close #682 which is a duplicate of this?

* [ ]  Have to link against libgcc

This is not necessary. It was required because libcompiler_rt was optimized out.

@ewenmcneill
Copy link
Contributor

should I close #682 which is a duplicate of this?

@DurandA It looks to me like #682 is much more active now. I'd probably lean towards closing this issue (which was already closed once, then reopened when the patch was reverted), with a comment like "LTO (re-)implementation work moved to #682"...

(Looks like @mithro or @enjoy-digital digital would have to be the ones to close this specific older issue.)

Ewen

@mithro
Copy link
Collaborator Author

mithro commented Nov 5, 2020

Let's keep #682 and close this one.

@mithro mithro closed this as completed Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants