Lunderberg commented on code in PR #12401:
URL: https://github.com/apache/tvm/pull/12401#discussion_r944462931


##########
src/tir/transforms/storage_rewrite.cc:
##########
@@ -1463,7 +1463,9 @@ class VectorTypeRewriter : public StmtExprMutator {
 
   Stmt VisitStmt_(const LetStmtNode* op) final {
     auto it = rewrite_map_.find(op->var.get());
-    if (it == rewrite_map_.end()) {
+    PrimExpr value = this->VisitExpr(op->value);
+    Stmt body = this->VisitStmt(op->body);
+    if (value.same_as(op->value) && body.same_as(op->body) && it == 
rewrite_map_.end()) {

Review Comment:
   Hmm, I'm not seeing where either `value` or `body` get returned.  Also, it 
looks like we'd have an error if the `body` gets modified, but `op->var` isn't 
in `rewrite_map_`, which would result in unprotected dereferencing of 
`it->second`.  Could we instead have something like the following, so that the 
var/value/body are all handled in a similar way?
   
   ```
   PrimExpr value = this->VisitExpr(op->value);
   Stmt body = this->VisitStmt(op->body);
   Var var = (it == rewrite_map_.end()) ? op-> var : it->second.new_buffer_var;
   if(var.same_as(op->var) && value.same_as(op->value) && 
body.same_as(op->body)) {
       return GetRef<Stmt>(op);
   } else {
       return LetStmt(var, value, body);
   }
   ```



-- 
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: commits-unsubscr...@tvm.apache.org

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

Reply via email to