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/06/24 16:55:38 UTC
[GitHub] [incubator-tvm] merrymercy opened a new pull request #5917: Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
merrymercy opened a new pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917
Add `LegalizeInvalidAttach` in `Schedule::normalize` to legalize the compute_at location if the target iterator of compute_at was split or fused.
- Case 1: If the target of compute_at is split, we will move the compute_at location to the inner iterator.
- Case 2: If the target of compute_at is fused, we will move the compute_at location to the newly fused iterator.
Case 2 can only happen if the target of compute_at is the innermost operand of fuse operation.
Examples: The following two cases will crash the compiler before this fix.
```python
A = te.compute((10, 10), lambda i, j: 1.0, name='A')
B = te.compute((10, 10), lambda i, j: A[i][j], name='B')
# Case 1: Split an axis which is the target of a compute_at
s = te.create_schedule([B.op])
s[A].compute_at(s[B], B.op.axis[1])
s[B].split(B.op.axis[1], 2)
stmt = tvm.lower(s, [A, B], simple_mode=True)['main'].body
assert isinstance(stmt.body.body, tvm.tir.stmt.For)
# Case 2: Fuse an axis which is the target of a compute_at
s = te.create_schedule([B.op])
s[A].compute_at(s[B], B.op.axis[1])
s[B].fuse(B.op.axis[0], B.op.axis[1])
stmt = tvm.lower(s, [A, B], simple_mode=True)['main'].body
assert isinstance(stmt, tvm.tir.stmt.For)
```
----------------------------------------------------------------
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] merrymercy commented on a change in pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#discussion_r445980708
##########
File path: .gitignore
##########
@@ -196,6 +196,7 @@ tvm_t.*
.python_history
.pytest_cache
.local
+cmake-build-debug
Review comment:
It is produced by my Jetbrain Clion
----------------------------------------------------------------
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] merrymercy commented on a change in pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#discussion_r445980708
##########
File path: .gitignore
##########
@@ -196,6 +196,7 @@ tvm_t.*
.python_history
.pytest_cache
.local
+cmake-build-debug
Review comment:
It is produced by my Clion
----------------------------------------------------------------
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] merrymercy commented on pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#issuecomment-648945692
This is a support PR for #5883. cc @tqchen @comaniac @jcf94
----------------------------------------------------------------
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] comaniac commented on a change in pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#discussion_r445086427
##########
File path: src/te/schedule/schedule_dataflow_rewrite.cc
##########
@@ -614,10 +614,81 @@ void InjectInline(ScheduleNode* sch) {
}
}
+void LegalizeInvalidAttach(ScheduleNode* sch) {
+ // Legalize the compute_at location if the target iterator of compute_at is split or fused.
+ // Case 1: If the target of compute_at is split,
+ // we will move the compute_at location to the inner iterator.
+ // Case 2: If the target of compute_at is fused,
+ // we will move the compute_at location to the newly fused iterator.
+ // Note that case 2 can only happen if the target of compute_at
+ // is the innermost operand of fuse operation.
+
+ std::unordered_map<IterVar, IterVar> replace_map;
Review comment:
Add comments to explain what this map does.
##########
File path: src/te/schedule/schedule_dataflow_rewrite.cc
##########
@@ -614,10 +614,81 @@ void InjectInline(ScheduleNode* sch) {
}
}
+void LegalizeInvalidAttach(ScheduleNode* sch) {
+ // Legalize the compute_at location if the target iterator of compute_at is split or fused.
+ // Case 1: If the target of compute_at is split,
+ // we will move the compute_at location to the inner iterator.
+ // Case 2: If the target of compute_at is fused,
+ // we will move the compute_at location to the newly fused iterator.
+ // Note that case 2 can only happen if the target of compute_at
+ // is the innermost operand of fuse operation.
+
+ std::unordered_map<IterVar, IterVar> replace_map;
+
+ for (Stage stage : sch->stages) {
+ for (Stage s = stage; s.defined();) {
+ Stage spec = s.GetAttachSpec();
+ if (spec->attach_type != kScope) {
+ break;
+ }
+ bool start_attach = false;
+ IterVar attach_ivar = spec->attach_ivar;
+ s = spec->attach_stage;
+ CHECK(attach_ivar.defined());
+ CHECK(s.defined());
+
+ for (size_t i = s->leaf_iter_vars.size(); i != 0; --i) {
+ IterVar iv = s->leaf_iter_vars[i - 1];
+ if (!start_attach && iv.same_as(attach_ivar)) {
+ start_attach = true;
+ }
Review comment:
Break this loop when matching.
----------------------------------------------------------------
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 edited a comment on pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#issuecomment-649748029
cc @vinx13 @Hzfengsy @spectrometerHBH please also help to take a look, we can proceed to merge after reviews by a few more eyes
----------------------------------------------------------------
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 #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#issuecomment-649748029
cc @vinx13 @Hzfengsy @spectrometerHBH please also help to take a look, we can proceed to merge after a few more eyes
----------------------------------------------------------------
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] junrushao1994 commented on a change in pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#discussion_r445941711
##########
File path: .gitignore
##########
@@ -196,6 +196,7 @@ tvm_t.*
.python_history
.pytest_cache
.local
+cmake-build-debug
Review comment:
Maybe we don’t need this line
----------------------------------------------------------------
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] merrymercy merged pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
merrymercy merged pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917
----------------------------------------------------------------
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] merrymercy commented on pull request #5917: [TE] Add LegalizeInvalidAttach to legalize the compute_at location after split or fuse
Posted by GitBox <gi...@apache.org>.
merrymercy commented on pull request #5917:
URL: https://github.com/apache/incubator-tvm/pull/5917#issuecomment-649232886
@tqchen @comaniac comments are addressed
----------------------------------------------------------------
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