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

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: AndreyChurbanov, protze.joachim.
Herald added subscribers: guansong, yaxunl.
tianshilei1992 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: openmp-commits, sstefan1.
Herald added a project: OpenMP.

As discussed in D107121 <https://reviews.llvm.org/D107121>, task wait doesn't work when a regular task T depends on
a detached task or a hidden helper task T' in a serialized team, and T is the last
task. The root cause is, since the team is serialized, the last task will not be
tracked by `td_incomplete_child_tasks`. When T' is finished, it first releases
its dependences, and then decrements its parent counter. So far so good. For the
thread that is running task wait, if at the moment it is still spinning and trying
to execute tasks, it is fine because it can detect the new task and execute it.
However, if it happends to finish the function `flag.execute_tasks(...)`, it will
be broken because `td_incomplete_child_tasks` is 0 now.

In this patch, we update the rule to track children tasks a little bit. If the
task team encounters a proxy task or a hidden helper task, all following tasks
will be tracked.


Repository:
  rG LLVM Github Monorepo

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
@@ -938,9 +938,18 @@
 
     // 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) {
+    bool track_children =
+        !(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser);
+    track_children = track_children ||
+                     taskdata->td_flags.detachable == TASK_DETACHABLE ||
+                     taskdata->td_flags.hidden_helper;
+    if (thread->th.th_task_team)
+      track_children =
+          track_children ||
+          thread->th.th_task_team->tt.tt_hidden_helper_task_encountered ||
+          thread->th.th_task_team->tt.tt_found_proxy_tasks;
+
+    if (track_children) {
       __kmp_release_deps(gtid, taskdata);
       // Predecrement simulated by "- 1" calculation
 #if KMP_DEBUG
@@ -1378,11 +1387,18 @@
   if (UNLIKELY(ompt_enabled.enabled))
     __ompt_task_init(taskdata, gtid);
 #endif
+  bool track_children =
+      !(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser);
+  track_children = track_children || flags->proxy == TASK_PROXY ||
+                   flags->detachable == TASK_DETACHABLE || flags->hidden_helper;
+  if (thread->th.th_task_team)
+    track_children =
+        track_children ||
+        thread->th.th_task_team->tt.tt_hidden_helper_task_encountered ||
+        thread->th.th_task_team->tt.tt_found_proxy_tasks;
   // 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)) {
+  if (track_children) {
     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.364219.patch
Type: text/x-patch
Size: 3210 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20210804/21ee2d95/attachment.bin>