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

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added inline comments.


================
Comment at: openmp/libomptarget/src/device.cpp:290
+      HostDataToTargetTy::LockGuard LG(*Entry);
+      if (Entry->addEventIfNecessary(*this, AsyncInfo) != OFFLOAD_SUCCESS)
+        return {{false /* IsNewEntry */, false /* IsHostPointer */},
----------------
jdoerfert wrote:
> ye-luo wrote:
> > 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.
> Right. 
> 
> So checking if we also do D2H would partially help but not completely. The stickiness of the event is still a problem. It might also be for the pure D2H case with multiple transfers as only the first is properly guarded.
> I think we need to:
> 1) only enter it here if we don't do D2H in the same directive (thus below).
> 2) remove events after we synchronized the AsyncInfo object it was recorded in.
Sorry I meant H2D not D2H. But seems you got my point. Both 1 and 2 are desired. I thought about 2 when the event was introduced but I didn't immediately figure out what is the best location to do 2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113119