adstraw commented on a change in pull request #9525:
URL: https://github.com/apache/tvm/pull/9525#discussion_r751651309



##########
File path: src/runtime/hexagon/hexagon/hexagon_buffer.cc
##########
@@ -30,67 +34,135 @@ namespace tvm {
 namespace runtime {
 namespace hexagon {
 
-static size_t GetDataAlignment(const DLDataType dtype) {
-  size_t align = (dtype.bits / 8) * dtype.lanes;
-  if (align < kAllocAlignment) return kAllocAlignment;
-  return align;
-}
+// TODO: Alignment not used
+struct Allocation {
+  Allocation(size_t nbytes, size_t alignment) : nbytes_(nbytes), 
alignment_(alignment) {}
+  virtual ~Allocation() {}
+  Allocation(const Allocation&) = delete;
+  Allocation& operator=(const Allocation&) = delete;
+  Allocation(Allocation&&) = delete;
+  Allocation& operator=(Allocation&&) = delete;
 
-HexagonBuffer::HexagonBuffer(int ndim, const int64_t* shape, DLDataType dtype,
-                             Optional<String> scope) {
-  ICHECK_LE(ndim, 1) << "Hexagon currently only supports flat allocations "
-                     << "and arrays of flat allocations.";
+  void* data_{nullptr};
+  size_t nbytes_;
+  size_t alignment_;
+};
 
-  size_t alignment = GetDataAlignment(dtype);
-  // TODO(csullivan): Extend to support arrays of allocations.
-  // Move assignment from r-value constructed flat allocation.
-  *this = HexagonBuffer(shape[0] * (dtype.bits / 8) * dtype.lanes, alignment, 
scope);
-}
+struct DDRAllocation : public Allocation {
+  DDRAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes, 
alignment) {
+    data_ = malloc(nbytes);
+  }
+  ~DDRAllocation() {
+    free(data_);
+  }
+};
 
-HexagonBuffer::HexagonBuffer(size_t nbytes, size_t alignment, Optional<String> 
scope) {
-  void* ptr = nullptr;
-  int ret = posix_memalign(&ptr, alignment, nbytes);
-  if (ret != 0) {
-    throw std::bad_alloc();
+struct VTCMAllocation : public Allocation {
+  VTCMAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes, 
alignment) {
+  #ifdef BUILD_FOR_HEXAGON
+    compute_res_attr_t res_info;
+    HEXAGON_SAFE_CALL(HAP_compute_res_attr_init(&res_info));
+    // TODO: Magic number 1

Review comment:
       Left some TODOs in the Hexagon specific code that need to be flushed out 
once we have integration testing running.  Testing is on CPU to date.

##########
File path: src/runtime/hexagon/hexagon/hexagon_buffer.cc
##########
@@ -21,6 +21,10 @@
 
 #include <tvm/runtime/module.h>
 
+#ifdef BUILD_FOR_HEXAGON

Review comment:
       Had to stub this out for testing on CPU.  Not sure if this is the 
correct top level cmake switch to #ifdef around.

##########
File path: src/runtime/hexagon/hexagon/hexagon_buffer.cc
##########
@@ -30,67 +34,135 @@ namespace tvm {
 namespace runtime {
 namespace hexagon {
 
-static size_t GetDataAlignment(const DLDataType dtype) {
-  size_t align = (dtype.bits / 8) * dtype.lanes;
-  if (align < kAllocAlignment) return kAllocAlignment;
-  return align;
-}
+// TODO: Alignment not used

Review comment:
       Need to think this out.  Alignment is provided but never used.

##########
File path: src/runtime/hexagon/hexagon/hexagon_buffer.cc
##########
@@ -30,67 +34,135 @@ namespace tvm {
 namespace runtime {
 namespace hexagon {
 
-static size_t GetDataAlignment(const DLDataType dtype) {
-  size_t align = (dtype.bits / 8) * dtype.lanes;
-  if (align < kAllocAlignment) return kAllocAlignment;
-  return align;
-}
+// TODO: Alignment not used
+struct Allocation {
+  Allocation(size_t nbytes, size_t alignment) : nbytes_(nbytes), 
alignment_(alignment) {}
+  virtual ~Allocation() {}
+  Allocation(const Allocation&) = delete;
+  Allocation& operator=(const Allocation&) = delete;
+  Allocation(Allocation&&) = delete;
+  Allocation& operator=(Allocation&&) = delete;
 
-HexagonBuffer::HexagonBuffer(int ndim, const int64_t* shape, DLDataType dtype,
-                             Optional<String> scope) {
-  ICHECK_LE(ndim, 1) << "Hexagon currently only supports flat allocations "
-                     << "and arrays of flat allocations.";
+  void* data_{nullptr};
+  size_t nbytes_;
+  size_t alignment_;
+};
 
-  size_t alignment = GetDataAlignment(dtype);
-  // TODO(csullivan): Extend to support arrays of allocations.
-  // Move assignment from r-value constructed flat allocation.
-  *this = HexagonBuffer(shape[0] * (dtype.bits / 8) * dtype.lanes, alignment, 
scope);
-}
+struct DDRAllocation : public Allocation {
+  DDRAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes, 
alignment) {
+    data_ = malloc(nbytes);
+  }
+  ~DDRAllocation() {
+    free(data_);
+  }
+};
 
-HexagonBuffer::HexagonBuffer(size_t nbytes, size_t alignment, Optional<String> 
scope) {
-  void* ptr = nullptr;
-  int ret = posix_memalign(&ptr, alignment, nbytes);
-  if (ret != 0) {
-    throw std::bad_alloc();
+struct VTCMAllocation : public Allocation {
+  VTCMAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes, 
alignment) {
+  #ifdef BUILD_FOR_HEXAGON
+    compute_res_attr_t res_info;
+    HEXAGON_SAFE_CALL(HAP_compute_res_attr_init(&res_info));
+    // TODO: Magic number 1
+    HEXAGON_SAFE_CALL(HAP_compute_res_attr_set_vtcm_param(&res_info, nbytes, 
1));
+    // TODO: HEXAGON_SAFE_CALL?
+    // TODO: Magic number 10000
+    context_id_ = HAP_compute_res_acquire(&res_info, 10000);
+    if (context_id_) {
+      // TODO: HEXAGON_SAFE_CALL?
+      data_ = HAP_compute_res_attr_get_vtcm_ptr(&res_info);
+      if (!data_) {
+        HEXAGON_PRINT(ERROR, "ERROR: Allocated VTCM ptr is null.");
+        HEXAGON_SAFE_CALL(HAP_compute_res_release(context_id_));
+        return;
+      }
+    } else {
+      HEXAGON_PRINT(ERROR, "ERROR: Unable to acquire requeisted resource.");
+      return;
+    }
+    // HEXAGON_PRINT(ALWAYS, "VTCMAllocation() - Context ID: %u, VTCM ptr: 
%p", context_id_, data_);
+  #else
+    data_ = malloc(nbytes);
+  #endif
   }
-  allocations_.push_back(ptr);
-  SetStorageScope(scope);
+  ~VTCMAllocation() {
+  #ifdef BUILD_FOR_HEXAGON
+    // HEXAGON_PRINT(ALWAYS, "~VTCMAllocation() - Context ID: %u, VTCM ptr: 
%p", context_id_, data_);
+    // TODO: Need to handle the else case(s) here
+    if (context_id_ && data_) {
+      HEXAGON_SAFE_CALL(HAP_compute_res_release(context_id_));
+      data_ = nullptr;
+    }
+  #else
+    free(data_);
+  #endif
+  }
+  #ifdef BUILD_FOR_HEXAGON
+  unsigned int context_id_{0};
+  #endif
+};
+
+template <HexagonBuffer::StorageScope S>
+std::unique_ptr<Allocation> Allocator(size_t nbytes, size_t alignment);
+
+template <>
+std::unique_ptr<Allocation> 
Allocator<HexagonBuffer::StorageScope::kDDR>(size_t nbytes,
+                                                                         
size_t alignment) {
+  return std::make_unique<DDRAllocation>(nbytes, alignment);;
 }
 
-HexagonBuffer::HexagonBuffer(void* data, Optional<String> scope) : 
managed_{false} {
+template <>
+std::unique_ptr<Allocation> 
Allocator<HexagonBuffer::StorageScope::kVTCM>(size_t nbytes,
+                                                                          
size_t alignment) {
+  return std::make_unique<VTCMAllocation>(nbytes, alignment);
+}
+
+HexagonBuffer::HexagonBuffer(size_t nbytes, size_t alignment, Optional<String> 
scope) : ndim_(1), nbytes_(nbytes) {
   SetStorageScope(scope);
-  allocations_.push_back(data);
+
+  std::unique_ptr<Allocation> alloca = nullptr;
+  if (GetStorageScope() == StorageScope::kDDR) {
+    alloca = Allocator<StorageScope::kDDR>(nbytes, alignment);
+  }
+  else if (GetStorageScope() == StorageScope::kVTCM) {
+    alloca = Allocator<StorageScope::kVTCM>(nbytes, alignment);
+  }
+  CHECK(alloca != nullptr);
+  allocations_.push_back(alloca->data_);
+  managed_allocations_.push_back(std::move(alloca));
 }
 
-HexagonBuffer::~HexagonBuffer() {
-  if (managed_) {
-    for (auto& ptr : allocations_) {
-      free(ptr);
+HexagonBuffer::HexagonBuffer(size_t ndim, size_t nbytes, size_t alignment, 
Optional<String> scope) : ndim_(ndim), nbytes_(ndim * nbytes) {
+  SetStorageScope(scope);
+  for (size_t i = 0; i < ndim; ++i) {
+    std::unique_ptr<Allocation> alloca = nullptr;
+    if (GetStorageScope() == StorageScope::kDDR) {
+      alloca = Allocator<StorageScope::kDDR>(nbytes, alignment);
+    }
+    else if (GetStorageScope() == StorageScope::kVTCM) {
+      alloca = Allocator<StorageScope::kVTCM>(nbytes, alignment);
     }
+    CHECK(alloca != nullptr);
+    allocations_.push_back(alloca->data_);
+    managed_allocations_.push_back(std::move(alloca));
   }
 }
 
-HexagonBuffer::HexagonBuffer(HexagonBuffer&& other)
-    : allocations_(other.allocations_),
-      managed_(other.managed_),
-      storage_scope_(other.storage_scope_) {
-  other.allocations_.clear();
-  other.managed_ = false;
-  other.storage_scope_ = StorageScope::kDDR;
+// TODO: add ExternalAllocation class?

Review comment:
       The separate tracking / management of `allocations_` and 
`managed_allocations_` is bothersome and could be solved by an 
`ExternalAllocation` class similar to what is done with VTCM / DDR.  It may be 
that `ExternalAllocation` is a separate `DDRAllocation` where the data pointer 
is provided rather than allocated given that the `Allocator` is templated based 
on storage scope.

##########
File path: src/runtime/hexagon/hexagon/hexagon_buffer.cc
##########
@@ -30,67 +34,135 @@ namespace tvm {
 namespace runtime {
 namespace hexagon {
 
-static size_t GetDataAlignment(const DLDataType dtype) {
-  size_t align = (dtype.bits / 8) * dtype.lanes;
-  if (align < kAllocAlignment) return kAllocAlignment;
-  return align;
-}
+// TODO: Alignment not used
+struct Allocation {
+  Allocation(size_t nbytes, size_t alignment) : nbytes_(nbytes), 
alignment_(alignment) {}
+  virtual ~Allocation() {}
+  Allocation(const Allocation&) = delete;
+  Allocation& operator=(const Allocation&) = delete;
+  Allocation(Allocation&&) = delete;
+  Allocation& operator=(Allocation&&) = delete;
 
-HexagonBuffer::HexagonBuffer(int ndim, const int64_t* shape, DLDataType dtype,
-                             Optional<String> scope) {
-  ICHECK_LE(ndim, 1) << "Hexagon currently only supports flat allocations "
-                     << "and arrays of flat allocations.";
+  void* data_{nullptr};
+  size_t nbytes_;
+  size_t alignment_;
+};
 
-  size_t alignment = GetDataAlignment(dtype);
-  // TODO(csullivan): Extend to support arrays of allocations.
-  // Move assignment from r-value constructed flat allocation.
-  *this = HexagonBuffer(shape[0] * (dtype.bits / 8) * dtype.lanes, alignment, 
scope);
-}
+struct DDRAllocation : public Allocation {
+  DDRAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes, 
alignment) {
+    data_ = malloc(nbytes);
+  }
+  ~DDRAllocation() {
+    free(data_);
+  }
+};
 
-HexagonBuffer::HexagonBuffer(size_t nbytes, size_t alignment, Optional<String> 
scope) {
-  void* ptr = nullptr;
-  int ret = posix_memalign(&ptr, alignment, nbytes);
-  if (ret != 0) {
-    throw std::bad_alloc();
+struct VTCMAllocation : public Allocation {
+  VTCMAllocation(size_t nbytes, size_t alignment) : Allocation(nbytes, 
alignment) {
+  #ifdef BUILD_FOR_HEXAGON
+    compute_res_attr_t res_info;
+    HEXAGON_SAFE_CALL(HAP_compute_res_attr_init(&res_info));
+    // TODO: Magic number 1
+    HEXAGON_SAFE_CALL(HAP_compute_res_attr_set_vtcm_param(&res_info, nbytes, 
1));
+    // TODO: HEXAGON_SAFE_CALL?
+    // TODO: Magic number 10000
+    context_id_ = HAP_compute_res_acquire(&res_info, 10000);
+    if (context_id_) {
+      // TODO: HEXAGON_SAFE_CALL?
+      data_ = HAP_compute_res_attr_get_vtcm_ptr(&res_info);
+      if (!data_) {
+        HEXAGON_PRINT(ERROR, "ERROR: Allocated VTCM ptr is null.");
+        HEXAGON_SAFE_CALL(HAP_compute_res_release(context_id_));
+        return;
+      }
+    } else {
+      HEXAGON_PRINT(ERROR, "ERROR: Unable to acquire requeisted resource.");
+      return;
+    }
+    // HEXAGON_PRINT(ALWAYS, "VTCMAllocation() - Context ID: %u, VTCM ptr: 
%p", context_id_, data_);
+  #else
+    data_ = malloc(nbytes);
+  #endif
   }
-  allocations_.push_back(ptr);
-  SetStorageScope(scope);
+  ~VTCMAllocation() {
+  #ifdef BUILD_FOR_HEXAGON
+    // HEXAGON_PRINT(ALWAYS, "~VTCMAllocation() - Context ID: %u, VTCM ptr: 
%p", context_id_, data_);
+    // TODO: Need to handle the else case(s) here
+    if (context_id_ && data_) {
+      HEXAGON_SAFE_CALL(HAP_compute_res_release(context_id_));
+      data_ = nullptr;
+    }
+  #else
+    free(data_);
+  #endif
+  }
+  #ifdef BUILD_FOR_HEXAGON
+  unsigned int context_id_{0};
+  #endif
+};
+
+template <HexagonBuffer::StorageScope S>
+std::unique_ptr<Allocation> Allocator(size_t nbytes, size_t alignment);
+
+template <>
+std::unique_ptr<Allocation> 
Allocator<HexagonBuffer::StorageScope::kDDR>(size_t nbytes,
+                                                                         
size_t alignment) {
+  return std::make_unique<DDRAllocation>(nbytes, alignment);;
 }
 
-HexagonBuffer::HexagonBuffer(void* data, Optional<String> scope) : 
managed_{false} {
+template <>
+std::unique_ptr<Allocation> 
Allocator<HexagonBuffer::StorageScope::kVTCM>(size_t nbytes,
+                                                                          
size_t alignment) {
+  return std::make_unique<VTCMAllocation>(nbytes, alignment);
+}
+
+HexagonBuffer::HexagonBuffer(size_t nbytes, size_t alignment, Optional<String> 
scope) : ndim_(1), nbytes_(nbytes) {
   SetStorageScope(scope);
-  allocations_.push_back(data);
+
+  std::unique_ptr<Allocation> alloca = nullptr;
+  if (GetStorageScope() == StorageScope::kDDR) {
+    alloca = Allocator<StorageScope::kDDR>(nbytes, alignment);
+  }
+  else if (GetStorageScope() == StorageScope::kVTCM) {
+    alloca = Allocator<StorageScope::kVTCM>(nbytes, alignment);
+  }
+  CHECK(alloca != nullptr);
+  allocations_.push_back(alloca->data_);
+  managed_allocations_.push_back(std::move(alloca));
 }
 
-HexagonBuffer::~HexagonBuffer() {
-  if (managed_) {
-    for (auto& ptr : allocations_) {
-      free(ptr);
+HexagonBuffer::HexagonBuffer(size_t ndim, size_t nbytes, size_t alignment, 
Optional<String> scope) : ndim_(ndim), nbytes_(ndim * nbytes) {
+  SetStorageScope(scope);
+  for (size_t i = 0; i < ndim; ++i) {
+    std::unique_ptr<Allocation> alloca = nullptr;
+    if (GetStorageScope() == StorageScope::kDDR) {
+      alloca = Allocator<StorageScope::kDDR>(nbytes, alignment);
+    }
+    else if (GetStorageScope() == StorageScope::kVTCM) {
+      alloca = Allocator<StorageScope::kVTCM>(nbytes, alignment);
     }
+    CHECK(alloca != nullptr);
+    allocations_.push_back(alloca->data_);
+    managed_allocations_.push_back(std::move(alloca));
   }
 }
 
-HexagonBuffer::HexagonBuffer(HexagonBuffer&& other)
-    : allocations_(other.allocations_),
-      managed_(other.managed_),
-      storage_scope_(other.storage_scope_) {
-  other.allocations_.clear();
-  other.managed_ = false;
-  other.storage_scope_ = StorageScope::kDDR;
+// TODO: add ExternalAllocation class?
+HexagonBuffer::HexagonBuffer(void* data, size_t nbytes, Optional<String> 
scope) : ndim_(1), nbytes_(nbytes) {
+  // TODO: disallow VTCM scope?

Review comment:
       Do we expect external VTCM allocations?

##########
File path: src/runtime/hexagon/hexagon/hexagon_buffer.cc
##########
@@ -110,11 +182,77 @@ void HexagonBuffer::SetStorageScope(Optional<String> 
scope) {
   }
 }
 
-HexagonBuffer* IsHexagonBuffer(DLTensor* tensor) {
-  if (TVMDeviceExtType(tensor->device.device_type) == kDLHexagon) {
-    return static_cast<HexagonBuffer*>(tensor->data);
+void HexagonBuffer::CopyTo(void *data, size_t nbytes)
+{
+  CHECK(nbytes_ == nbytes);
+  size_t offset = 0;
+  for (size_t i = 0; i < ndim_; ++i) {
+    CHECK(nbytes / ndim_ == managed_allocations_[i]->nbytes_);
+
+    memcpy(static_cast<char*>(data) + offset,
+           static_cast<const char*>(managed_allocations_[i]->data_),  
+           managed_allocations_[i]->nbytes_);
+
+    offset += managed_allocations_[i]->nbytes_;
+  }
+}
+
+void HexagonBuffer::CopyFrom(void *data, size_t nbytes)
+{
+  CHECK(nbytes_ == nbytes);
+  size_t offset = 0;
+  for (size_t i = 0; i < ndim_; ++i) {
+    CHECK(nbytes / ndim_ == managed_allocations_[i]->nbytes_);
+
+    memcpy(static_cast<char*>(managed_allocations_[i]->data_), 
+           static_cast<const char*>(data) + offset, 
+           managed_allocations_[i]->nbytes_);
+
+    offset += managed_allocations_[i]->nbytes_;
+  }
+}
+
+void HexagonBuffer::CopyFrom(const HexagonBuffer& other)
+{
+  CHECK(nbytes_ == other.nbytes_);
+
+  if(ndim_ == other.ndim_) {
+    for (size_t i = 0; i < ndim_; ++i) {
+      CHECK(managed_allocations_[i]->nbytes_ == 
other.managed_allocations_[i]->nbytes_);
+
+      memcpy(static_cast<char*>(managed_allocations_[i]->data_),
+             static_cast<const char*>(other.managed_allocations_[i]->data_),
+             managed_allocations_[i]->nbytes_);
+    }
+  }
+  else if (ndim_ == 1) {
+    size_t offset = 0;
+    for (size_t i = 0; i < other.ndim_; ++i) {
+      CHECK(nbytes_ / other.ndim_ == other.managed_allocations_[i]->nbytes_);
+
+      memcpy(static_cast<char*>(managed_allocations_[0]->data_) + offset,
+             static_cast<const char*>(other.managed_allocations_[i]->data_),
+             other.managed_allocations_[i]->nbytes_);
+
+      offset += other.managed_allocations_[i]->nbytes_;
+    }
+  }
+  else if (other.ndim_ == 1) {
+    size_t offset = 0;
+    for (size_t i = 0; i < ndim_; ++i) {
+      CHECK(other.nbytes_ / ndim_ == managed_allocations_[i]->nbytes_);
+
+      memcpy(static_cast<char*>(managed_allocations_[i]->data_),
+             static_cast<const char*>(other.managed_allocations_[0]->data_) + 
offset,
+             managed_allocations_[i]->nbytes_);
+
+      offset += managed_allocations_[i]->nbytes_;
+    }
+  }
+  else {
+    // TODO: Add error string

Review comment:
       Will clean this up shortly.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to