[GitHub] [tvm] rkimball commented on a change in pull request #7832: [Relay] Recursive destructor call replaced with non-recursive for Call nodes.

[GitBox <gi...@apache.org>]

rkimball commented on a change in pull request #7832:
URL: https://github.com/apache/tvm/pull/7832#discussion_r613458068



##########
File path: include/tvm/relay/expr.h
##########
@@ -625,6 +629,64 @@ class TempExpr : public Expr {
   TVM_DEFINE_OBJECT_REF_METHODS(TempExpr, RelayExpr, TempExprNode);
 };
 
+/*
+ * Non-recursive traversal with dismantling unused call nodes,
+ * a derivative from ExpandDataflow method
+ */
+inline void __dismantle(Expr expr) {

Review comment:
       names with double underscore in them are reserved in any scope in C++. Remove the underscores in the name.

##########
File path: include/tvm/relay/expr.h
##########
@@ -625,6 +629,64 @@ class TempExpr : public Expr {
   TVM_DEFINE_OBJECT_REF_METHODS(TempExpr, RelayExpr, TempExprNode);
 };
 
+/*
+ * Non-recursive traversal with dismantling unused call nodes,
+ * a derivative from ExpandDataflow method
+ */
+inline void __dismantle(Expr expr) {
+  std::unordered_set<const RelayExprNode*> visited;
+  std::stack<std::pair<Expr, bool>> stack;
+  auto fpush_to_stack = [&visited, &stack](const Expr& expr) {
+    if (expr.use_count() < 3 && !visited.count(expr.get())) {
+      stack.push({expr, false});
+    }
+  };
+  fpush_to_stack(expr);
+  while (stack.size() > 0) {
+    auto node = stack.top().first;
+
+    if (visited.count(node.get())) {
+      stack.pop();
+
+      // dismantle node
+      if (node.use_count() < 3) {
+        if (auto* op = const_cast<CallNode*>(node.as<CallNode>())) {
+          op->args = Array<Expr>();
+        }
+      }
+    } else if (stack.top().second) {
+      visited.insert(node.get());
+    } else {
+      stack.top().second = true;
+
+      // special handilg

Review comment:
       ```suggestion
         // special handling
   ```

##########
File path: include/tvm/relay/expr.h
##########
@@ -625,6 +629,64 @@ class TempExpr : public Expr {
   TVM_DEFINE_OBJECT_REF_METHODS(TempExpr, RelayExpr, TempExprNode);
 };
 
+/*
+ * Non-recursive traversal with dismantling unused call nodes,
+ * a derivative from ExpandDataflow method
+ */
+inline void __dismantle(Expr expr) {
+  std::unordered_set<const RelayExprNode*> visited;
+  std::stack<std::pair<Expr, bool>> stack;
+  auto fpush_to_stack = [&visited, &stack](const Expr& expr) {
+    if (expr.use_count() < 3 && !visited.count(expr.get())) {
+      stack.push({expr, false});
+    }
+  };
+  fpush_to_stack(expr);
+  while (stack.size() > 0) {
+    auto node = stack.top().first;
+
+    if (visited.count(node.get())) {
+      stack.pop();
+
+      // dismantle node
+      if (node.use_count() < 3) {

Review comment:
       Agreed. This needs a comment

##########
File path: include/tvm/relay/expr.h
##########
@@ -625,6 +629,64 @@ class TempExpr : public Expr {
   TVM_DEFINE_OBJECT_REF_METHODS(TempExpr, RelayExpr, TempExprNode);
 };
 
+/*
+ * Non-recursive traversal with dismantling unused call nodes,
+ * a derivative from ExpandDataflow method
+ */
+inline void __dismantle(Expr expr) {

Review comment:
       Can you move this large function to expr.cc? Too big here.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org