[Openmp-commits] [PATCH] D111954: [OpenMP][Plugin] Introduce resouce manager

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:196
+/// - `void AllocatorTy::deallocate(Ty)`: deallocate the resource.
+template <typename Ty, typename AllocatorTy> class ResourceManagerTy {
+  /// Index of the next available resource.
----------------
Too many managers. How about ResourcePoolTy


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:270
+
+  CUstream allocate() noexcept {
+    if (!checkResult(cuCtxSetCurrent(DeviceData.Context),
----------------
allocate and deallocate are really meant for pointers. How about create/destroy for single object.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:346
 
   std::unique_ptr<StreamManagerTy> StreamManager;
   std::vector<DeviceDataTy> DeviceData;
----------------
I think we can avoid StreamManagerTy abstraction and have
std::vector<std::unique_ptr<RMTy>> StreamPools;
directly here.

DeviceData, Modules, DeviceAllocators, MemoryManagers don't need additional layer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111954