-
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
Allow early otherwise branch on mir-opt-level=2 #82905
Allow early otherwise branch on mir-opt-level=2 #82905
Conversation
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit f37adef with merge 76689995512a02828b62e1adda15545c90423f80... |
☀️ Try build successful - checks-actions |
Queued 76689995512a02828b62e1adda15545c90423f80 with parent 1d6b0f6, future comparison URL. |
Finished benchmarking try commit (76689995512a02828b62e1adda15545c90423f80): 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 |
Many improvements of up to 0.6%. Some regressions up to 0.5%. |
Any changes to non cc @rust-lang/wg-mir-opt I think we have our first candidate for a MIR optimization that we only turn on in release mode. As to what this optimization does, I think the documentation explains that best: rust/compiler/rustc_mir/src/transform/early_otherwise_branch.rs Lines 8 to 23 in f37adef
|
I think the transformation is incorrect for reasons described in #78496 (comment). |
@spastorino please add a check for unsound mir opts to this optimization, referencing the linked issue. |
Closing this one as I've just opened #83277. |
…und, r=oli-obk Mark early otherwise optimization unsound r? `@oli-obk` cc `@tmiasko` Related to rust-lang#78496 and rust-lang#82905 Should I also bump this one to level 3 or 4 or given that is unsound it doesn't matter?. Probably need to adjust some tests.
Let's test the performance of making this change, note that mir-opt-level is 2 by default in optimized builds.
r? @oli-obk