You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/11/17 21:51:59 UTC

[GitHub] [tvm] csullivan commented on a change in pull request #9525: Add Hexagon VTCM support

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



##########
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);

Review comment:
       Use this for DDR allocation instead of malloc.

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

Review comment:
       Perhaps we pull the allocation classes into a separate file(s) and let VTCMAllocation inherit from DDRAllocation when not building for hexagon so that we exercise the DDR allocation  when running tests on x86 and don't have yet another way to allocate data.

##########
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);

Review comment:
       See my other comment on DDR allocations, let's use `posix_memalign` instead of `malloc`.




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