[perf] Measure Destination Propagation performance#72635
Closed
jonas-schievink wants to merge 15 commits intorust-lang:masterfrom
jonas-schievink:dest-prop-perf
Closed
[perf] Measure Destination Propagation performance#72635jonas-schievink wants to merge 15 commits intorust-lang:masterfrom jonas-schievink:dest-prop-perf
jonas-schievink wants to merge 15 commits intorust-lang:masterfrom
jonas-schievink:dest-prop-perf
Conversation
Contributor
|
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Contributor
Author
|
r? @ghost @bors try @rust-timer queue |
Collaborator
|
Awaiting bors try build completion |
Collaborator
|
⌛ Trying commit 0428499dd5e7cf2b0cb239e109d53edfa03b16d8 with merge 2ff3d6679dd95841be552b208843a29851ab3296... |
Collaborator
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-author
Collaborator
|
☔ The latest upstream changes (presumably #72935) made this pull request unmergeable. Please resolve the merge conflicts. |
Elinvynia
added
S-waiting-on-author
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
Contributor
Author
|
@bors try @rust-timer queue |
Collaborator
|
Awaiting bors try build completion |
Collaborator
|
⌛ Trying commit 4902abb6d6c2a56501c1724955b096f9a926d468 with merge 54d23ab3a579a168e304187cd36a34db01b718b4... |
Contributor
|
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 |
Collaborator
|
☀️ Try build successful - checks-azure |
Collaborator
|
Queued 54d23ab3a579a168e304187cd36a34db01b718b4 with parent ff5b446, future comparison URL. |
Collaborator
|
Finished benchmarking try commit (54d23ab3a579a168e304187cd36a34db01b718b4): comparison url. |
Member
|
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.
Contributor
Author
|
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 |
Collaborator
|
Awaiting bors try build completion |
Collaborator
|
⌛ Trying commit d36b287 with merge 050a0aaf22905193a289b2687e085242282e9f74... |
Contributor
Author
|
(this still regresses |
Contributor
|
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 |
Collaborator
|
☀️ Try build successful - checks-azure |
Collaborator
|
Queued 050a0aaf22905193a289b2687e085242282e9f74 with parent d8ed1b0, future comparison URL. |
Collaborator
|
Finished benchmarking try commit (050a0aaf22905193a289b2687e085242282e9f74): comparison url. |
Contributor
Author
|
Huh, that did nothing to the ucd benchmarks, odd. |
crlf0710
added
S-waiting-on-author
|
@jonas-schievink any updates? |
Contributor
Author
|
This was just for perf runs of #72632, closing in favor of that. |
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 20, 2020
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.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 20, 2020
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on #72632