ganler commented on code in PR #14417:
URL: https://github.com/apache/tvm/pull/14417#discussion_r1152449966


##########
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:
   I see. Because the constraint or relation between nodes may or may not be 
single-way so in the beginning I made it bidirectional such that the pattern 
can be matched as long as you can let any node of the matched subgraph be start 
hint and get the pattern matched eventually. Specifying certain matching order 
definitely makes the code logic and debugging easier but I am afraid it also 
cuts the flexibility in some way. Not sure if it is worth to have some 
flexibility or if we can keep them both. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to