[Openmp-commits] [PATCH] D82718: [OpenMP] Use primary context in CUDA plugin

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim added a comment.

My comment was just an observation based on the code modification. I have not much understanding of the implications of this flag other than the few lines I read in the documentation and the comparison @tianshilei1992 posted.

The intention of your latest change makes sense to me, I just think that there could be more flags active (see inline comment)

I leave it to others to judge whether this is the desired behavior. As an alternative, you could create a new context in case the primary context uses a different scheduling policy.



================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:428
+      DP("The primary context is active, no change to its flags\n");
+      if (FormerPrimaryCtxFlags != CU_CTX_SCHED_BLOCKING_SYNC)
+        DP("Warning the current flags are not CU_CTX_SCHED_BLOCKING_SYNC\n");
----------------
I think, it should be
```
if (! (FormerPrimaryCtxFlags & CU_CTX_SCHED_BLOCKING_SYNC))

```
or
```
if ((FormerPrimaryCtxFlags & CU_CTX_SCHED_MASK) != CU_CTX_SCHED_BLOCKING_SYNC)

```
They might be equivalent, if only one scheduling policy can be set at a time.


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

https://reviews.llvm.org/D82718