[Fix] Fix recursive let for well formed check

[icemelon]

Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

cc @lixiaoquan @jroesch

[icemelon]

@lixiaoquan Could you check if the pr fixes your issue?

[lixiaoquan]

Thanks for fixing this, I found other passes also have similar issue. This is VarVisitor used in FreeVars().

diff --git a/src/relay/analysis/util.cc b/src/relay/analysis/util.cc
index 0885a35df..633c3b065 100644
--- a/src/relay/analysis/util.cc
+++ b/src/relay/analysis/util.cc
@@ -214,9 +214,13 @@ class VarVisitor : protected ExprVisitor, protected PatternVisitor {
   }
 
   void VisitExpr_(const LetNode* op) final {
-    MarkBounded(op->var);
-    VisitExpr(op->value);
-    VisitExpr(op->body);
+    Expr let = GetRef<Let>(op);
+    while (auto let_node = let.as<LetNode>()) {
+      MarkBounded(let_node->var);
+      VisitExpr(let_node->value);
+      let = let_node->body;
+    }
+    VisitExpr(let);
   }
 
   void VisitPattern(const Pattern& p) final { PatternVisitor::VisitPattern(p); }

I also run into stack overflow in DeDup() and ConstantFold(), but I'm not sure whether they are similar issue.

[zhiics]

We probably need to port the passes to the non-recursive form.

[icemelon]

Thanks @lixiaoquan. I've applied your patch. For other passes, I agree with @zhiics that we should rewrite them to non-recursive form.

[jroesch]

@MarisaKirisame can you check this out.

[MarisaKirisame]

@icemelon9 @lixiaoquan I am missing context. what is this patch trying to solve?

[icemelon]

@MarisaKirisame This is to fix too many recursive visits into the let list that causes the stack overflow.
@tqchen It's not easy to add a test case for this PR unless we want to create a large graph that will cause stack overflow before this PR.

[MarisaKirisame]

can you use unique_ptr?