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

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:118
+      DP("Error when setting current CUDA context\n");
+      CUDA_ERR_STRING(err);
+      // We will return if cannot switch to the right context in case of
----------------
jdoerfert wrote:
> Outline this into a helper. Also the error checking should be a helper.
> 
> So something like:
> ```
> /// ....
> bool checkResult(CUResult err, const char *ErrMsg) {
>   if (err == CUDA_SUCCESS)
>     return true;
>    DP(ErrMsg);
>    CUDA_ERR_STRING(err);
>    return false;
> }
> 
> /// ...
> bool switchToDevice(DeviceId) {
>   CUresult err = cuCtxSetCurrent((*ContextsPtr)[DeviceId]);
>   return checkResult(err, "Error when creating CUDA stream to resize stream pool\n");
> }
> ```
> 
> and then use it eveywhere. Should cut down the size and duplication.
We can do the first one. As for the second function, it must be the member function. Let's do that in next patch which moves every CUDA related things into class.


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

https://reviews.llvm.org/D77412