Enable const propagation into operands at mir_opt_level=2#77107
Enable const propagation into operands at mir_opt_level=2#77107bors merged 1 commit intorust-lang:masterfrom
Conversation
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 6ebd9dcb210c82a59ce07351dcead6f255632b8a with merge 8ead925df872b31e6ed360837751ac75e873642b... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
Queued 8ead925df872b31e6ed360837751ac75e873642b with parent 33aa8be, future comparison URL. |
|
Finished benchmarking try commit (8ead925df872b31e6ed360837751ac75e873642b): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
Looks like this doesn't cause additional codegen units anymore, but still causes a slowdown in llvm optimizing |
|
The slowdown seems not too bad, and it looks like some of the performance actually increased, maybe because rustc is more optimized itself. |
jyn514
added
I-compiletime
|
How can you tell there's a slowdown wrt LLVM? |
|
You can expand the entry and then click on the percentage. That opens a new view: https://perf.rust-lang.org/detailed-query.html?commit=8ead925df872b31e6ed360837751ac75e873642b&base_commit=33aa8be8b5fa6872186a94d9e1fa5088da386e4b&benchmark=regex-opt&run_name=full |
It seems faster in some places but slower in other. |
|
This shows that at the very least we should set it as mir-opt-level=2, there's no reason to keep in in level 3 anymore |
|
The tests that improved the most are ctfe (compile time function evaluation) stress tests. Most of the real-world tests didn't show as much improvement so, as @oli-obk said, this seems like a good candidate for mir-opt-level=2. |
|
Is it okay if I do it in this PR, just by setting in the same place as the current diff, or is there something else to do? Also, what are the criteria for the different mir-opt-levels? It's not really well documented as far as I can see. |
|
Yes, it's fine to do in this PR, I'd just adjust the PR title to make it clear what's going on. The criteria we're using in for mir-opt-levels comes from this compiler-team Major Change Proposal: rust-lang/compiler-team#319. We should figure out how to document that better, perhaps by upstreaming that policy to the rustc-guide or something? |
bugadani
changed the title
|
Thank you. I've adjusted the comments as well to indicate that the current level of slowdown is not as severe as before, but still exists, I hope that's all right. |
|
@bors r+ that's great. Thanks! |
|
📌 Commit 90c7731 has been approved by |
bors
added
the
S-waiting-on-bors
|
@bors rollup- |
…as-schievink Rollup of 10 pull requests Successful merges: - rust-lang#76917 (Add missing code examples on HashMap types) - rust-lang#77107 (Enable const propagation into operands at mir_opt_level=2) - rust-lang#77129 (Update cargo) - rust-lang#77167 (Fix FIXME in core::num test: Check sign of zero in min/max tests.) - rust-lang#77184 (Rust vec bench import specific rand::RngCore) - rust-lang#77208 (Late link args order) - rust-lang#77209 (Fix documentation highlighting in ty::BorrowKind) - rust-lang#77231 (Move helper function for `missing_const_for_fn` out of rustc to clippy) - rust-lang#77235 (pretty-print-reparse hack: Rename some variables for clarity) - rust-lang#77243 (Test more attributes in test issue-75930-derive-cfg.rs) Failed merges: r? `@ghost`
9 participants
Feature was added in #74507 but gated with
mir_opt_level>=3because of compile time regressions. Let's see whether the LLVM 11 update solves that.As the perf results show, enabling this optimization results in a lot less regression as before.
cc @oli-obk
r? @ghost