[Openmp-commits] [PATCH] D81054: [OpenMP] Introduce target memory manager

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim added inline comments.


================
Comment at: openmp/libomptarget/src/MemoryManager.cpp:88
+
+MemoryManagerTy::~MemoryManagerTy() {
+  // TODO: There is a little issue that target plugin is destroyed before this
----------------
JonChesterfield wrote:
> This "little issue" of calling into the target plugin after it has been destroyed is a contender for this patch not working on amdgpu.
> 
> I still think the target plugin, if it wishes to use this allocator, should hold the state itself. That means the allocator can be used internally, e.g. for call frames or the parallel region malloc, as well making destruction order straightforward and correct.
@tianshilei1992 Any plan to fix this? 
This does not only break for AMD, but also for a plugin our group is working on.

Without understanding all the details, I think, the destructor of DeviceTy should delete the MemoryManager?
Would this solve the issue? I.e. is the DeviceTy destroyed before the target plugin is unloaded?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81054