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

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added inline comments.


================
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:
> 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.
I did it in another way: to avoid passing `AllocatorTy` as a template argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111954