[Openmp-commits] [PATCH] D77412: [OpenMP] Introduce stream pool to make sure the correctness of device synchronization

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

I like this a lot. I have one high-level question and a request for documentation:
Shouldn't we hide the stream stuff behind the `async_info` object instead?
I mean, we could avoid accidental misuse by restricting the getStream/returnStream to the constructor/destructor of async_info for example.
We should also unify the naming.

Maybe, put the entire "stream" (=queue) management in a class. All the initialization happens there, all the tear down as well, and when you make it friends with `async_info` we can restrict access too.

WDYT?



================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:226
+      StreamPool[DeviceId][--Id] = Stream;
+    }
   }
----------------
This is clever. So clever in fact that we need a comment above `StreamPool` (or similar) describing how the different values interact and how the `StreamPool` evolves. Basically how it works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77412