[Relay] Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc.#9542
Merged
tmoreau89 merged 2 commits intoapache:mainfrom Nov 23, 2021
Merged
[Relay] Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc.#9542tmoreau89 merged 2 commits intoapache:mainfrom
tmoreau89 merged 2 commits intoapache:mainfrom
Conversation
mbs-octoml
requested review from
MarisaKirisame,
ZihengJiang,
anijain2305,
areusch,
comaniac,
icemelon,
jroesch,
junrushao,
jwfromm,
mbrookhart,
merrymercy,
slyubomirsky,
tqchen,
vinx13,
wweic,
yzhliu and
zhiics
as code owners
mbs-octoml
changed the title
mbs-octoml
force-pushed
the
mbs-dead-code
branch
from
3ba6e79 to
16c2342
Compare
jroesch
approved these changes
Nov 22, 2021
Member
jroesch
left a comment
jroesch
left a comment
There was a problem hiding this comment.
It would be good to get a second set of eyes but looks good to me. Modulo my comment.
Contributor
|
LGTM |
As part of apache#9483 we need to prepare some critical Relay passes for running after lowering and conversion to DPS. For DCE we need to make sure we never remove side-effecting let-bound expressions, such as for allocation or evaluation of an external function with unknown effectfulness. Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting for functions by tracking both 'eval' and 'call' purity, but must fallback to assuming call-impurity in more difficult cases (eg calling a function passed as a parameter, calling a function projected from a tuple, etc). However it seems plenty good enough. Purity must also be accounted for when determining the usage count of let-bound variables, so reworked that. Collapsed the let-bound value accumulation pass into the usage counting pass to make up for inserting the new purity analysis pass. A few tests assume DCE eliminates dead reference writes. The previous implementation certainly did that, but by eliminating *all* writes. Filed CORE-118 to extend DCE to soundly elim dead writes (a very simple-minded analysis would probably do just fine and we don't need to get hung up on alias analysis). In the meantime, added an 'ignore_impurity' flag (default False) and set to true just in the few unit tests which rely on the unsound impl.
electriclilies
approved these changes
Nov 23, 2021
mbs-octoml
force-pushed
the
mbs-dead-code
branch
from
16c2342 to
3f73bbc
Compare
Contributor
|
Thank you @mbs-octoml @mikepapadim @electriclilies @jroesch ; the PR has been merged |
dchauhan-arm
pushed a commit
to dchauhan-arm/tvm
that referenced
this pull request
Nov 29, 2021
…festAlloc. (apache#9542) * Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc. As part of apache#9483 we need to prepare some critical Relay passes for running after lowering and conversion to DPS. For DCE we need to make sure we never remove side-effecting let-bound expressions, such as for allocation or evaluation of an external function with unknown effectfulness. Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting for functions by tracking both 'eval' and 'call' purity, but must fallback to assuming call-impurity in more difficult cases (eg calling a function passed as a parameter, calling a function projected from a tuple, etc). However it seems plenty good enough. Purity must also be accounted for when determining the usage count of let-bound variables, so reworked that. Collapsed the let-bound value accumulation pass into the usage counting pass to make up for inserting the new purity analysis pass. A few tests assume DCE eliminates dead reference writes. The previous implementation certainly did that, but by eliminating *all* writes. Filed CORE-118 to extend DCE to soundly elim dead writes (a very simple-minded analysis would probably do just fine and we don't need to get hung up on alias analysis). In the meantime, added an 'ignore_impurity' flag (default False) and set to true just in the few unit tests which rely on the unsound impl. * [checkpoint] Merge Lily's suggestions.
mehrdadh
pushed a commit
to mehrdadh/tvm
that referenced
this pull request
Dec 1, 2021
…festAlloc. (apache#9542) * Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc. As part of apache#9483 we need to prepare some critical Relay passes for running after lowering and conversion to DPS. For DCE we need to make sure we never remove side-effecting let-bound expressions, such as for allocation or evaluation of an external function with unknown effectfulness. Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting for functions by tracking both 'eval' and 'call' purity, but must fallback to assuming call-impurity in more difficult cases (eg calling a function passed as a parameter, calling a function projected from a tuple, etc). However it seems plenty good enough. Purity must also be accounted for when determining the usage count of let-bound variables, so reworked that. Collapsed the let-bound value accumulation pass into the usage counting pass to make up for inserting the new purity analysis pass. A few tests assume DCE eliminates dead reference writes. The previous implementation certainly did that, but by eliminating *all* writes. Filed CORE-118 to extend DCE to soundly elim dead writes (a very simple-minded analysis would probably do just fine and we don't need to get hung up on alias analysis). In the meantime, added an 'ignore_impurity' flag (default False) and set to true just in the few unit tests which rely on the unsound impl. * [checkpoint] Merge Lily's suggestions.
mehrdadh
pushed a commit
to mehrdadh/tvm
that referenced
this pull request
Dec 1, 2021
…festAlloc. (apache#9542) * Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc. As part of apache#9483 we need to prepare some critical Relay passes for running after lowering and conversion to DPS. For DCE we need to make sure we never remove side-effecting let-bound expressions, such as for allocation or evaluation of an external function with unknown effectfulness. Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting for functions by tracking both 'eval' and 'call' purity, but must fallback to assuming call-impurity in more difficult cases (eg calling a function passed as a parameter, calling a function projected from a tuple, etc). However it seems plenty good enough. Purity must also be accounted for when determining the usage count of let-bound variables, so reworked that. Collapsed the let-bound value accumulation pass into the usage counting pass to make up for inserting the new purity analysis pass. A few tests assume DCE eliminates dead reference writes. The previous implementation certainly did that, but by eliminating *all* writes. Filed CORE-118 to extend DCE to soundly elim dead writes (a very simple-minded analysis would probably do just fine and we don't need to get hung up on alias analysis). In the meantime, added an 'ignore_impurity' flag (default False) and set to true just in the few unit tests which rely on the unsound impl. * [checkpoint] Merge Lily's suggestions.
ylc
pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
…festAlloc. (apache#9542) * Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc. As part of apache#9483 we need to prepare some critical Relay passes for running after lowering and conversion to DPS. For DCE we need to make sure we never remove side-effecting let-bound expressions, such as for allocation or evaluation of an external function with unknown effectfulness. Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting for functions by tracking both 'eval' and 'call' purity, but must fallback to assuming call-impurity in more difficult cases (eg calling a function passed as a parameter, calling a function projected from a tuple, etc). However it seems plenty good enough. Purity must also be accounted for when determining the usage count of let-bound variables, so reworked that. Collapsed the let-bound value accumulation pass into the usage counting pass to make up for inserting the new purity analysis pass. A few tests assume DCE eliminates dead reference writes. The previous implementation certainly did that, but by eliminating *all* writes. Filed CORE-118 to extend DCE to soundly elim dead writes (a very simple-minded analysis would probably do just fine and we don't need to get hung up on alias analysis). In the meantime, added an 'ignore_impurity' flag (default False) and set to true just in the few unit tests which rely on the unsound impl. * [checkpoint] Merge Lily's suggestions.
yangulei
pushed a commit
to yangulei/tvm
that referenced
this pull request
Jan 11, 2022
…festAlloc. (apache#9542) * Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc. As part of apache#9483 we need to prepare some critical Relay passes for running after lowering and conversion to DPS. For DCE we need to make sure we never remove side-effecting let-bound expressions, such as for allocation or evaluation of an external function with unknown effectfulness. Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting for functions by tracking both 'eval' and 'call' purity, but must fallback to assuming call-impurity in more difficult cases (eg calling a function passed as a parameter, calling a function projected from a tuple, etc). However it seems plenty good enough. Purity must also be accounted for when determining the usage count of let-bound variables, so reworked that. Collapsed the let-bound value accumulation pass into the usage counting pass to make up for inserting the new purity analysis pass. A few tests assume DCE eliminates dead reference writes. The previous implementation certainly did that, but by eliminating *all* writes. Filed CORE-118 to extend DCE to soundly elim dead writes (a very simple-minded analysis would probably do just fine and we don't need to get hung up on alias analysis). In the meantime, added an 'ignore_impurity' flag (default False) and set to true just in the few unit tests which rely on the unsound impl. * [checkpoint] Merge Lily's suggestions.
ylc
pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
…festAlloc. (apache#9542) * Prepare DeadCodeElimination for running post LowerTEPass/ManifestAlloc. As part of apache#9483 we need to prepare some critical Relay passes for running after lowering and conversion to DPS. For DCE we need to make sure we never remove side-effecting let-bound expressions, such as for allocation or evaluation of an external function with unknown effectfulness. Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting for functions by tracking both 'eval' and 'call' purity, but must fallback to assuming call-impurity in more difficult cases (eg calling a function passed as a parameter, calling a function projected from a tuple, etc). However it seems plenty good enough. Purity must also be accounted for when determining the usage count of let-bound variables, so reworked that. Collapsed the let-bound value accumulation pass into the usage counting pass to make up for inserting the new purity analysis pass. A few tests assume DCE eliminates dead reference writes. The previous implementation certainly did that, but by eliminating *all* writes. Filed CORE-118 to extend DCE to soundly elim dead writes (a very simple-minded analysis would probably do just fine and we don't need to get hung up on alias analysis). In the meantime, added an 'ignore_impurity' flag (default False) and set to true just in the few unit tests which rely on the unsound impl. * [checkpoint] Merge Lily's suggestions.
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
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.
As part of #9483 we need to prepare some critical Relay passes for running after
lowering and conversion to DPS. For DCE we need to make sure we never remove
side-effecting let-bound expressions, such as for allocation or evaluation of
an external function with unknown effectfulness.
Introduce a new purity pre-pass. It makes a half-hearted attempt at accounting
for functions by tracking both 'eval' and 'call' purity, but must fallback to
assuming call-impurity in more difficult cases (eg calling a function passed as
a parameter, calling a function projected from a tuple, etc). However it seems
plenty good enough.
Purity must also be accounted for when determining the usage count of let-bound
variables, so reworked that. Collapsed the let-bound value accumulation pass into
the usage counting pass to make up for inserting the new purity analysis pass.
Note that some unit tests assume DCE eliminates dead writes to references. The
previous implementation certainly did that, but also eliminated all writes. To do this
soundly will require a bit of liveness analysis. Rather than doing that now I've added
an 'ignore_impurity' flag to DeadCodeElimination() to recover the (unsound) legacy
behavior.