[Openmp-commits] [PATCH] D113119: [OpenMP] Introduce asynchronous malloc and free

Ye Luo via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu Jan 20 10:18:06 PST 2022

ye-luo requested changes to this revision.
ye-luo added inline comments.
This revision now requires changes to proceed.

Comment at: openmp/libomptarget/include/device.h:305
+  /// Flag to indicate if the device supports asynchronous frees.
+  bool SupportsAsyncFree;
What about async malloc?

Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:321
+static CUstream getStream(const int DeviceId, __tgt_async_info *AsyncInfo,
+                          const StreamPoolVectorTy &StreamPoolVector) {
pass StreamPoolTy& as an argument and there is no need of DeviceId

Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:371
-    CUDADeviceAllocatorTy(int DeviceId, std::vector<DeviceDataTy> &DeviceData)
-        : DeviceId(DeviceId), DeviceData(DeviceData) {}
+    CUDADeviceAllocatorTy(int DeviceId, std::vector<DeviceDataTy> &DeviceData,
+                          StreamPoolVectorTy &StreamPoolVector)
This class is one object per device. I would expect removing DeviceId replace vectors with a single element.

Comment at: openmp/libomptarget/src/device.cpp:290
+      HostDataToTargetTy::LockGuard LG(*Entry);
+      if (Entry->addEventIfNecessary(*this, AsyncInfo) != OFFLOAD_SUCCESS)
+        return {{false /* IsNewEntry */, false /* IsHostPointer */},
The current Event is intended for D2H, you expanded its usage. In case of one record after alloc and another record after the transfer. I think this will go wrong. Once the event is fulfilled after alloc. other thread may think the transfer has been completed.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list