[Openmp-commits] [PATCH] D84996: [OpenMP] Fixed the issue that target memory deallocation might be called when they're being used

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added a comment.

Thanks for fixing the bug. It should be good for the moment.
When I think about the existence of recursive mapper, we may still have more sync than needed. I think recursion the whole targetDataBegin/targetDataEnd is convenient but sub-optimal choice.
Recursion should only be done on the map/mapper analysis. Just leave my thoughts here. It needs a discussion beyond this patch.



================
Comment at: openmp/libomptarget/src/omptarget.cpp:599
+  // We need to synchronize before deallocating data
+  Ret = Device.synchronize(AsyncInfo);
+  if (Ret != OFFLOAD_SUCCESS) {
----------------
If targetDataEnd is not called from target.
AsyncInfo can be nullptr.
AsyncInfo->Queue also be nullptr in "target exit data nowait" if there is no transfer.

At the end of target(), the synchronize needs to check AsyncInfo->Queue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84996