[GitHub] [tvm] csullivan commented on a diff in pull request #10878: [Hexagon] Generalized HexagonBuffer::CopyTo/CopyFrom

[GitBox <gi...@apache.org>]

csullivan commented on code in PR #10878:
URL: https://github.com/apache/tvm/pull/10878#discussion_r842215421


##########
src/runtime/hexagon/hexagon/hexagon_buffer.cc:
##########
@@ -205,73 +205,116 @@ void HexagonBuffer::SetStorageScope(Optional<String> scope) {
   }
 }
 
-void HexagonBuffer::CopyTo(void* data, size_t nbytes) const {
-  CHECK_LE(nbytes, TotalBytes());
-  CHECK(managed_allocations_.size() && "CopyTo not supported on unmanaged `external` allocations");
+struct BufferSet {
+  BufferSet(void* const* buffers, size_t num_regions, size_t region_size_bytes)
+      : buffers(buffers), num_regions(num_regions), region_size_bytes(region_size_bytes) {}
+
+  size_t TotalBytes() const { return num_regions * region_size_bytes; }
+
+  void* const* buffers;
+  size_t num_regions;
+  size_t region_size_bytes;
+};
+
+struct MemoryCopy {
+  MemoryCopy(void* dest, void* src, size_t num_bytes)
+      : dest(dest), src(src), num_bytes(num_bytes) {}
 
-  size_t copied = 0;
-  for (const auto& managed_alloc : managed_allocations_) {
-    size_t bytes_to_copy = std::min(nbytes - copied, managed_alloc->allocation_nbytes_);
-    if (bytes_to_copy == 0) break;
+  bool IsDirectlyBefore(const MemoryCopy& other) {
+    void* src_end = static_cast<unsigned char*>(src);
+    void* dest_end = static_cast<unsigned char*>(dest);

Review Comment:
   Missing `num_bytes` offset here to do the pointer arith for src_end and dest_end.



##########
src/runtime/hexagon/hexagon/hexagon_buffer.cc:
##########
@@ -205,73 +205,116 @@ void HexagonBuffer::SetStorageScope(Optional<String> scope) {
   }
 }
 
-void HexagonBuffer::CopyTo(void* data, size_t nbytes) const {
-  CHECK_LE(nbytes, TotalBytes());
-  CHECK(managed_allocations_.size() && "CopyTo not supported on unmanaged `external` allocations");
+struct BufferSet {
+  BufferSet(void* const* buffers, size_t num_regions, size_t region_size_bytes)
+      : buffers(buffers), num_regions(num_regions), region_size_bytes(region_size_bytes) {}
+
+  size_t TotalBytes() const { return num_regions * region_size_bytes; }
+
+  void* const* buffers;
+  size_t num_regions;
+  size_t region_size_bytes;
+};
+
+struct MemoryCopy {
+  MemoryCopy(void* dest, void* src, size_t num_bytes)
+      : dest(dest), src(src), num_bytes(num_bytes) {}
 
-  size_t copied = 0;
-  for (const auto& managed_alloc : managed_allocations_) {
-    size_t bytes_to_copy = std::min(nbytes - copied, managed_alloc->allocation_nbytes_);
-    if (bytes_to_copy == 0) break;
+  bool IsDirectlyBefore(const MemoryCopy& other) {
+    void* src_end = static_cast<unsigned char*>(src);
+    void* dest_end = static_cast<unsigned char*>(dest);

Review Comment:
   Probably we should have a test that covers the coalescing of the memcpy's that would trigger this.



-- 
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