tlopex commented on code in PR #20337:
URL: https://github.com/apache/tvm/pull/20337#discussion_r4028856426


##########
src/backend/metal/codegen/codegen_metal.cc:
##########
@@ -330,6 +331,9 @@ void CodeGenMetal::PrintStorageScope(const std::string& 
scope, std::ostream& os)
 }
 
 void CodeGenMetal::VisitStmt_(const BindNode* op) {
+  if (auto prim_value = op->value.as<PrimExpr>()) {
+    analyzer_->Bind(op->var, prim_value.value());
+  }

Review Comment:
   Please only bind pure expressions here, as in `stmt_simplify.cc`. Buffer 
loads are also `PrimExpr`: after `snapshot = state[0]` and an intervening 
store, the analyzer can incorrectly simplify `state[0] - snapshot` to zero. I 
reproduced both Metal and WebGPU allocating only one element while retaining a 
write to index 31. Guarding `Bind` with `SideEffect(value) <= 
CallEffectKind::kPure` would prevent this.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to