You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/07/05 22:55:50 UTC

[GitHub] [tvm] ganler opened a new pull request, #12013: [BugFix] IRSubstitution should keep Var type consistency

ganler opened a new pull request, #12013:
URL: https://github.com/apache/tvm/pull/12013

   Very similar to https://github.com/apache/tvm/pull/11582
   
   Can be triggered by pool2d->binary op with integer 64 in attributes (the attached test case fail on GPU before this fix). cc: @junrushao1994 


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


[GitHub] [tvm] ganler commented on a diff in pull request #12013: [BugFix] IRSubstitution should keep Var type consistency

Posted by GitBox <gi...@apache.org>.
ganler commented on code in PR #12013:
URL: https://github.com/apache/tvm/pull/12013#discussion_r914383061


##########
src/tir/ir/stmt_functor.cc:
##########
@@ -640,10 +641,44 @@ Stmt IRTransform(Stmt ir_node, const runtime::PackedFunc& f_preorder,
   return transform(std::move(ir_node));
 }
 
+#define DEFINE_BIOP_EXPR_PROMOTABLE_MUTATE_(OP)                                           \
+  PrimExpr VisitExpr_(const OP##Node* op) override {                                      \
+    PrimExpr a = this->VisitExpr(op->a);                                                  \
+    PrimExpr b = this->VisitExpr(op->b);                                                  \
+    if (a.same_as(op->a) && b.same_as(op->b)) {                                           \
+      return GetRef<PrimExpr>(op);                                                        \
+    } else {                                                                              \
+      if (a.dtype().code() == b.dtype().code() && a.dtype().bits() != b.dtype().bits()) { \

Review Comment:
   IR substituion will eventually fail if `a.dtype() != b.dtype()` which is required by binary operators at construction time (`OP(a, b)`). By promoting the operand types automatically when their base dtype (e.g., int or float) is the same can pass the test case (which is a valid model and failed previously due to `int32` in pool2d's topi conflicts `int64` in GPU attributes.



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


[GitHub] [tvm] kparzysz-quic commented on pull request #12013: [BugFix] IRSubstitution should keep Var type consistency

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12013:
URL: https://github.com/apache/tvm/pull/12013#issuecomment-1178394640

   I actually have a patch that fixes your problem without changing substitution, I can create a PR tomorrow.


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


[GitHub] [tvm] ganler closed pull request #12013: [BugFix] IRSubstitution should keep Var type consistency

Posted by GitBox <gi...@apache.org>.
ganler closed pull request #12013: [BugFix] IRSubstitution should keep Var type consistency
URL: https://github.com/apache/tvm/pull/12013


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


[GitHub] [tvm] kparzysz-quic commented on pull request #12013: [BugFix] IRSubstitution should keep Var type consistency

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12013:
URL: https://github.com/apache/tvm/pull/12013#issuecomment-1178307732

   Substituting an expression of one type for a variable of a different type is an error.  We should find places where this happens and fix it there.


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


[GitHub] [tvm] ganler commented on pull request #12013: [BugFix] IRSubstitution should keep Var type consistency

Posted by GitBox <gi...@apache.org>.
ganler commented on PR #12013:
URL: https://github.com/apache/tvm/pull/12013#issuecomment-1178413151

   Yeah, I think @kparzysz-quic is right. We should look for the root cause. Looking forward to the PR!


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