makes dead code elimination less conservative #906
Merged
Conversation
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
As was noted in BinaryAnalysisPlatform#905 the optimization passes sometimes misses an opportunity to remove obvious assignments. The underlying reason is that we compute a very conservative set of variables which are used for interprocedural communication. And if a variable is a member of this set then we do not delete it (under assumption, that it could be used in other subroutines). What was overconservative is that all versions of the same variable were protected, while only the last definition one could be used for interprocedural communication. The propsed change relaxes the protected set, and removes only the last version of a variable from the set of the dead variables. Fixes BinaryAnalysisPlatform#905
Unfortunately, the original proposal was unsound, as the assumption that only the last definition should be preserved is implied by the assumption that only the last definition is used for interprocedural communication, which is totally wrong. The liveness property is propagated from each call (and indirect jump) to all members of the protected set. We're using the SSA form for def-use analysis which is inherently intraprocedural, so to preserve the liveness of the variables with the interprocedural scope we use the protected set as an overapproximation. A tighter approximation would be to reinstantiate their liveness after each call or indirect jump. However, this would require tampering with the ssa algorithm, that we don't want to do right now. The updated solution reinstantiates liveness of protected variables at the end of each block, even the block is not a call or even jump. The liveness is killed by the first use of protection, so it is propagated only to the last (if any) definition.
note: this PR shall be merged after BinaryAnalysisPlatform#907 is merged
gitoleg
reviewed
Dec 11, 2018
| let computed_def_use sub = | ||
| def_use_collector#visit_sub sub (Var.Set.empty,Var.Set.empty) | ||
|
|
||
| let protect dead protected = |
There was a problem hiding this comment.
dead code elimination requested)
to make it cleaner
gitoleg
approved these changes
Dec 11, 2018
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.
As was noted in #905 the optimization passes sometimes misses an
opportunity to remove obvious assignments. The underlying reason
is that we compute a very conservative set of variables which are
used for interprocedural communication. And if a variable is a
member of this set then we do not delete it (under assumption, that
it could be used in other subroutines).
What was overconservative is that all versions of the same variable
were protected, while only the last definition one could be used for
interprocedural communication. The propsed change relaxes the
protected set, and removes only the last version of a variable from
the set of the dead variables.
Fixes #905