-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Conversation
Co-authored-by: Noah Verke <[email protected]> Co-authored-by: Eric Lunderberg <[email protected]>
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 |
There was a problem hiding this 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.
@@ -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) { |
There was a problem hiding this comment.
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);
});
There was a problem hiding this comment.
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.
This reverts commit c6c89c3.
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.