[Openmp-commits] [PATCH] D120142: [OpenMP][CUDA] Use one event pool per device

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:176
+protected:
   CUcontext Context;
+};
----------------
jdoerfert wrote:
> tianshilei1992 wrote:
> > I don't think it's good to put `CUcontext` in allocator. That template is only a reference for other specialization.
> It was but it is not anymore. It is a base class now. Can you elaborate why you think it's not good to put a CUcontext in here? After all, all the associated allocators do allocate things that are bound to a/this context.
Well, it's not about correctness or necessity. It's just different styles. Like I said, it's just a reference/interface. All other allocators can implement in their own way, including some resources that don't require context. But of course you could argue we have not encountered any resource that doesn't require context. That's true and that's why I think it's just about styles. I'd like to keep things more flexible, even currently I don't see any use.
This is not a blocker. I'm fine if you think it's the good way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120142