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 2022/12/14 18:19:26 UTC
[GitHub] [tvm] nverke opened a new pull request, #13613: Add check for non-contiguous memory access when lowering to async dma…
nverke opened a new pull request, #13613:
URL: https://github.com/apache/tvm/pull/13613
… copies.
Currently we do not support non contiguous memory copies for async dma so this adds a check to error out in the case that either load or store is non contiguous. Unfortunately if we do not throw an error in this case then we get inaccurate outputs. Added a test to verify that we are indeed throwing an error in this case that should be removed once support is added.
--
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 #13613: Add check for non-contiguous memory access when lowering to async dma…
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13613:
URL: https://github.com/apache/tvm/pull/13613#issuecomment-1351903647
<!---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-->
* No users to auto-tag found, no teams are specified in PR title <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] adstraw commented on a diff in pull request #13613: Add check for non-contiguous memory access when lowering to async dma…
Posted by GitBox <gi...@apache.org>.
adstraw commented on code in PR #13613:
URL: https://github.com/apache/tvm/pull/13613#discussion_r1049933426
##########
src/tir/transforms/lower_async_dma.cc:
##########
@@ -146,13 +153,32 @@ class AsyncDMALowerer : public StmtExprMutator {
// map loop variable to zero for the store index & simplify
Array<PrimExpr> store_index = bufferstorenode->indices;
+
+ // Use DetectIterMap to detect whether store index is non-contiguous.
+ arith::Analyzer analyzer;
+ auto store_iter_map = DetectIterMap(store_index, input_iters, 1, arith::IterMapLevel::NoCheck,
+ &analyzer, false);
+ if (!store_iter_map->errors.empty()) {
+ LOG(FATAL) << "Unable to lower async dma for non contiguous memory access with index: "
+ << store_index;
+ }
+
store_index.MutateByApply([&](PrimExpr expr) {
arith::Analyzer analyzer;
return analyzer.Simplify(Substitute(std::move(expr), loop_var_remap));
});
// map loop variable to zero for the load index & simplify
Array<PrimExpr> load_index = bufferloadnode->indices;
+
+ // Use DetectIterMap to detect whether load index is non-contiguous.
+ auto load_iter_map =
+ DetectIterMap(load_index, input_iters, 1, arith::IterMapLevel::NoCheck, &analyzer, false);
+ if (!load_iter_map->errors.empty()) {
+ LOG(FATAL) << "Unable to lower async dma for non contiguous memory access with index: "
Review Comment:
Nit: Same as above. Add text indicating "buffer load" contained non contiguous access.
##########
src/tir/transforms/lower_async_dma.cc:
##########
@@ -146,13 +153,32 @@ class AsyncDMALowerer : public StmtExprMutator {
// map loop variable to zero for the store index & simplify
Array<PrimExpr> store_index = bufferstorenode->indices;
+
+ // Use DetectIterMap to detect whether store index is non-contiguous.
+ arith::Analyzer analyzer;
+ auto store_iter_map = DetectIterMap(store_index, input_iters, 1, arith::IterMapLevel::NoCheck,
+ &analyzer, false);
+ if (!store_iter_map->errors.empty()) {
+ LOG(FATAL) << "Unable to lower async dma for non contiguous memory access with index: "
Review Comment:
Nit: You might differentiate the error message by including something text indicating that the "buffer store" contained the non contiguous access.
--
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 #13613: Add check for non-contiguous memory access when lowering to async dma…
Posted by GitBox <gi...@apache.org>.
masahi merged PR #13613:
URL: https://github.com/apache/tvm/pull/13613
--
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] nverke commented on a diff in pull request #13613: Add check for non-contiguous memory access when lowering to async dma…
Posted by GitBox <gi...@apache.org>.
nverke commented on code in PR #13613:
URL: https://github.com/apache/tvm/pull/13613#discussion_r1050165016
##########
src/tir/transforms/lower_async_dma.cc:
##########
@@ -146,13 +153,32 @@ class AsyncDMALowerer : public StmtExprMutator {
// map loop variable to zero for the store index & simplify
Array<PrimExpr> store_index = bufferstorenode->indices;
+
+ // Use DetectIterMap to detect whether store index is non-contiguous.
+ arith::Analyzer analyzer;
+ auto store_iter_map = DetectIterMap(store_index, input_iters, 1, arith::IterMapLevel::NoCheck,
+ &analyzer, false);
+ if (!store_iter_map->errors.empty()) {
+ LOG(FATAL) << "Unable to lower async dma for non contiguous memory access with index: "
Review Comment:
Added!
##########
src/tir/transforms/lower_async_dma.cc:
##########
@@ -146,13 +153,32 @@ class AsyncDMALowerer : public StmtExprMutator {
// map loop variable to zero for the store index & simplify
Array<PrimExpr> store_index = bufferstorenode->indices;
+
+ // Use DetectIterMap to detect whether store index is non-contiguous.
+ arith::Analyzer analyzer;
+ auto store_iter_map = DetectIterMap(store_index, input_iters, 1, arith::IterMapLevel::NoCheck,
+ &analyzer, false);
+ if (!store_iter_map->errors.empty()) {
+ LOG(FATAL) << "Unable to lower async dma for non contiguous memory access with index: "
+ << store_index;
+ }
+
store_index.MutateByApply([&](PrimExpr expr) {
arith::Analyzer analyzer;
return analyzer.Simplify(Substitute(std::move(expr), loop_var_remap));
});
// map loop variable to zero for the load index & simplify
Array<PrimExpr> load_index = bufferloadnode->indices;
+
+ // Use DetectIterMap to detect whether load index is non-contiguous.
+ auto load_iter_map =
+ DetectIterMap(load_index, input_iters, 1, arith::IterMapLevel::NoCheck, &analyzer, false);
+ if (!load_iter_map->errors.empty()) {
+ LOG(FATAL) << "Unable to lower async dma for non contiguous memory access with index: "
Review Comment:
Added!
--
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] nverke commented on pull request #13613: Add check for non-contiguous memory access when lowering to async dma…
Posted by GitBox <gi...@apache.org>.
nverke commented on PR #13613:
URL: https://github.com/apache/tvm/pull/13613#issuecomment-1353742695
We are still discussing the pros and cons of the potential options but the possible solutions are.
1. Lower the copy into n x 1D dma copies.
2. Lower to 2D dma copies.
3. Use tensorize to make synchronous dma copies then in the lower_async_dma pass add the necessary asynchronous pieces.
--
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 pull request #13613: Add check for non-contiguous memory access when lowering to async dma…
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #13613:
URL: https://github.com/apache/tvm/pull/13613#issuecomment-1353734805
> once support is added.
What support is being planed? 2D DMA or breaking up into contiguous copies?
--
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