[Unity][Transform] Canonicalize and use CSE between pattern matches#15904
Merged
masahi merged 3 commits intoapache:unityfrom Oct 13, 2023
Merged
Conversation
masahi
approved these changes
Oct 11, 2023
Member
masahi
left a comment
masahi
left a comment
There was a problem hiding this comment.
Makes sense, please fix the tests.
Lunderberg
force-pushed
the
unity_improved_robustness_of_pattern_matching
branch
from
ae05763 to
c3d3bad
Compare
Contributor
Author
|
Tests should now be fixed. Rebased on |
Contributor
Author
|
Current CI failure is a segfault while generating TVMScript output. The segfault is caused by a dangling reference, and is resolved by #15923. |
Member
|
@tvm-bot rerun |
Contributor
|
Failed to re-run CI in https://github.com/apache/tvm/actions/runs/6502819765 Detailswith response |
The `PatternRewriter` is intended to iterate until no matching patterns remain. Prior to this commit, this only involved repeating the pattern match rewrite rules. However, intermediate results produced by pattern replacement could cause the iterative pattern matching to terminate early. * If two rewrite rules each introduce the same intermediate, there will exist two copies of that intermediate, which can prevent `only_used_by` patterns from matching. Applying `EliminateCommonSubexpr` allows the pattern matching to continue. * Applying a rewrite rule may result in dangling intermediates that are no longer used. These dangling intermediates may prevent the next application of a rewrite rule that uses the `only_used_by` constraint. Applying `RemoveAllUnused` allows the pattern matching to continue. * A rewrite rule that returns a `relax::Var` or `relax::TupleGetItem` as the replacement introduces trivial var-to-var rebinding, which are not tracked by `PatternRewriter`. Applying `CanonicalizeBindings` allows the pattern matching to continue. While this could be fixed externally by repeatedly applying `rewrite_call`, this would require re-inspecting the entire function, and not just the dataflow block in which the replacement occurred.
Lunderberg
force-pushed
the
unity_improved_robustness_of_pattern_matching
branch
from
c3d3bad to
001298d
Compare
Contributor
Author
|
Thank you for re-launching the CI, and that's a weird HTTP error from it. I've started it up again, and hopefully it won't have the same issue. |
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
2 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.
The
PatternRewriteris intended to iterate until no matching patterns remain, as implemented in #14446 and #15495. Prior to this commit, this only involved repeating the pattern match rewrite rules. However, intermediate results produced by pattern replacement could cause the iterative pattern matching to terminate early.If two rewrite rules each introduce the same intermediate, there will exist two copies of that intermediate, which can prevent
only_used_bypatterns from matching. ApplyingEliminateCommonSubexprallows the pattern matching to continue.Applying a rewrite rule may result in dangling intermediates that are no longer used. These dangling intermediates may prevent the next application of a rewrite rule that uses the
only_used_byconstraint. ApplyingRemoveAllUnusedallows the pattern matching to continue.A rewrite rule that returns a
relax::Varorrelax::TupleGetItemas the replacement introduces trivial var-to-var rebinding, which are not tracked byPatternRewriter. ApplyingCanonicalizeBindingsallows the pattern matching to continue.While this could be fixed externally by repeatedly applying
rewrite_call, this would require re-inspecting the entire function, and not just the dataflow block in which the replacement occurred.