[Openmp-commits] [PATCH] D87269: [OpenMP] Introduce GOMP taskwait depend in the runtime

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim added a comment.

The GOMP wrapper makes sense to me. 
Several inline comments for the test.



================
Comment at: openmp/runtime/test/tasking/omp50_taskwait_depend.c:14
+int b = 0;
+volatile int flag = 0;
+
----------------
since this is an OpenMP program, shouldn't we use OpenMP atomic store and load?


================
Comment at: openmp/runtime/test/tasking/omp50_taskwait_depend.c:25
+        // to execute before taskwait depend()
+        #pragma omp task shared(a)
+        {
----------------
How can we be sure, that not all threads get occupied by these tasks, while none of the other tasks are scheduled?


================
Comment at: openmp/runtime/test/tasking/omp50_taskwait_depend.c:50
+
+      #pragma omp taskwait depend(inout: a)
+      // The test will loop forever if the taskwait is a normal
----------------
The test succeeds with clang, when I replace this line with
```
#pragma omp task if(0) depend(inout: a)
{}
```
Can you please add this copy of the test?


================
Comment at: openmp/runtime/test/tasking/omp50_taskwait_depend.c:59
+      }
+      if (b) {
+        fprintf(stderr, "tasks were signaled prematurely\n");
----------------
b should be loaded with omp atomic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87269