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

rustc: Disable ThinLTO for LLVM's build #56890

Closed
wants to merge 1 commit into from

Conversation

alexcrichton
Copy link
Member

This commit goes back to disabling ThinLTO for building LLVM and its
tools. It turns out that this caused a regression at #56849 which I
believe we'll definitely want to fix before it goes live. The bisection
turned up an upstream LLVM bug, so hopefully there'll eventually
be a fix we can backport.

I've also been a little uneasy about how we're only enabling ThinLTO for
LLVM on Linux, and we haven't had a chance to turn it on for OSX and
Windows (other "Tier 1" platforms) yet. Additionally it had an
unexpected hit to CI time we never got around to fixing. I'd ideally
prefer that we fix these issues as well before re-landing.

Closes #56849

This commit goes back to disabling ThinLTO for building LLVM and its
tools. It turns out that this caused a regression at rust-lang#56849 which I
believe we'll definitely want to fix before it goes live. The bisection
turned up an [upstream LLVM bug][bug], so hopefully there'll eventually
be a fix we can backport.

I've also been a little uneasy about how we're only enabling ThinLTO for
LLVM on Linux, and we haven't had a chance to turn it on for OSX and
Windows (other "Tier 1" platforms) yet. Additionally it had an
unexpected hit to CI time we never got around to fixing. I'd ideally
prefer that we fix these issues as well before re-landing.

Closes rust-lang#56849

[bug]: https://bugs.llvm.org/show_bug.cgi?id=40045
@alexcrichton alexcrichton added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 16, 2018
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 16, 2018
@alexcrichton
Copy link
Member Author

r? @michaelwoerister

@nikic
Copy link
Contributor

nikic commented Dec 16, 2018

To double check, this is a bug in the clang 7.0 we use to compile LLVM and does not affect our own use of LLVM (which should have https://reviews.llvm.org/rL339301), right?

@alexcrichton
Copy link
Member Author

@nikic correct yeah, and since this is already fixed in LLVM I'm going to submit a different PR to fix this.

@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 16, 2018
@Mark-Simulacrum
Copy link
Member

De-beta nominating since we're not planning on landing this.

@alexcrichton
Copy link
Member Author

For posterity I've submitted #56892 which also fixes the original issue by updating LLD, and #56893 which fixes the bug on beta by going back to Clang 6

@alexcrichton alexcrichton deleted the less-thin branch December 17, 2018 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Latest nightly crashes on centos 5
6 participants