Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hexagon] Add concept of DMA groups #14254

Merged
merged 14 commits into from
Mar 20, 2023
Merged

Conversation

adstraw
Copy link
Contributor

@adstraw adstraw commented Mar 9, 2023

Add the concept of DMA group to align Hexagon User DMA with existing async copy solutions e.g. nVidia. Allows for the grouping of one or more DMA copies into a group and tracking the group with "in flight" counts created by InjectSoftwarePipeline pass. For now, this allows for the removal of merge_async_commit_queue_scope which was a Hexagon specific workaround due to lack of support for grouping DMA copies. Later, in a future PR, this functionality may lead to the possibility of doing DMA copy on Hexagon over non-contiguous regions.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Mar 9, 2023

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 by @-ing them in a comment.

Generated by tvm-bot

Copy link
Contributor

@Lunderberg Lunderberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, just a collection of nitpicks.

include/tvm/tir/builtin.h Outdated Show resolved Hide resolved
include/tvm/tir/builtin.h Show resolved Hide resolved
include/tvm/tir/builtin.h Show resolved Hide resolved
@@ -233,6 +233,19 @@ TVM_REGISTER_GLOBAL("device_api.hexagon.dma_wait").set_body([](TVMArgs args, TVM
*rv = static_cast<int32_t>(0);
});

TVM_REGISTER_GLOBAL("device_api.hexagon.dma_start_group")
.set_body([](TVMArgs args, TVMRetValue* rv) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: When the input/output types are fixed, the .set_body_typed() method can be used to avoid needing manual argument wrangling.

.set_body_typed([](int queue_id) -> int32_t {
      return HexagonDeviceAPI::Global()->UserDMA()->StartGroup(queue_id);
    });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chose not to implement this for this iteration as it seems like we could / should redo the entire Hexagon Device API with this change.

src/runtime/hexagon/hexagon_user_dma.cc Outdated Show resolved Hide resolved
src/runtime/hexagon/hexagon_device_api.cc Show resolved Hide resolved
src/tir/transforms/lower_async_dma.cc Outdated Show resolved Hide resolved
src/tir/transforms/lower_async_dma.cc Outdated Show resolved Hide resolved
@Lunderberg Lunderberg merged commit c6c89c3 into apache:main Mar 20, 2023
@adstraw adstraw deleted the straw-hex-dma-groups branch March 21, 2023 17:23
LeiWang1999 added a commit to LeiWang1999/tvm that referenced this pull request Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants