masahi commented on code in PR #14417: URL: https://github.com/apache/tvm/pull/14417#discussion_r1152443315
########## src/relax/ir/dataflow_matcher.cc: ########## @@ -541,15 +541,21 @@ struct RNode { * \brief This method try to match a real node and a pattern node along with its neighbors. */ static bool try_match(PNode* p, RNode* r, DFPatternMatcher* m, - const std::map<const VarNode*, std::set<const VarNode*>>& def2use, + const std::map<const VarNode*, std::vector<const VarNode*>>& def2use, const std::map<const VarNode*, std::vector<const VarNode*>>& use2def) { - if (nullptr != p->matched && p->matched == r->ptr) return true; // matched before. + if (p->matched != nullptr && p->matched == r->ptr) return true; // matched before. if (!m->Match(GetRef<DFPattern>(p->ptr), GetRef<Var>(r->ptr))) return false; std::stack<std::pair<PNode*, RNode*>> undo_stack{}; const auto commit = [&undo_stack](PNode* p, RNode* r) { // match with each other. + // TODO(ganler, masahi): Why commit on the same p-r pair happens more than once? Review Comment: Here my concern is not about performance. If the same pair is committed more than once, I think there is something odd about the matching algorithm. Later I'll try to improve the matching algorithm implementation. In particular, I want to remove `try_match` loop on parents, https://github.com/apache/tvm/blob/278bc9bbdf7d9b12c44bb78b71f91e4872b14353/src/relax/ir/dataflow_matcher.cc#L577-L607. This bidirectional matching has been a source of confusion to me - Since the constraint graph is assumed to be a DAG, I think we can start matching from the root nodes in a topo-sorted order, and matching should be able to proceed purely in one direction. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org