[Openmp-commits] [PATCH] D111954: [OpenMP][Plugin] Introduce resouce manager

Ye Luo via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Sun Dec 26 16:15:31 PST 2021

ye-luo added inline comments.

Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:192
+/// `Ty` is the type of the resource. It should be copyable.
+/// `AllocatorTy` should provide two functions:
What does it mean a resource is copyable? Did you mean a resource handle is copyable?

Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:213
+    for (size_t I = CurSize; I < Size; ++I)
+      Resources.push_back(Allocator.allocate());
ye-luo wrote:
> tianshilei1992 wrote:
> > ye-luo wrote:
> > > Whe CUDA call fails. Allocator.allocate() returns nullptr and the code continues running. This doesn't make sense.
> > `ResourceManagerTy` doesn't know what is a valid return value. `Ty` can be pointer, object, etc. Of course, for CUDA, everything is pointer, but it doesn't sound right to have if (Ty == nullptr)`.
> Allocator.allocate(Ty&) should return  OFFLOAD_SUCCESS, OFFLOAD_FAIL.
> You have AllocatorTy which knows what Ty is. Indeed, Ty should be removed, it can be derived from AllocatorTy
If Ty can be accessed from AllocatorTy, we'd better avoid passing Ty as a template argument.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list