-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Up the LLVM #36508
Up the LLVM #36508
Conversation
Nominating for beta, as it fixes a regression. |
Hm so we've got two parallel updates to the LLVM submodule right now. Both this and the wasm backend. I'd feel pretty uncomfortable about backporting the emscripten backend. To be honest our LLVM backend on beta is at rust-lang/llvm@80ad955 which is way off from what's on nightly... I guess all that's to say:
|
I’d gladly backport the LLVM patch as many times as necessary (its a simple one-liner after all), but I’ll need new branches from (the branches with the backported patch at relevant base commits are https://github.com/nagisa/llvm/tree/backport-281650-beta for beta and https://github.com/nagisa/llvm/tree/backport-281650-master for nightly)
This is a fix for beta regression and we do 🚋 roll-over in just a bit over a week. I’m fine with waiting before backporting the fix to the nightlies’ LLVM (in which case you might have to live through 2 beta backports of the same patch), but beta rust has to get that LLVM patch ASAP. |
I rebased the 2016-07-09 branch to make your patch appear ahead of the fastcomp in the history, so you don't need to worry about any fallout from it, but you'll need to update this PR with the correct new commit. From what I can tell @alexcrichton was mistaken about beta being on a different LLVM. It looks to me like the most recent commit is rust-lang/llvm@eee68ea, which is only one merge behind nightly, this fix to run GVN again rust-lang/llvm@cc2009f. If that looks right to you @nagisa @eddyb we should just be able to take the same commit on both master and beta. |
a797bd2
to
e5c4d78
Compare
Updated the LLVM commit; this is good to go. |
@bors r+ p=1 |
📌 Commit e5c4d78 has been approved by |
⌛ Testing commit e5c4d78 with merge 3a35e66... |
💔 Test failed - auto-win-msvc-64-opt-rustbuild |
The bot in question did not rebuild LLVM. What do? EDIT: it seems like all the rustbuild bots decided that rebuilding LLVM is not necessary. Can we skip/ignore them somehow? |
@nagisa IIRC src/rustllvm/llvm-auto-clean-trigger should also be updated. |
e5c4d78
to
d104e5b
Compare
@bors r+ |
📌 Commit d104e5b has been approved by |
Up the LLVM Fixes #36474 The relevant patch to rust-llvm is at rust-lang/llvm#51 r? @alexcrichton
Accepting for beta. Trivial patch (hiding LLVM changes of course...), regression. |
After looking into this further, @alexcrichton was right that beta and master are on two different merge bases. The LLVM patch is going to need to be ported to the beta LLVM specifically. |
Actually, they do look like the have the same merge-base. For some reason when cherry-picking this git says "Failed to merge submodule src/llvm (commits don't follow merge-base)". I don't understand why. |
Fixes #36474
The relevant patch to rust-llvm is at rust-lang/llvm#51
r? @alexcrichton