Copy propagation, remove unused values (closures!) and local variables #4837
Conversation
This file contains 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
This allows using an AliasingAnalyzer for copy propagation (subsequent commit).
|
Review by @retronym |
|
Community build running here: https://scala-ci.typesafe.com/job/scala-2.12.x-integrate-community-build/205/parameters/ |
|
The community build found some issues, will look at those next week: also akka and twitter-util failed, the rest passed. |
|
(the twitter-util failure is unrelated to your changes) |
Previously the VarInstruction extractor did not include IINCs. Also give rename the isVarInstruction predicate to isLoadStoreOrRet, as it doesn't include IINC. Also fixes a bug in removeJumpAndAdjustStack, it checked the wrong opcode range before.
When running DCE, directly remove eliminated callsites from the call graph (instead delegating this task to the callees).
By convention (for performance), `aliasingFrame.aliases(slot) == null` means that the value at `slot` has no aliases, so this is isomorphic to having a singleton `AliasSet(slot)`. When merging an AliasingFrame `other` into `this`, at every value slot we have to keep the intersection of the two alias sets. In other words we have to remove from `this.aliases(slot)` those values that are not in `other.aliases(slot)`. This was wrong for the cases where `other.aliases == null` - we did not treat this case as a singleton set.
Copy propagation uses an AliasingAnalyzer: it replaces a `LOAD n` instruction by `LOAD m` where m is the smallest alias of n. This leads to stale STORE instructions. Stale STOREs are identified using a ProdCons analyzer and replaced by POPs. Values that are pushed on the stack by a side-effect free instruction and consumed by a POP are then removed by `eliminatePushPop`. This includes elimination of unused closure allocations and unused boxes and tuple allocations (*). A final cleanup eliminates `STORE x; LOADx` pairs where the stored value is not otherwise used. Fixes - scala/scala-dev#25 - scala/scala-dev#7 - scala/scala-dev#14 - scala/scala-dev#12 (*) We don't yet rewrite reads of boxes and tuples yet. For example, `val x = (1, 2); x._1` remains a method invocation and the tuple cannot be eliminated (scala/scala-dev#11). Inspired in many ways by Miguel's work!
|
community build (with the optimizer enabled) succeeded: https://scala-ci.typesafe.com/view/scala-2.12.x/job/scala-2.12.x-integrate-community-build/213/ |
|
I'll take another pass over this tomorrow. |
|
closing in favor of #4858 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See individual commit messages.