[Openmp-commits] [PATCH] D81054: [OpenMP] Introduce target memory manager

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added inline comments.


================
Comment at: openmp/libomptarget/src/MemoryManager.cpp:214
+      std::lock_guard<std::mutex> Guard(MapTableLock);
+      PtrToNodeTable.insert({TgtPtr, NodeTy(Size, TgtPtr)});
+    }
----------------
Use emplace and its return value iterator to avoid the later lookup(at).


================
Comment at: openmp/libomptarget/src/MemoryManager.cpp:234
+    std::lock_guard<std::mutex> G(MapTableLock);
+    std::unordered_map<void *, NodeTy>::iterator Itr =
+        PtrToNodeTable.find(TgtPtr);
----------------
I don't what the policy of using auto. auto makes the code cleaner.
There are a few similar places with iterators.


================
Comment at: openmp/libomptarget/src/device.cpp:419
+
   return RTL->data_delete(RTLDeviceID, TgtPtrBegin);
 }
----------------
Prefer
```
else
  return RTL->data_delete(RTLDeviceID, TgtPtrBegin);
```
the same change to RTL->data_alloc above


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81054/new/

https://reviews.llvm.org/D81054