[Openmp-commits] [PATCH] D77005: [OpenMP] Optimized stream selection by scheduling data mapping for the same target region into a same stream

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:909
+  // If there is no any information, just return
+  if (!async_info->Identifier) {
+    return OFFLOAD_SUCCESS;
----------------
jdoerfert wrote:
> tianshilei1992 wrote:
> > lildmh wrote:
> > > It seems no synchronization by default, not aligned with the original behavior
> > The assumption here is: if this function is called at the end of `target` function, `Identifier` must be a valid pointer because anyhow during kernel launch it is assigned with a valid `CUstream`.
> It seems wrong to call this with `Identifier = nullptr`, doesn't it? If so we should make it an assert.
Yeah, seems right. "If there is nothing to be synchronized, what do users expect to synchronize?" Yes, should do an assert here.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:913
+
+  auto Stream = reinterpret_cast<CUstream>(async_info->Identifier);
+  CUresult Err = cuStreamSynchronize(Stream);
----------------
jdoerfert wrote:
> Why reinterpret_Cast? Is a CUstream a pointer? If not, why do we move objects around?
Yep.
```
typedef struct CUstream_st *CUstream;                     /**< CUDA stream */
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77005