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 2020/10/25 04:54:35 UTC
[GitHub] [incubator-tvm] spectrometerHBH opened a new pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
spectrometerHBH opened a new pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753
cc @tqchen
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] spectrometerHBH commented on pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
spectrometerHBH commented on pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753#issuecomment-716248618
Take another look please, thanks! @tqchen
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] spectrometerHBH commented on pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
spectrometerHBH commented on pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753#issuecomment-716146008
cc @tqchen
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753#discussion_r511619874
##########
File path: src/arith/iter_affine_map.cc
##########
@@ -351,31 +367,45 @@ class IterMapRewriter : public ExprMutator {
std::vector<IterSplitExpr> iters;
iters.reserve(expr->args.size());
// canonicalize the expression
+ // find the base scale first
+ IntImm base_scale(nullptr);
Review comment:
Use Optional<IntImm> to better express our intent that base_scale is not defined in the begining. This can make things better for future cases when we make IntImm not nullable
Alternatively, we can make base_index int64, and use base_index == -1 to determine its value
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753#discussion_r511593103
##########
File path: src/arith/iter_affine_map.cc
##########
@@ -351,8 +374,18 @@ class IterMapRewriter : public ExprMutator {
std::vector<IterSplitExpr> iters;
iters.reserve(expr->args.size());
// canonicalize the expression
+ // find the base scale first
+ PrimExpr base_scale(nullptr);
+ for (const auto& arg : expr->args) {
+ if (is_const_int(arg->scale)) {
+ if (!base_scale.defined() || (CanProveLess(arg->scale, base_scale))) {
Review comment:
because it is constant, we don't have to do this, just use args->scale.as<IntImmNode> and dig up the values will do
##########
File path: src/arith/iter_affine_map.cc
##########
@@ -351,8 +374,18 @@ class IterMapRewriter : public ExprMutator {
std::vector<IterSplitExpr> iters;
iters.reserve(expr->args.size());
// canonicalize the expression
+ // find the base scale first
+ PrimExpr base_scale(nullptr);
+ for (const auto& arg : expr->args) {
Review comment:
We can save one iteration of the for loop below after we find the base scale.
Let us simply remember the index and find the base_index. in args. start with base_index == -1
Then mark the that index as visited. So the next for loop can start with i = 1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753#issuecomment-716511833
Thanks @spectrometerHBH @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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753#discussion_r511619874
##########
File path: src/arith/iter_affine_map.cc
##########
@@ -351,31 +367,45 @@ class IterMapRewriter : public ExprMutator {
std::vector<IterSplitExpr> iters;
iters.reserve(expr->args.size());
// canonicalize the expression
+ // find the base scale first
+ IntImm base_scale(nullptr);
Review comment:
Use Optional<IntImm> to better express our intent that base_scale is not defined in the begining. This can make things better for future cases when we make IntImm not nullable
Alternatively, we can make base_index int64, and use base_index == -1 to determine that we have not yet find the solution
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen merged pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] spectrometerHBH commented on pull request #6753: [ARITH] iter_affine_map bug fix, stride generalize
Posted by GitBox <gi...@apache.org>.
spectrometerHBH commented on pull request #6753:
URL: https://github.com/apache/incubator-tvm/pull/6753#issuecomment-716512343
Thanks, @tqchen @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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org