[Openmp-commits] [PATCH] D107496: [OpenMP] Fix task wait doesn't work as expected in serialized team

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 updated this revision to Diff 364517.
tianshilei1992 added a comment.
Herald added a subscriber: jfb.

rebase and fix comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107496

Files:
  openmp/runtime/src/kmp_tasking.cpp
  openmp/runtime/test/tasking/hidden_helper_task/depend.cpp
  openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp


Index: openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp
===================================================================
--- openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp
+++ openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp
@@ -1,4 +1,5 @@
 // RUN: %libomp-cxx-compile-and-run
+// RUN: %libomp-cxx-compile && env OMP_NUM_THREADS=1 %libomp-run
 
 /*
  * This test aims to check whether hidden helper thread has right gtid. We also
Index: openmp/runtime/test/tasking/hidden_helper_task/depend.cpp
===================================================================
--- openmp/runtime/test/tasking/hidden_helper_task/depend.cpp
+++ openmp/runtime/test/tasking/hidden_helper_task/depend.cpp
@@ -1,4 +1,5 @@
 // RUN: %libomp-cxx-compile-and-run
+// RUN: %libomp-cxx-compile && env OMP_NUM_THREADS=1 %libomp-run
 
 /*
  * This test aims to check whether hidden helper task can work with regular task
Index: openmp/runtime/src/kmp_tasking.cpp
===================================================================
--- openmp/runtime/src/kmp_tasking.cpp
+++ openmp/runtime/src/kmp_tasking.cpp
@@ -809,6 +809,25 @@
            gtid, taskdata, children));
 }
 
+// Only need to keep track of child task counts if any of the following:
+// 1. team parallel and tasking not serialized;
+// 2. it is a proxy or detachable or hidden helper task
+// 3. the children counter of its parent task is greater than 0.
+// The reason for the 3rd one is for serialized team that found detached task,
+// hidden helper task, T. In this case, the execution of T is still deferred,
+// and it is also possible that a regular task depends on T. In this case, if we
+// don't track the children, task synchronization will be broken.
+static bool __kmp_track_children_task(kmp_taskdata_t *taskdata) {
+  kmp_tasking_flags_t flags = taskdata->td_flags;
+  bool ret =
+      !(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser);
+  ret = ret || flags.proxy == TASK_PROXY ||
+        flags.detachable == TASK_DETACHABLE || flags.hidden_helper;
+  ret = ret ||
+        KMP_ATOMIC_LD_ACQ(&taskdata->td_parent->td_incomplete_child_tasks) > 0;
+  return ret;
+}
+
 // __kmp_task_finish: bookkeeping to do when a task finishes execution
 //
 // gtid: global thread ID for calling thread
@@ -935,12 +954,9 @@
     if (ompt)
       __ompt_task_finish(task, resumed_task, ompt_task_complete);
 #endif
-
-    // Only need to keep track of count if team parallel and tasking not
-    // serialized, or task is detachable and event has already been fulfilled
-    if (!(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser) ||
-        taskdata->td_flags.detachable == TASK_DETACHABLE ||
-        taskdata->td_flags.hidden_helper) {
+    // TODO: What would be the balance between the conditions in the function
+    // and an atomic operation?
+    if (__kmp_track_children_task(taskdata)) {
       __kmp_release_deps(gtid, taskdata);
       // Predecrement simulated by "- 1" calculation
 #if KMP_DEBUG
@@ -1378,11 +1394,9 @@
   if (UNLIKELY(ompt_enabled.enabled))
     __ompt_task_init(taskdata, gtid);
 #endif
-  // Only need to keep track of child task counts if team parallel and tasking
-  // not serialized or if it is a proxy or detachable or hidden helper task
-  if (flags->proxy == TASK_PROXY || flags->detachable == TASK_DETACHABLE ||
-      flags->hidden_helper ||
-      !(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser)) {
+  // TODO: What would be the balance between the conditions in the function and
+  // an atomic operation?
+  if (__kmp_track_children_task(taskdata)) {
     KMP_ATOMIC_INC(&parent_task->td_incomplete_child_tasks);
     if (parent_task->td_taskgroup)
       KMP_ATOMIC_INC(&parent_task->td_taskgroup->count);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107496.364517.patch
Type: text/x-patch
Size: 3776 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20210805/68011284/attachment-0001.bin>