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/11/03 13:31:23 UTC
[GitHub] [incubator-tvm] mbaret opened a new pull request #6823: [TIR] Make loop unrolling in LoopPartition optional
mbaret opened a new pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823
For certain analysis/tensorization, it can be useful to keep the loop structure when partitioning loops. The current behaviour removes For loops of length 1. This change introduces the option to preserve these loops with the 'unroll' flag.
----------------------------------------------------------------
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] ANSHUMAN87 commented on a change in pull request #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#discussion_r517112153
##########
File path: src/tir/transforms/loop_partition.cc
##########
@@ -40,9 +40,11 @@ namespace tir {
struct LoopPartitionConfigNode : public tvm::AttrsNode<LoopPartitionConfigNode> {
bool partition_const_loop;
+ bool unroll;
Review comment:
If we want a config point here. I think it is good to start with optional config, not mandatory config.
I think it is good, if keep option like no_unroll_single_iter_loop = False
##########
File path: src/tir/transforms/loop_partition.cc
##########
@@ -402,6 +404,7 @@ class LoopPartitioner : public StmtMutator {
std::unordered_map<const VarNode*, IntSet> relax_map_;
arith::Analyzer analyzer_;
CandidateSelector selector;
+ bool unroll_;
Review comment:
Please change the name as per last comment.
----------------------------------------------------------------
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] mbaret commented on pull request #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
mbaret commented on pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#issuecomment-721266418
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] giuseros commented on a change in pull request #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
giuseros commented on a change in pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#discussion_r516980104
##########
File path: tests/python/unittest/test_tir_transform_loop_partition.py
##########
@@ -66,6 +66,29 @@ def test_const_loop():
assert not any(collect_visit(stmt, lambda x: isinstance(x, tvm.tir.IfThenElse)))
+def test_no_unroll_loop():
+ n = 21
+ A = te.placeholder((n,), name="A")
+ B = te.placeholder((n,), name="B")
+
+ T = te.compute((n,), lambda i: A[i] + B[i])
+ s = te.create_schedule(T.op)
+ xo, xi = s[T].split(T.op.axis[0], factor=4)
+
+ bounds = tvm.te.schedule.InferBound(s)
+ stmt = tvm.te.schedule.ScheduleOps(s, bounds)
+
+ mod = tvm.IRModule.from_expr(tvm.tir.PrimFunc([], stmt))
+ with tvm.transform.PassContext(
+ config={"tir.LoopPartition": {"partition_const_loop": True, "unroll": False}}
Review comment:
Could we be more specific than "unroll"? Something like "simplify_loops"? With "unroll" it seems like we unroll or not all the loops
----------------------------------------------------------------
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] ANSHUMAN87 commented on pull request #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#issuecomment-721531822
Also cc @MarisaKirisame !
----------------------------------------------------------------
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 #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823
----------------------------------------------------------------
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 #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#discussion_r518745251
##########
File path: src/tir/transforms/loop_partition.cc
##########
@@ -40,9 +40,13 @@ namespace tir {
struct LoopPartitionConfigNode : public tvm::AttrsNode<LoopPartitionConfigNode> {
bool partition_const_loop;
+ bool no_unroll_single_iter_loop;
Review comment:
how about `no_unroll_loop_with_extent_one`
----------------------------------------------------------------
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 #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#discussion_r518745251
##########
File path: src/tir/transforms/loop_partition.cc
##########
@@ -40,9 +40,13 @@ namespace tir {
struct LoopPartitionConfigNode : public tvm::AttrsNode<LoopPartitionConfigNode> {
bool partition_const_loop;
+ bool no_unroll_single_iter_loop;
Review comment:
how about `unroll_loop_with_extent_one` and default to true?
----------------------------------------------------------------
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] ANSHUMAN87 commented on pull request #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#issuecomment-723120249
> In terms of why I need this, I want to do some pattern matching on loops to determine if they correspond to a particular operation. This is made a lot more consistent if loops of extent 1 aren't dropped from the IR.
Thanks @mbaret , i got your point and concur in case of pattern matching. Also I totally support the idea of having more control over optimization techniques :)
----------------------------------------------------------------
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] mbaret commented on pull request #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
mbaret commented on pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#issuecomment-723076881
In terms of why I need this, I want to do some pattern matching on loops to determine if they correspond to a particular operation. This is made a lot more consistent if loops of extent 1 aren't dropped from the IR.
----------------------------------------------------------------
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 #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#issuecomment-723078886
Thanks @mbaret I understand where you are coming from. Ideally we would also like to develop tools to be able to detect patterns that also removes these trivial loops. But we can tackle that later
----------------------------------------------------------------
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 #6823: [TIR] Make loop unrolling in LoopPartition optional
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6823:
URL: https://github.com/apache/incubator-tvm/pull/6823#discussion_r518745251
##########
File path: src/tir/transforms/loop_partition.cc
##########
@@ -40,9 +40,13 @@ namespace tir {
struct LoopPartitionConfigNode : public tvm::AttrsNode<LoopPartitionConfigNode> {
bool partition_const_loop;
+ bool no_unroll_single_iter_loop;
Review comment:
how about `no_unroll_loop_with_extent_one` , since single iter can be ambiguious
##########
File path: src/tir/transforms/loop_partition.cc
##########
@@ -40,9 +40,13 @@ namespace tir {
struct LoopPartitionConfigNode : public tvm::AttrsNode<LoopPartitionConfigNode> {
bool partition_const_loop;
+ bool no_unroll_single_iter_loop;
Review comment:
how about `no_unroll_loop_with_extent_one` , since single iter can be ambiguous
----------------------------------------------------------------
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