-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
[perf] Measure Destination Propagation performance #72635
Conversation
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
r? @ghost @bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 0428499dd5e7cf2b0cb239e109d53edfa03b16d8 with merge 2ff3d6679dd95841be552b208843a29851ab3296... |
💔 Test failed - checks-azure |
☔ The latest upstream changes (presumably #72935) made this pull request unmergeable. Please resolve the merge conflicts. |
The new diff is to convince me that this is correct and nothing funky is going on.
The additional copies are due to the lack of copy propagation
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 4902abb6d6c2a56501c1724955b096f9a926d468 with merge 54d23ab3a579a168e304187cd36a34db01b718b4... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☀️ Try build successful - checks-azure |
Queued 54d23ab3a579a168e304187cd36a34db01b718b4 with parent ff5b446, future comparison URL. |
Finished benchmarking try commit (54d23ab3a579a168e304187cd36a34db01b718b4): comparison url. |
Very mixed results. |
Some MIR statements and terminators have an (undocumented...) invariant that some of their input and outputs must not overlap. This records conflicts between locals used in these positions.
Looks basically as expected, I'd say. The biggest slowdown is ~10%, in ucd-check.
I've pushed a commit that skips functions that have too many blocks (in addition to skipping ones with too many locals). @bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit d36b287 with merge 050a0aaf22905193a289b2687e085242282e9f74... |
(this still regresses |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☀️ Try build successful - checks-azure |
Queued 050a0aaf22905193a289b2687e085242282e9f74 with parent d8ed1b0, future comparison URL. |
Finished benchmarking try commit (050a0aaf22905193a289b2687e085242282e9f74): comparison url. |
Huh, that did nothing to the ucd benchmarks, odd. |
@jonas-schievink any updates? |
This was just for perf runs of #72632, closing in favor of that. |
Implement a generic Destination Propagation optimization on MIR This takes the work that was originally started by @eddyb in rust-lang#47954, and then explored by me in rust-lang#71003, and implements it in a general (ie. not limited to acyclic CFGs) and dataflow-driven way (so that no additional infrastructure in rustc is needed). The pass is configured to run at `mir-opt-level=2` and higher only. To enable it by default, some followup work on it is still needed: * Performance needs to be evaluated. I did some light optimization work and tested against `tuple-stress`, which caused trouble in my last attempt, but didn't go much in depth here. * We can also enable the pass only at `opt-level=2` and higher, if it is too slow to run in debug mode, but fine when optimizations run anyways. * Debuginfo needs to be fixed after locals are merged. I did not look into what is required for this. * Live ranges of locals (aka `StorageLive` and `StorageDead`) are currently deleted. We either need to decide that this is fine, or if not, merge the variable's live ranges (or remove these statements entirely – rust-lang#68622). Some benchmarks of the pass were done in rust-lang#72635.
Implement a generic Destination Propagation optimization on MIR This takes the work that was originally started by `@eddyb` in rust-lang#47954, and then explored by me in rust-lang#71003, and implements it in a general (ie. not limited to acyclic CFGs) and dataflow-driven way (so that no additional infrastructure in rustc is needed). The pass is configured to run at `mir-opt-level=2` and higher only. To enable it by default, some followup work on it is still needed: * Performance needs to be evaluated. I did some light optimization work and tested against `tuple-stress`, which caused trouble in my last attempt, but didn't go much in depth here. * We can also enable the pass only at `opt-level=2` and higher, if it is too slow to run in debug mode, but fine when optimizations run anyways. * Debuginfo needs to be fixed after locals are merged. I did not look into what is required for this. * Live ranges of locals (aka `StorageLive` and `StorageDead`) are currently deleted. We either need to decide that this is fine, or if not, merge the variable's live ranges (or remove these statements entirely – rust-lang#68622). Some benchmarks of the pass were done in rust-lang#72635.
Based on #72632