[BugFix] Support rewrite_once when the number of callbacks > 1 #14344
Conversation
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
|
What do you think @masahi ? Thank you! |
src/relay/ir/dataflow_matcher.cc
Outdated
| @@ -796,24 +796,38 @@ Expr PatternRewriter::Rewrite(const Array<DFPatternCallback>& callbacks, const E | |||
| bool equal = true; | |||
| static auto* structural_equal = runtime::Registry::Get("node.StructuralEqual"); | |||
| ICHECK(structural_equal) << "node.StructuralEqual is not registered."; | |||
| // Use the callback until the callback's attribute is rewrite_once=true and has been rewritten | |||
| std::unordered_map<DFPatternCallback, bool, ObjectPtrHash, ObjectPtrEqual> callbacks_map; | |||
There was a problem hiding this comment.
callbacks_map -> done, swapping false and true
There was a problem hiding this comment.
Please take another look. Thank you!
src/relay/ir/dataflow_matcher.cc
Outdated
| @@ -796,24 +796,38 @@ Expr PatternRewriter::Rewrite(const Array<DFPatternCallback>& callbacks, const E | |||
| bool equal = true; | |||
| static auto* structural_equal = runtime::Registry::Get("node.StructuralEqual"); | |||
| ICHECK(structural_equal) << "node.StructuralEqual is not registered."; | |||
| // Use the callback until the callback's attribute is rewrite_once=true and has been rewritten | |||
There was a problem hiding this comment.
// Keep track of callbacks that have finished rewriting
sisleyli
force-pushed
the
feat_rewrite_once
branch
from
429c287 to
d274784
Compare
|
It seems that CI has passed, could you please help me merge this PR? Thanks @masahi ! |
sisleyli
force-pushed
the
feat_rewrite_once
branch
from
d274784 to
22f86d9
Compare
I don't know whether our repo will automatically rebase when it merges PR, so I just performed a rebase operation myself, do I need to do this every time the main branch is updated? |
|
No you don't have to do that. You now have to wait for another 4h :) |
|
Thanks masahi, I should have consulted before performing the rebase operation :( |
|
CI passed again...I won't do redundant operations this time.hhhh |
The
rewrite_onceof the currentPatternRewriterrewrites once only when the last callback in the callbacks list isrewrite_once. I don't think this is as expected.I think the
rewrite_oncestrategy should be that the callback marked withrewrite_onceis rewritten only once, and the rest of the callbacks are still rewritten multiple times.