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 2022/04/06 13:41:37 UTC

[GitHub] [tvm] Lunderberg commented on a diff in pull request #10910: [Hexagon] Refactor to keep HexagonBuffer private to the device api

Lunderberg commented on code in PR #10910:
URL: https://github.com/apache/tvm/pull/10910#discussion_r843936368


##########
src/runtime/hexagon/hexagon/hexagon_common.h:
##########
@@ -47,14 +47,6 @@
 namespace tvm {

Review Comment:
   Can we remove the now empty namespace blocks?



##########
src/runtime/hexagon/hexagon/hexagon_device_api_v2.h:
##########
@@ -125,8 +126,23 @@ class HexagonDeviceAPIv2 final : public DeviceAPI {
                       TVMStreamHandle stream) final;
 
  private:
-  //! Lookup table for the HexagonBuffer managing a workspace allocation.
-  std::unordered_map<void*, HexagonBuffer*> workspace_allocations_;
+  /*! \brief Helper to allocate a HexagonBuffer and register the result
+   *  in the owned buffer map.
+   *  \return Raw data storage managed by the hexagon buffer
+   */
+  template <typename... Args>
+  void* AllocateHexagonBuffer(Args&&... args) {
+    auto buf = std::unique_ptr<HexagonBuffer>(new HexagonBuffer(std::forward<Args>(args)...));

Review Comment:
   Since TVM uses C++14, we can use `std::make_unique<HexagonBuffer>(std::forward<Args>(args)...)` instead of `new`.



##########
src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc:
##########
@@ -148,20 +139,29 @@ void HexagonDeviceAPIv2::CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamH
   CHECK_EQ(to->byte_offset, 0);
   CHECK_EQ(GetDataSize(*from), GetDataSize(*to));
 
-  HexagonBuffer* hex_from_buf = static_cast<HexagonBuffer*>(from->data);
-  HexagonBuffer* hex_to_buf = static_cast<HexagonBuffer*>(to->data);
+  auto lookup_hexagon_buffer = [this](void* ptr) -> HexagonBuffer* {
+    auto it = this->hexagon_buffer_map_.find(ptr);
+    if (it != this->hexagon_buffer_map_.end()) {
+      return it->second.get();
+    }
+    return nullptr;

Review Comment:
   Since the `nullptr` check is done after each lookup, let's move it in here as `CHECK(it != this->hexagon_buffer_map_.end())`.



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