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 2021/08/26 11:08:50 UTC
[GitHub] [tvm] Hzfengsy opened a new pull request #8855: [TIR] Fix opaque access in buffer locator pass and match_buffer in region detector
Hzfengsy opened a new pull request #8855:
URL: https://github.com/apache/tvm/pull/8855
In this PR, I have fixed two bugs:
1. Incorrectly detect access region for opaque access in `PlanAndUpdateBufferAllocationLocation`
Currently, the pass `PlanAndUpdateBufferAllocationLocation` will re-calculate the access region. It will enlarge the access region even though with manually specified access regions. Actually, the only thing we should do is erase the buffers allocated inside the block.
2. Incorrectly analyze the buffer access region created by `match_buffer`.
`block_access_region_detector` will only detector buffers in the given map. As for the buffer created by `match_buffer`, we SHOULD only consider the buffers whose source buffer is in the given map. But we detect all buffers created by `match_buffer` by mistake. Also, the previous testcase is incorrect, so we can pass it even the logic is wrong.
Thanks @MasterJH5574 for reporting this error.
--
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] tqchen commented on pull request #8855: [TIR] Fix opaque access in buffer locator pass and match_buffer in region detector
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8855:
URL: https://github.com/apache/tvm/pull/8855#issuecomment-906450513
also cc @zxybazh @shingjan
--
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] Hzfengsy commented on pull request #8855: [TIR] Fix opaque access in buffer locator pass and match_buffer in region detector
Posted by GitBox <gi...@apache.org>.
Hzfengsy commented on pull request #8855:
URL: https://github.com/apache/tvm/pull/8855#issuecomment-906310265
cc @junrushao1994. It would be great to have a look at it.
--
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] MasterJH5574 commented on a change in pull request #8855: [TIR] Fix opaque access in buffer locator pass and match_buffer in region detector
Posted by GitBox <gi...@apache.org>.
MasterJH5574 commented on a change in pull request #8855:
URL: https://github.com/apache/tvm/pull/8855#discussion_r696813532
##########
File path: src/tir/transforms/plan_update_buffer_allocation_location.cc
##########
@@ -85,11 +83,22 @@ class BufferAllocationLocator : public StmtExprMutator {
buffer_data_to_buffer_.Set(buf->data, buf);
}
}
+ for (const MatchBufferRegion match_buffer : op->match_buffers) {
+ const Var& target_var = match_buffer->buffer->data;
+ const Var& source_var = match_buffer->source->buffer->data;
+ ICHECK(buffer_data_to_buffer_.count(source_var));
+ buffer_data_to_buffer_.Set(target_var, match_buffer->buffer);
+ }
Stmt stmt = StmtMutator::VisitStmt_(op);
op = stmt.as<BlockNode>();
ICHECK(op != nullptr);
- // Ignore buffer allocated inside the block when getting access region.
+ // Ignore buffer created by match_buffer inside the block when updating access region.
Review comment:
Using "we no longer consider" might be a better expression. WDYT?
```suggestion
// No longer consider buffers created by match_buffer inside the block when updating access region.
```
##########
File path: src/tir/transforms/plan_update_buffer_allocation_location.cc
##########
@@ -85,11 +83,22 @@ class BufferAllocationLocator : public StmtExprMutator {
buffer_data_to_buffer_.Set(buf->data, buf);
}
}
+ for (const MatchBufferRegion match_buffer : op->match_buffers) {
+ const Var& target_var = match_buffer->buffer->data;
+ const Var& source_var = match_buffer->source->buffer->data;
+ ICHECK(buffer_data_to_buffer_.count(source_var));
+ buffer_data_to_buffer_.Set(target_var, match_buffer->buffer);
+ }
Stmt stmt = StmtMutator::VisitStmt_(op);
op = stmt.as<BlockNode>();
ICHECK(op != nullptr);
- // Ignore buffer allocated inside the block when getting access region.
+ // Ignore buffer created by match_buffer inside the block when updating access region.
+ for (const MatchBufferRegion match_buffer : op->match_buffers) {
+ const Var& target_var = match_buffer->buffer->data;
+ buffer_data_to_buffer_.erase(target_var);
+ }
+ // Ignore buffer allocated inside the block when updating access region.
Review comment:
Ditto.
```suggestion
// No longer consider buffers allocated inside the block when updating access region.
```
##########
File path: src/tir/transforms/plan_update_buffer_allocation_location.cc
##########
@@ -127,8 +133,18 @@ class BufferAllocationLocator : public StmtExprMutator {
return std::move(realize);
}
+ Array<BufferRegion> UpdateRegion(const Array<BufferRegion>& region) const {
Review comment:
It would be greater to name the function in a more informative way. After all, what we do is removing stuffs instead of updating stuffs. What about `RemoveRedundantBufferRegion`? Although I admit that it is quite long...
--
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] tqchen edited a comment on pull request #8855: [TIR] Fix opaque access in buffer locator pass and match_buffer in region detector
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #8855:
URL: https://github.com/apache/tvm/pull/8855#issuecomment-906450513
also cc @zxybazh @shingjan @vinx13
--
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] shingjan commented on a change in pull request #8855: [TIR] Fix opaque access in buffer locator pass and match_buffer in region detector
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8855:
URL: https://github.com/apache/tvm/pull/8855#discussion_r696944448
##########
File path: src/tir/transforms/plan_update_buffer_allocation_location.cc
##########
@@ -127,8 +133,18 @@ class BufferAllocationLocator : public StmtExprMutator {
return std::move(realize);
}
+ Array<BufferRegion> UpdateRegion(const Array<BufferRegion>& region) const {
Review comment:
That name makes sense to me.
--
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] junrushao1994 merged pull request #8855: [TIR] Fix opaque access in buffer locator pass and match_buffer in region detector
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #8855:
URL: https://github.com/apache/tvm/pull/8855
--
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