adstraw commented on code in PR #12727:
URL: https://github.com/apache/tvm/pull/12727#discussion_r968868937


##########
src/runtime/hexagon/hexagon_device_api.h:
##########
@@ -45,11 +45,27 @@ class HexagonDeviceAPI final : public DeviceAPI {
   static HexagonDeviceAPI* Global();
 
   //! \brief Constructor
-  HexagonDeviceAPI() {}
+  HexagonDeviceAPI() { hexbuffs = std::make_unique<HexagonBufferManager>(); }
 
   //! \brief Destructor
   ~HexagonDeviceAPI() {}
 
+  //! \brief Creates resource managers for the runtime
+  void AcquireResources() {
+    if (!hexbuffs->empty()) {
+      LOG(INFO) << "hexbuffs was not empty in AcquireResources";

Review Comment:
   Would it make more sense to put `hexbuffs = 
std::make_unique<HexagonBufferManager>();` in this function 
(`AcquireResources`) rather than in the constructor for the device API.  This 
way, we know that `hexbuffs` is empty at the time we run "acquire" - no need to 
check.  It would also alleviate the need to check whether `hexbuffs` is empty 
on "release" and we could simply `hexbuffs.reset()`.
   
   ```
   void AcquireResources() {
     CHECK_EQ(hexbuffs, nullptr);
     hexbuffs = std::make_unique<HexagonBufferManager>();
   }
   
   void ReleaseResources() {
     hexbuffs.reset();
     hexbuffs = nullptr;
   }
   ```
   
   The device api ctor could just set hexbuffs=nullptr.



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