-
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] Generalized HexagonBuffer::CopyTo/CopyFrom #10878
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change operates on the allocation regions in a `HexagonBuffer`, rather than referencing the managed allocation owned by a buffer, handling copies between two sets of possibly discontiguous regions. This will be necessary to handle discontiguous buffers that cannot be statically planned at compile-time, such as user-initiated allocations, within a shared memory pool. Contiguous regions of memory are recognized and result in a single DMA call.
Copy between 3-region buffers and 4-region buffers is now allowed.
csullivan
requested changes
Apr 4, 2022
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.
This is really great! One quick CR below, and unfortunately I'll have to finish the review tomorrow as I've run out of time today.
csullivan
reviewed
Apr 5, 2022
Lunderberg
force-pushed
the
hexagon_buffer_copy
branch
from
April 5, 2022 15:43
2eaca67
to
06d1f7b
Compare
This was referenced Apr 5, 2022
csullivan
approved these changes
Apr 5, 2022
Comment on lines
+261
to
276
void hexagon_buffer_copy_across_regions(const BufferSet& dest, const BufferSet& src, | ||
size_t bytes_to_copy) { | ||
// First, determine all copies that do not cross boundaries in | ||
// either source or destination region. | ||
auto micro_copies = BufferSet::MemoryCopies(dest, src, bytes_to_copy); | ||
|
||
// If regions are contiguously allocated, we can reduce the number | ||
// of copies required by merging adjacent copies. | ||
auto macro_copies = MemoryCopy::MergeAdjacent(std::move(micro_copies)); | ||
|
||
// Finally, do the memory copies. | ||
for (const auto& copy : macro_copies) { | ||
int error_code = hexagon_user_dma_1d_sync(copy.dest, copy.src, copy.num_bytes); | ||
CHECK_EQ(error_code, 0); | ||
} | ||
} |
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.
❤️
pfk-beta
pushed a commit
to pfk-beta/tvm
that referenced
this pull request
Apr 11, 2022
* [Hexagon] Generalized HexagonBuffer::CopyTo/CopyFrom This change operates on the allocation regions in a `HexagonBuffer`, rather than referencing the managed allocation owned by a buffer, handling copies between two sets of possibly discontiguous regions. This will be necessary to handle discontiguous buffers that cannot be statically planned at compile-time, such as user-initiated allocations, within a shared memory pool. Contiguous regions of memory are recognized and result in a single DMA call.
mehrdadh
pushed a commit
to mehrdadh/tvm
that referenced
this pull request
Apr 11, 2022
* [Hexagon] Generalized HexagonBuffer::CopyTo/CopyFrom This change operates on the allocation regions in a `HexagonBuffer`, rather than referencing the managed allocation owned by a buffer, handling copies between two sets of possibly discontiguous regions. This will be necessary to handle discontiguous buffers that cannot be statically planned at compile-time, such as user-initiated allocations, within a shared memory pool. Contiguous regions of memory are recognized and result in a single DMA call.
Lucien0
pushed a commit
to Lucien0/tvm
that referenced
this pull request
Apr 19, 2022
* [Hexagon] Generalized HexagonBuffer::CopyTo/CopyFrom This change operates on the allocation regions in a `HexagonBuffer`, rather than referencing the managed allocation owned by a buffer, handling copies between two sets of possibly discontiguous regions. This will be necessary to handle discontiguous buffers that cannot be statically planned at compile-time, such as user-initiated allocations, within a shared memory pool. Contiguous regions of memory are recognized and result in a single DMA call.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the
HexagonBuffer::CopyTo
andHexagonBuffer::CopyFrom
functions to operate on the allocation regions in aHexagonBuffer
, rather than referencing the managed allocation owned by a buffer, and to handle copies between two sets of possibly discontiguous regions. This will be necessary to handle discontiguous buffers that cannot be statically planned at compile-time, such as user-initiated allocations, within a shared memory pool.Contiguous regions of memory are recognized and result in a single DMA call.