Uh oh!
There was an error while loading. Please reload this page.
A way to remove otherwise unused locals from MIR - #36306
Merged
Merged
Conversation
MemberAuthor
There was a problem hiding this comment.
Got cherry-picked from the dataflow commit. Doesn’t matter here, will remove.
nagisaforce-pushed
the
mir-local-cleanup
branch
from
September 6, 2016 21:49
10f1b2c to
08c2d29Comparebors
commented
Sep 7, 2016
Collaborator
☔ The latest upstream changes (presumably #36296) made this pull request unmergeable. Please resolve the merge conflicts. |
arielb1
commented
Sep 8, 2016
Contributor
I think we want a pass that also removes written-but-never-used locals. |
eddyb
commented
Sep 8, 2016
Contributor
@arielb1 Isn't that DSE? That would remove the stores, but not the locals themselves. |
nagisaforce-pushed
the
mir-local-cleanup
branch
from
September 8, 2016 14:29
08c2d29 to
e4a52acComparenagisa
commented
Sep 8, 2016
MemberAuthor
Yes, that’s dead store/dead code elimination (yes, we certainly want one). It would work well together with this pass in a sense that dead store/dead code pass wouldn’t need to bother with remapping the locals or removing the declarations; that’s something this pass would do for it later. This pass is still useful without DSE/DCE because we happen to generate (and remove some uses in subsequent passes) variables which end up not being used. |
bors
commented
Sep 29, 2016
Collaborator
☔ The latest upstream changes (presumably #36752) made this pull request unmergeable. Please resolve the merge conflicts. |
alexcrichton
commented
Oct 31, 2016
Member
ping, what's the next steps on this? |
nagisa
commented
Nov 1, 2016
MemberAuthor
Needs a review, I guess? |
alexcrichton
commented
Nov 1, 2016
Member
In that case, ping @eddyb |
eddyb
commented
Nov 1, 2016
Contributor
r=me (ping me after rebase) |
Replaces the hack where a similar thing is done within trans.
nagisaforce-pushed
the
mir-local-cleanup
branch
from
November 3, 2016 04:17
e4a52ac to
4752367Comparenagisa
commented
Nov 3, 2016
MemberAuthor
Rebased. |
eddyb
commented
Nov 3, 2016
Contributor
@bors r+ |
bors
commented
Nov 3, 2016
Collaborator
📌 Commit 4752367 has been approved by |
bors
commented
Nov 3, 2016
Collaborator
bors added a commit
that referenced
this pull request
Nov 3, 2016
A way to remove otherwise unused locals from MIR There is a certain amount of desire for a pass which cleans up the provably unused variables (no assignments or reads). There has been an implementation of such pass by @scottcarr, and another (two!) implementations by me in my own dataflow efforts. PR like #35916 proves that this pass is useful even on its own, which is why I cherry-picked it out from my dataflow effort. @nikomatsakis previously expressed concerns over this pass not seeming to be very cheap to run and therefore unsuitable for regular cleanup duties. Turns out, regular cleanup of local declarations is not at all necessary, at least now, because majority of passes simply do not (or should not) care about them. That’s why it is viable to only run this pass once (perhaps a few more times in the future?) per function, right before translation. r? @eddyb or @nikomatsakis
bors
commented
Nov 3, 2016
Collaborator
💔 Test failed - auto-mac-32-opt |
alexcrichton
commented
Nov 3, 2016
Member
@bors: retry |
bors
commented
Nov 3, 2016
Collaborator
alexcrichton
commented
Nov 3, 2016
Member
@bors: retry force clean |
bors
commented
Nov 3, 2016
Collaborator
bors added a commit
that referenced
this pull request
Nov 3, 2016
A way to remove otherwise unused locals from MIR There is a certain amount of desire for a pass which cleans up the provably unused variables (no assignments or reads). There has been an implementation of such pass by @scottcarr, and another (two!) implementations by me in my own dataflow efforts. PR like #35916 proves that this pass is useful even on its own, which is why I cherry-picked it out from my dataflow effort. @nikomatsakis previously expressed concerns over this pass not seeming to be very cheap to run and therefore unsuitable for regular cleanup duties. Turns out, regular cleanup of local declarations is not at all necessary, at least now, because majority of passes simply do not (or should not) care about them. That’s why it is viable to only run this pass once (perhaps a few more times in the future?) per function, right before translation. r? @eddyb or @nikomatsakis
alexcrichton
commented
Nov 4, 2016
Member
@bors: retry force clean |
bors
commented
Nov 4, 2016
Collaborator
bors added a commit
that referenced
this pull request
Nov 4, 2016
A way to remove otherwise unused locals from MIR There is a certain amount of desire for a pass which cleans up the provably unused variables (no assignments or reads). There has been an implementation of such pass by @scottcarr, and another (two!) implementations by me in my own dataflow efforts. PR like #35916 proves that this pass is useful even on its own, which is why I cherry-picked it out from my dataflow effort. @nikomatsakis previously expressed concerns over this pass not seeming to be very cheap to run and therefore unsuitable for regular cleanup duties. Turns out, regular cleanup of local declarations is not at all necessary, at least now, because majority of passes simply do not (or should not) care about them. That’s why it is viable to only run this pass once (perhaps a few more times in the future?) per function, right before translation. r? @eddyb or @nikomatsakis
bors
commented
Nov 4, 2016
Collaborator
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
There is a certain amount of desire for a pass which cleans up the provably unused variables (no assignments or reads). There has been an implementation of such pass by @scottcarr, and another (two!) implementations by me in my own dataflow efforts.
PR like #35916 proves that this pass is useful even on its own, which is why I cherry-picked it out from my dataflow effort.
@nikomatsakis previously expressed concerns over this pass not seeming to be very cheap to run and therefore unsuitable for regular cleanup duties. Turns out, regular cleanup of local declarations is not at all necessary, at least now, because majority of passes simply do not (or should not) care about them. That’s why it is viable to only run this pass once (perhaps a few more times in the future?) per function, right before translation.
r? @eddyb or @nikomatsakis