[GitHub] [tvm] slyubomirsky commented on a diff in pull request #15509: [Unity] Implement relax.Function.bind_symbolic_vars

["slyubomirsky (via GitHub)" <gi...@apache.org>]

slyubomirsky commented on code in PR #15509:
URL: https://github.com/apache/tvm/pull/15509#discussion_r1293832517


##########
src/relax/utils.cc:
##########
@@ -61,6 +65,49 @@ class ExprBinder : public ExprMutator {
     }
   }
 
+  Expr VisitExpr_(const CallNode* op) final {
+    auto call_node = Downcast<Call>(ExprMutator::VisitExpr_(op));
+
+    // Special case for strided_slice
+    //
+    // The strided_slice operator currently stores the begins/ends in
+    // the CallNode::attrs.  Because the CallNode::attrs is only
+    // intended to store static information, any PrimExpr members in
+    // the attributes are not visited by `ExprMutator::VisitPrimExpr`.
+    // Therefore, these must be explicitly visited.
+    //
+    // When the strided_slice operator is updated to store begins/ends
+    // as a tuple of `relax::PrimValue` in the arguments, this special
+    // case can be removed.

Review Comment:
   I wonder if this is worth changing to avoid needing a special case here. Namely, we can use PrimExpr arguments for strided_slice (the reason it's implemented this way is probably because we didn't have PrimExpr at the time we added this op).



-- 
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-unsubscribe@tvm.apache.org

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