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

Remove the LTO warning for ENABLE_MASON builds #3481

Closed
wants to merge 1 commit into from

Conversation

springmeyer
Copy link
Contributor

This PR removes a warning that is only known to be helpful for arch users that enable LTO (#3480) and warns unnecessary for ubuntu versions built per the wiki (https://github.com/Project-OSRM/osrm-backend/wiki/Building-on-Ubuntu). I think the noise of this warning is just not worth it, so this removes it.

@daniel-j-h
Copy link
Member

Arch Linux is the only platform we know about not compiling with mason and lto because @TheMarex is running it, right? Why not leave the warning in then, until we fix it for all systems?

@TheMarex
Copy link
Member

When setting on source compilation on an Ubuntu 14.04 box using the ubuntu testing toolchain and clang-3.8 from the llvm PPA I had similar problems.

I can try to reproduce this in a docker container, but there is definitely something weird going on with different binuitils/clang/libstdc++ combinations.

@MoKob MoKob added the Review label Jan 2, 2017
@daniel-j-h
Copy link
Member

With docker run -it ubuntu:16.04 /bin/bash and following the Mason Wiki docs the Clang lto plugin included with mason (3.9.0) segfaults by default - you have to disable LTO manually: #3501 (comment)

@springmeyer
Copy link
Contributor Author

the Clang lto plugin included with mason (3.9.0) segfaults by default

This is a clang bug and not an issue with OSRM or mason as noted at #3501 (comment).

My preference would be to disable LTO by default, then keep this warning in place. Thoughts?

@springmeyer
Copy link
Contributor Author

When setting on source compilation on an Ubuntu 14.04 box using the ubuntu testing toolchain and clang-3.8 from the llvm PPA I had similar problems.

@TheMarex please ticket when you have more details.

@daniel-j-h
Copy link
Member

This is a clang bug and not an issue with OSRM or mason as noted at #3501 (comment).

Users don't care who is to blame here - if the default installation instructions for OSRM result in segfaults we're doing something wrong. I will update the installation instructions to use Clang 3.9.1 and also disable LTO now by default. LTO is still resulting in segfaults and linker errors from time to time on some platforms and some compiler + binutils combinations. Not recommended by default, unless you know what you're doing and need the performance.

daniel-j-h added a commit that referenced this pull request Jan 5, 2017
This disables the `-flto` LTO flag by default since we're seeing
segfaults in compiler lto plugins, binutils and linker errors again and
again for various clang / gcc / binutils combinations.

Pass `-DNEBALE_LTO` to `cmake` in order to re-enable LTO.

LTO situation in short:
- LTO does not work at all for gcc<4.9
- With gcc>=4.9 the "slim" LTO format is getting used dumping IR
- Older binutils need LTO plugins which know how to read this IR
- Recent binutils handle this format all by themselves
- LLVM is more or less the same with some Clang versions segfaulting

If you need the performance benefit of LTO, make sure your compiler and
binutils are up to date and see for yourself if LTO builds work for you.

References:
- https://gcc.gnu.org/wiki/LinkTimeOptimizationFAQ
- #3481 (comment)
- #3501
- #3441

(and a ton of other LTO tickets if you search for them)
daniel-j-h added a commit that referenced this pull request Jan 6, 2017
This disables the `-flto` LTO flag by default since we're seeing
segfaults in compiler lto plugins, binutils and linker errors again and
again for various clang / gcc / binutils combinations.

Pass `-DNEBALE_LTO` to `cmake` in order to re-enable LTO.

LTO situation in short:
- LTO does not work at all for gcc<4.9
- With gcc>=4.9 the "slim" LTO format is getting used dumping IR
- Older binutils need LTO plugins which know how to read this IR
- Recent binutils handle this format all by themselves
- LLVM is more or less the same with some Clang versions segfaulting

If you need the performance benefit of LTO, make sure your compiler and
binutils are up to date and see for yourself if LTO builds work for you.

References:
- https://gcc.gnu.org/wiki/LinkTimeOptimizationFAQ
- #3481 (comment)
- #3501
- #3441

(and a ton of other LTO tickets if you search for them)
@daniel-j-h
Copy link
Member

LTO is off by default with #3524 - let's keep this warning. Users will only see it if they enable lto and are using mason.

@daniel-j-h daniel-j-h closed this Jan 6, 2017
@springmeyer springmeyer deleted the remove-lto-warning branch January 18, 2017 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants