[Openmp-commits] [openmp] c2c4313 - [OpenMP] Fix bug 50022

[Shilei Tian via Openmp-commits]

Author: Shilei Tian
Date: 2021-07-23T16:54:11-04:00
New Revision: c2c43132f69c6f7e89d97c0c2ce32b0cf2f30e90

URL: https://github.com/llvm/llvm-project/commit/c2c43132f69c6f7e89d97c0c2ce32b0cf2f30e90
DIFF: https://github.com/llvm/llvm-project/commit/c2c43132f69c6f7e89d97c0c2ce32b0cf2f30e90.diff

LOG: [OpenMP] Fix bug 50022

Bug 50022 [0] reports target nowait fails in certain case, which is added in this
patch. The root cause of the failure is, when the second task is created, its
parent's `td_incomplete_child_tasks` will not be incremented because there is no
parallel region here thus its team is serialized. Therefore, when the initial
thread is waiting for its unfinished children tasks, it thought there is only
one, the first task, because it is hidden helper task, so it is tracked. The
second task will only be pushed to the queue when the first task is finished.
However, when the first task finishes, it first decrements the counter of its
parent, and then release dependences. Once the counter is decremented, the thread
will move on because its counter is reset, but actually, the second task has not
been executed at all. As a result, since in this case, the main function finishes,
then `libomp` starts to destroy. When the second task is pushed somewhere, all
some of the structures might already have already been destroyed, then anything
could happen.

This patch simply moves `__kmp_release_deps` ahead of decrement of the counter.
In this way, we can make sure that the initial thread is aware of the existence
of another task(s) so it will not move on. In addition, in order to tackle
dependence chain starting with hidden helper thread, when hidden helper task is
encountered, we force the task to release dependences.

Reference:
[0] https://bugs.llvm.org/show_bug.cgi?id=50022

Reviewed By: AndreyChurbanov

Differential Revision: https://reviews.llvm.org/D106519

Added: 
    openmp/libomptarget/test/offloading/bug50022.cpp

Modified: 
    openmp/runtime/src/kmp_tasking.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/test/offloading/bug50022.cpp b/openmp/libomptarget/test/offloading/bug50022.cpp
new file mode 100644
index 0000000000000..54ce06ea2d539
--- /dev/null
+++ b/openmp/libomptarget/test/offloading/bug50022.cpp
@@ -0,0 +1,39 @@
+// RUN: %libomptarget-compilexx-and-run-generic
+
+#include <cassert>
+#include <iostream>
+#include <stdexcept>
+
+int main(int argc, char *argv[]) {
+  int a = 0;
+  std::cout << "outside a = " << a << " addr " << &a << std::endl;
+#pragma omp target map(tofrom : a) depend(out : a) nowait
+  {
+    int sum = 0;
+    for (int i = 0; i < 100000; i++)
+      sum++;
+    a = 1;
+  }
+
+#pragma omp task depend(inout : a) shared(a)
+  {
+    std::cout << "a = " << a << " addr " << &a << std::endl;
+    if (a != 1)
+      throw std::runtime_error("wrong result!");
+    a = 2;
+  }
+
+#pragma omp task depend(inout : a) shared(a)
+  {
+    std::cout << "a = " << a << " addr " << &a << std::endl;
+    if (a != 2)
+      throw std::runtime_error("wrong result!");
+    a = 3;
+  }
+
+#pragma omp taskwait
+
+  assert(a == 3 && "wrong result!");
+
+  return 0;
+}

diff  --git a/openmp/runtime/src/kmp_tasking.cpp b/openmp/runtime/src/kmp_tasking.cpp
index bb41768dab850..936e79ccd03b3 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -940,16 +940,17 @@ static void __kmp_task_finish(kmp_int32 gtid, kmp_task_t *task,
     if (!(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser) ||
         taskdata->td_flags.detachable == TASK_DETACHABLE ||
         taskdata->td_flags.hidden_helper) {
+      __kmp_release_deps(gtid, taskdata);
       // Predecrement simulated by "- 1" calculation
       children =
           KMP_ATOMIC_DEC(&taskdata->td_parent->td_incomplete_child_tasks) - 1;
       KMP_DEBUG_ASSERT(children >= 0);
       if (taskdata->td_taskgroup)
         KMP_ATOMIC_DEC(&taskdata->td_taskgroup->count);
-      __kmp_release_deps(gtid, taskdata);
-    } else if (task_team && task_team->tt.tt_found_proxy_tasks) {
-      // if we found proxy tasks there could exist a dependency chain
-      // with the proxy task as origin
+    } else if (task_team && (task_team->tt.tt_found_proxy_tasks ||
+                             task_team->tt.tt_hidden_helper_task_encountered)) {
+      // if we found proxy or hidden helper tasks there could exist a dependency
+      // chain with the proxy task as origin
       __kmp_release_deps(gtid, taskdata);
     }
     // td_flags.executing must be marked as 0 after __kmp_release_deps has been