[Openmp-commits] [PATCH] D101082: [OpenMP] Fix deadlock for detachable task with child tasks

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim updated this revision to Diff 346470.
protze.joachim edited the summary of this revision.
protze.joachim added a comment.

The initial fix broke in some cases. I missed the difference between the incomplete and allocated task counters.
This updated patch should fix the issue, but I still run into starvation issues. I'll attach another reproducer in the bug.


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

https://reviews.llvm.org/D101082

Files:
  openmp/runtime/src/kmp_tasking.cpp
  openmp/runtime/test/tasking/detach_nested_task.c


Index: openmp/runtime/test/tasking/detach_nested_task.c
===================================================================
--- /dev/null
+++ openmp/runtime/test/tasking/detach_nested_task.c
@@ -0,0 +1,54 @@
+// RUN: %libomp-compile-and-run
+
+// The outer detachable task creates multiple child tasks with dependencies
+// when the last inner task incremented ret, the task calls omp_fulfill_event
+// to release the outer task.
+
+#include <omp.h>
+#include <stdio.h>
+
+int *buf;
+
+int foo(int n)
+{
+  int ret = 0;
+  for (int i = 0; i < n; ++i) {
+    omp_event_handle_t event;
+    #pragma omp task detach(event) firstprivate(i,n,event) shared(ret) default(none)
+    {
+      for (int j = 0; j < n; ++j) {
+        #pragma omp task firstprivate(event,i,j,n) shared(ret) default(none) depend(out:ret)
+        {
+          //printf("Task %i, %i: %i\n", i, j, omp_get_thread_num());
+          #pragma omp atomic
+            ret++;
+#if _OPENMP
+          if (j == n-1) {
+            //printf("Task %i, %i: omp_fulfill_event()\n", i, j);
+            omp_fulfill_event(event);
+          }
+#endif
+        }
+      }
+    }
+  }
+  // the taskwait only guarantees the outer tasks to complete.
+  #pragma omp taskwait
+
+  return ret;
+}
+
+
+int main()
+{
+  int ret;
+#pragma omp parallel
+#pragma omp master
+  {
+    ret = foo(8);
+  }
+  printf("%i\n", ret);
+  //CHECK: 64
+  return 0;
+}
+
Index: openmp/runtime/src/kmp_tasking.cpp
===================================================================
--- openmp/runtime/src/kmp_tasking.cpp
+++ openmp/runtime/src/kmp_tasking.cpp
@@ -3842,6 +3842,7 @@
   return result;
 }
 
+#define PROXY_TASK_FLAG 0x40000000
 /* The finish of the proxy tasks is divided in two pieces:
     - the top half is the one that can be done from a thread outside the team
     - the bottom half must be run from a thread within the team
@@ -3871,7 +3872,7 @@
 
   // Create an imaginary children for this task so the bottom half cannot
   // release the task before we have completed the second top half
-  KMP_ATOMIC_INC(&taskdata->td_incomplete_child_tasks);
+  KMP_ATOMIC_OR(&taskdata->td_incomplete_child_tasks, PROXY_TASK_FLAG);
 }
 
 static void __kmp_second_top_half_finish_proxy(kmp_taskdata_t *taskdata) {
@@ -3883,7 +3884,7 @@
   KMP_DEBUG_ASSERT(children >= 0);
 
   // Remove the imaginary children
-  KMP_ATOMIC_DEC(&taskdata->td_incomplete_child_tasks);
+  KMP_ATOMIC_AND(&taskdata->td_incomplete_child_tasks, !PROXY_TASK_FLAG);
 }
 
 static void __kmp_bottom_half_finish_proxy(kmp_int32 gtid, kmp_task_t *ptask) {
@@ -3896,7 +3897,8 @@
 
   // We need to wait to make sure the top half is finished
   // Spinning here should be ok as this should happen quickly
-  while (KMP_ATOMIC_LD_ACQ(&taskdata->td_incomplete_child_tasks) > 0)
+  while ((KMP_ATOMIC_LD_ACQ(&taskdata->td_incomplete_child_tasks) &
+          PROXY_TASK_FLAG) > 0)
     ;
 
   __kmp_release_deps(gtid, taskdata);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101082.346470.patch
Type: text/x-patch
Size: 2938 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20210519/79408c40/attachment.bin>