adstraw commented on a change in pull request #9525:
URL: https://github.com/apache/tvm/pull/9525#discussion_r752440627
##########
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:
I like the idea of having VTCMAllocation inherit from DDRAllocation. I
can look into doing that. However, the current implementation achieves the
same functionality by simply doing
```
#ifdef BUILD_FOR_HEXAGON
VTCM_allcoc
#else
malloc
#endif
```
I agree that using inheritance is a cleaner solution.
NOTE: I looked into moving the `Allocation` classes to separate file but
this is complicated by the fact that the `Allocator` is templated on
`HexagonBuffer` storage scope enum. I.e. there is a cross dependency. It
still can be done but will also need to move the enum to hexagon_common.h
probably.
--
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]
