tqchen commented on a change in pull request #7084:
URL: https://github.com/apache/tvm/pull/7084#discussion_r544715878



##########
File path: src/tir/transforms/make_packed_api.cc
##########
@@ -41,6 +41,56 @@
 namespace tvm {
 namespace tir {
 
+class ReturnRewriter : public StmtMutator {
+ public:
+  explicit ReturnRewriter(Var ret_var, Var ret_tcode) : ret_var_(ret_var), 
ret_tcode_(ret_tcode) {}
+
+  Stmt VisitStmt_(const EvaluateNode* node) override {
+    Stmt ret = StmtMutator::VisitStmt_(node);
+    const EvaluateNode* eval = ret.as<EvaluateNode>();
+    CHECK(eval);

Review comment:
       I would agree for cases where the invariant was less clear an error 
message would be helpful. In this particular case the impl of StmtMutator was 
quite direct and invariant is clear, so adding error message won't buy us too 
much in this particular case. Additionally, having to have a long error message 
for all cases would increase the binary size.
   
   I think we should certainly deliberate error messages that we do hit. And 
prioritize on clear error messages on those cases. While still encourage 
deliberate add of ICHECKs to more places without requiring error message on the 
obvious cases.
   
   One way to do so is to re-actively add them when we hit the problem. This 
case is certainly fine




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


Reply via email to