Faster compile times for release builds with llvm fix

[andjo403]

Run global optimizations after the inliner to avoid spending time on optimizing dead code.

fixes #44655

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[andjo403]

r? @alexcrichton

[alexcrichton]

@bors: r+ p=1

Raising priority as this seems like some awesome compile-time wins. Thanks so much for discovering this @andjo403! And also thanks so much to @dcci for helping get this patch upstream in LLVM itself.

[bors]

📌 Commit 8fd3c8f has been approved by alexcrichton

[bors]

⌛ Testing commit 8fd3c8f with merge f5e036a...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing f5e036a to master...

[alexcrichton]

@andjo403 hm I seem to surprisingly see no difference in optimized benchmarks on perf.rust-lang.org, mind double-checking locally to see if this is fixed?

[andjo403]

@alexcrichton I'm as surprised as you. did not think of do a extra test to see that the faster compile time was there when the submodule was updated due to hade made so many test before for the PR to the llvm repo. the strange part is that I see that the extra passes is present in printout from cargo rustc --release -- Ztime-llvm-passes but no speed up. will try to reproduce the earlier findings

[andjo403]

@alexcrichton @dcci the current llvm patches is do not give as much gain as my original did. Have added a comment in the llvm review why the original placement of the passes was not good.

[alexcrichton]

Ok thanks! I'll watch the LLVM bug

[dotdash]

@alexcrichton there was no update to the LLVM trigger file, maybe that's why?

Edit: that would only explain the problem on the bots though, not that it's reproducible locally

[alexcrichton]

@dotdash ah unfortunately that wouldn't cause this, our CI now unconditionally builds LLVM from scratch on all builds (using sccache to not be super slow), so that'd account for local checkouts not rebuilding but not CI

[andjo403]

sadly this is all my fault. failed to the test the updated patch from llvm before making this PR and according to comment in llvm review my first patch that showed the big improvements

breaks the entire pipelining model of the LLVM passes and inliner.

and the way forward is

The only thing I know to help at that point would be to build a CGSCC version of GlobalDCE.
This is something we've wanted for quite some time, but it is a fair amount of work.

Edit: but do not know how to do that

[alexcrichton]

@andjo403 I sort of forget at this point, your original fix, was there a known problem with it?

[andjo403]

@alexcrichton I hade no problems with it, but it sounds bad to brake the entire pipelining model of llvm.
I do not know what that the result of braking that model is.

[andjo403]

@alexcrichton from what I have understand/learned from the llvm review I think that most of the time improvement was due to I broke the pipeline. so think that I will close the ticket #44655 as it is not possible to implement

edit: but after writing this I started to think of the improvements of the compile time that we hade. so even if the binary was less optimized it was faster

[alexcrichton]

@andjo403 alas :(