You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "kparzysz-quic (via GitHub)" <gi...@apache.org> on 2023/06/15 22:13:08 UTC
[GitHub] [tvm] kparzysz-quic opened a new pull request, #15112: [Unity] Make RewriteDataflowReshape only rewrite volume-preserving ops
kparzysz-quic opened a new pull request, #15112:
URL: https://github.com/apache/tvm/pull/15112
The reshape operator expects that the number of elements in the source is the same as the number of elements in the result. There are operators that could have a reshape pattern that don't meet this requirement (e.g. strided_slice), and they should not be converted to reshape.
--
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] tvm-bot commented on pull request #15112: [Unity] Make RewriteDataflowReshape only rewrite volume-preserving ops
Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #15112:
URL: https://github.com/apache/tvm/pull/15112#issuecomment-1593790289
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @quic-sanirudh <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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] masahi merged pull request #15112: [Unity][Relax] Make RewriteDataflowReshape only rewrite volume-preserving ops
Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi merged PR #15112:
URL: https://github.com/apache/tvm/pull/15112
--
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 a diff in pull request #15112: [Unity] Make RewriteDataflowReshape only rewrite volume-preserving ops
Posted by "kparzysz-quic (via GitHub)" <gi...@apache.org>.
kparzysz-quic commented on code in PR #15112:
URL: https://github.com/apache/tvm/pull/15112#discussion_r1232195776
##########
src/relax/transform/rewrite_dataflow_reshape.cc:
##########
@@ -89,8 +95,32 @@ class DataflowReshapeRewriter : public ExprMutator {
auto arg = arg_tuple[used_arg_indices[0]];
- TensorStructInfo res_sinfo = Downcast<TensorStructInfo>(call->struct_info_);
- ICHECK(res_sinfo->shape.defined());
+ // The reshape operator expects that the number of elements in the source is the same
+ // as the number of elements in the result. There are operators that could have a reshape
+ // pattern that don't meet this requirement (e.g. strided_slice), and they should not be
+ // converted to reshape.
+ ICHECK(arg->struct_info_.defined() && call->struct_info_.defined());
+ TensorStructInfo arg_sinfo = Downcast<TensorStructInfo>(arg->struct_info_.value());
+ TensorStructInfo res_sinfo = Downcast<TensorStructInfo>(call->struct_info_.value());
+
+ if (arg_sinfo->IsUnknownDtype() || arg_sinfo->dtype != res_sinfo->dtype) {
+ return Unchanged;
+ }
+ ICHECK(arg_sinfo->shape.defined() && res_sinfo->shape.defined());
+ if (arg_sinfo->IsUnknownNdim() || res_sinfo->IsUnknownNdim()) {
+ return Unchanged;
+ }
+ auto product = [](Array<PrimExpr> args) -> PrimExpr {
+ ICHECK(!args.empty());
+ return std::reduce(args.begin(), args.end(), PrimExpr(1),
+ [](auto a, auto b) { return a * b; });
+ };
+ auto arg_count = product(arg_sinfo->GetShape().value());
+ auto res_count = product(res_sinfo->GetShape().value());
+ if (!arith::Analyzer().CanProveEqual(arg_count, res_count)) {
+ return Unchanged;
+ }
Review Comment:
This code needs the input arg, so I'll move the call to `IsCallingTIRReshape` to where the arg is known. I hope that's ok.
--
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] masahi commented on a diff in pull request #15112: [Unity] Make RewriteDataflowReshape only rewrite volume-preserving ops
Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi commented on code in PR #15112:
URL: https://github.com/apache/tvm/pull/15112#discussion_r1232101358
##########
src/relax/transform/rewrite_dataflow_reshape.cc:
##########
@@ -69,8 +73,10 @@ class DataflowReshapeRewriter : public ExprMutator {
}
Expr VisitExpr_(const CallNode* call) final {
+ auto Unchanged = GetRef<Call>(call);
Review Comment:
We don't use capital for var names.
##########
src/relax/transform/rewrite_dataflow_reshape.cc:
##########
@@ -89,8 +95,32 @@ class DataflowReshapeRewriter : public ExprMutator {
auto arg = arg_tuple[used_arg_indices[0]];
- TensorStructInfo res_sinfo = Downcast<TensorStructInfo>(call->struct_info_);
- ICHECK(res_sinfo->shape.defined());
+ // The reshape operator expects that the number of elements in the source is the same
+ // as the number of elements in the result. There are operators that could have a reshape
+ // pattern that don't meet this requirement (e.g. strided_slice), and they should not be
+ // converted to reshape.
+ ICHECK(arg->struct_info_.defined() && call->struct_info_.defined());
+ TensorStructInfo arg_sinfo = Downcast<TensorStructInfo>(arg->struct_info_.value());
+ TensorStructInfo res_sinfo = Downcast<TensorStructInfo>(call->struct_info_.value());
+
+ if (arg_sinfo->IsUnknownDtype() || arg_sinfo->dtype != res_sinfo->dtype) {
+ return Unchanged;
+ }
+ ICHECK(arg_sinfo->shape.defined() && res_sinfo->shape.defined());
+ if (arg_sinfo->IsUnknownNdim() || res_sinfo->IsUnknownNdim()) {
+ return Unchanged;
+ }
+ auto product = [](Array<PrimExpr> args) -> PrimExpr {
+ ICHECK(!args.empty());
+ return std::reduce(args.begin(), args.end(), PrimExpr(1),
+ [](auto a, auto b) { return a * b; });
+ };
+ auto arg_count = product(arg_sinfo->GetShape().value());
+ auto res_count = product(res_sinfo->GetShape().value());
+ if (!arith::Analyzer().CanProveEqual(arg_count, res_count)) {
+ return Unchanged;
+ }
Review Comment:
How about moving this logic to `IsCallingTIRReshape`?
--
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 a diff in pull request #15112: [Unity] Make RewriteDataflowReshape only rewrite volume-preserving ops
Posted by "kparzysz-quic (via GitHub)" <gi...@apache.org>.
kparzysz-quic commented on code in PR #15112:
URL: https://github.com/apache/tvm/pull/15112#discussion_r1232195776
##########
src/relax/transform/rewrite_dataflow_reshape.cc:
##########
@@ -89,8 +95,32 @@ class DataflowReshapeRewriter : public ExprMutator {
auto arg = arg_tuple[used_arg_indices[0]];
- TensorStructInfo res_sinfo = Downcast<TensorStructInfo>(call->struct_info_);
- ICHECK(res_sinfo->shape.defined());
+ // The reshape operator expects that the number of elements in the source is the same
+ // as the number of elements in the result. There are operators that could have a reshape
+ // pattern that don't meet this requirement (e.g. strided_slice), and they should not be
+ // converted to reshape.
+ ICHECK(arg->struct_info_.defined() && call->struct_info_.defined());
+ TensorStructInfo arg_sinfo = Downcast<TensorStructInfo>(arg->struct_info_.value());
+ TensorStructInfo res_sinfo = Downcast<TensorStructInfo>(call->struct_info_.value());
+
+ if (arg_sinfo->IsUnknownDtype() || arg_sinfo->dtype != res_sinfo->dtype) {
+ return Unchanged;
+ }
+ ICHECK(arg_sinfo->shape.defined() && res_sinfo->shape.defined());
+ if (arg_sinfo->IsUnknownNdim() || res_sinfo->IsUnknownNdim()) {
+ return Unchanged;
+ }
+ auto product = [](Array<PrimExpr> args) -> PrimExpr {
+ ICHECK(!args.empty());
+ return std::reduce(args.begin(), args.end(), PrimExpr(1),
+ [](auto a, auto b) { return a * b; });
+ };
+ auto arg_count = product(arg_sinfo->GetShape().value());
+ auto res_count = product(res_sinfo->GetShape().value());
+ if (!arith::Analyzer().CanProveEqual(arg_count, res_count)) {
+ return Unchanged;
+ }
Review Comment:
This code needs the input arg, so I'll move the call to `IsCallingTIRReshape` until the arg is known. I hope that's ok.
--
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 a diff in pull request #15112: [Unity] Make RewriteDataflowReshape only rewrite volume-preserving ops
Posted by "kparzysz-quic (via GitHub)" <gi...@apache.org>.
kparzysz-quic commented on code in PR #15112:
URL: https://github.com/apache/tvm/pull/15112#discussion_r1232193103
##########
src/relax/transform/rewrite_dataflow_reshape.cc:
##########
@@ -69,8 +73,10 @@ class DataflowReshapeRewriter : public ExprMutator {
}
Expr VisitExpr_(const CallNode* call) final {
+ auto Unchanged = GetRef<Call>(call);
Review Comment:
Ugh, my mistake. Done.
--
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