manupa-arm commented on a change in pull request #8856:
URL: https://github.com/apache/tvm/pull/8856#discussion_r696698801



##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -84,10 +84,7 @@ class AOTOnDemandAllocator : public ExprVisitor {
     AssignReturnSid(GetRef<Expr>(op));
   }
 
-  void VisitExpr_(const VarNode* op) final {
-    ExprVisitor::VisitExpr_(op);
-    AssignReturnSid(GetRef<Expr>(op));
-  }
+  void VisitExpr_(const VarNode* op) final { 
AssignReturnSid(GetRef<Expr>(op)); }

Review comment:
       The original impl seems to do further visits, why is this change here as 
part of changing the visitor?

##########
File path: src/relay/backend/aot_executor_codegen.cc
##########
@@ -437,7 +434,7 @@ class AOTExecutorCodegen : public ExprVisitor {
   void VisitExpr_(const OpNode* op) override {
     throw std::runtime_error("can not compile op in non-eta expanded form");
   }
-  void VisitExpr_(const GlobalVarNode* op) override { throw 
std::runtime_error(""); }
+  void VisitExpr_(const GlobalVarNode* op) override {}

Review comment:
       Here also,
   Given that we had an error (though its empty -- being not very useful), this 
seems to just stop the visitation. Any reason ?




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