[Openmp-commits] [PATCH] D74145: [OpenMP][Offloading] Added support for multiple streams so that multiple kernels can be executed concurrently

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

Are cuda streams available on all versions of cuda that the rest of openmp works from? I'm not sure when they were introduced.

A couple of minor comments inline. This seems to be a fairly straightforward wrapper over the cuda functionality.



================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:95
   std::vector<std::list<FuncOrGblEntryTy>> FuncGblEntries;
+  std::vector<std::unique_ptr<std::atomic_int>> NextStreamId;
 
----------------
tianshilei1992 wrote:
> jdoerfert wrote:
> > Make it `uint` please.
> Right, in case of integer overflow, my bad...
vector of pointers to atomic_int is interesting. What's the advantage over vector<atomic_int>?

It might be worth putting a few asserts in the code to the effect that resizing the vector after the initial construction will break access from other threads.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:259
+    for (std::unique_ptr<std::atomic_int> &Ptr : NextStreamId) {
+      Ptr = std::unique_ptr<std::atomic_int>(new std::atomic_int(0));
+    }
----------------
If we do need the pointer wrapper, this should be make_unique


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

https://reviews.llvm.org/D74145