[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)
> 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
CHANGES SINCE LAST ACTION
More information about the Openmp-commits