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

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

Took me a while to get back to this. Way behind on certain things. Apologies.



================
Comment at: openmp/libomptarget/plugins/common/MemoryManager/MemoryManager.h:146-148
+  int deleteOnDevice(void *Ptr) const {
+    return DeviceAllocator.free(Ptr, /* AsyncInfo */ nullptr);
+  }
----------------
grokos wrote:
> We aren't we passing an `AsyncInfo` object as an extra argument to `deleteOnDevice` and subsequently call `DeviceAllocator.free()` with that `AsyncInfo` instead of `nullptr`? I suspect it's because we need to make room for subsequent allocations before attempting to re-allocate, if that's the case can you add a comment to make it clearer?
I'll add a comment to explain that this operation should not be delayed.


================
Comment at: openmp/libomptarget/plugins/common/MemoryManager/MemoryManager.h:293
   /// Deallocate memory pointed by \p TgtPtr
-  int free(void *TgtPtr) {
+  int free(void *TgtPtr, __tgt_async_info *AsyncInfo) {
     DP("MemoryManagerTy::free: target memory " DPxMOD ".\n", DPxPTR(TgtPtr));
----------------
grokos wrote:
> If there is a reason why `deleteOnDevice()` cannot be called with an extra `AsyncInfo` argument, we are we adding it here? It's not used anywhere in the body of `free()`.
I added it to keep the interface the same but I'll remove it now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113119