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

build: add lto configure option #10800

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@theuni
Copy link
Member

theuni commented Jul 12, 2017

Addresses #10616. Please see the commit messages and note that AR/RANLIB may need to be set manually!

This is testing the waters for LTO. There are a few unknowns:

  • A boost LTO issue was addressed here, but others may remain for different libs/versions
  • ar/ranlib detection will likely turn out to be annoying
  • Not yet tested with Gitian

I propose adding this as-is (maybe after Gitian testing), and seeking feedback. I suspect that there will be quite a few platforms/toolchains where it's broken, but there's only so much we can do about it.

theuni added some commits Jul 11, 2017

lto: workaround for boost issue
I'm not sure what the root cause of the issue is yet, but defining a
boost::filesystem::path at global scope breaks linking with lto. The issue is
either:
- Some non-obvious ODR violation
- A bug or UB in gcc's handling of where to put the symbol for the inlined dtor
- Same as above, but for ld at link time

Presumably this is caused by the fact that boost::filesystem::path contains a
few methods which use static instances of a path object,

The work-around is to use a static string instead.
lto: add --enable-lto option
Note that for gcc, AR and RANLIB may need to be manually set to gcc-ar and
gcc-ranlib respectively.

Depending on how much trouble this causes, we may need to attempt to set these.
@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented Jul 12, 2017

Built successfully on my gitian machine (haven't tested the build yes though): https://bitcoin.jonasschnelli.ch/build/213

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Sep 6, 2017

Bleh, the boost work-around is really nasty. It breaks my use-case with cloudabi, where fs::path is not actually convertible from/to a string losslessly.

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Apr 9, 2018

@theuni This can be closed given #10616 (comment) ? Otherwise needs rebase

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jun 24, 2018

Closing, let me know when it needs to be reopened.

@laanwj laanwj closed this Jun 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.