Skip to content

Commit 8442967

Browse files
committed
[OpenMP] Fix task wait doesn't work as expected in serialized team
As discussed in 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. 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. Reviewed By: AndreyChurbanov Differential Revision: https://reviews.llvm.org/D107496
1 parent 6c0181c commit 8442967

File tree

3 files changed

+26
-11
lines changed

3 files changed

+26
-11
lines changed

openmp/runtime/src/kmp_tasking.cpp

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,24 @@ static void __kmp_free_task_and_ancestors(kmp_int32 gtid,
807807
gtid, taskdata, children));
808808
}
809809

810+
// Only need to keep track of child task counts if any of the following:
811+
// 1. team parallel and tasking not serialized;
812+
// 2. it is a proxy or detachable or hidden helper task
813+
// 3. the children counter of its parent task is greater than 0.
814+
// The reason for the 3rd one is for serialized team that found detached task,
815+
// hidden helper task, T. In this case, the execution of T is still deferred,
816+
// and it is also possible that a regular task depends on T. In this case, if we
817+
// don't track the children, task synchronization will be broken.
818+
static bool __kmp_track_children_task(kmp_taskdata_t *taskdata) {
819+
kmp_tasking_flags_t flags = taskdata->td_flags;
820+
bool ret = !(flags.team_serial || flags.tasking_ser);
821+
ret = ret || flags.proxy == TASK_PROXY ||
822+
flags.detachable == TASK_DETACHABLE || flags.hidden_helper;
823+
ret = ret ||
824+
KMP_ATOMIC_LD_ACQ(&taskdata->td_parent->td_incomplete_child_tasks) > 0;
825+
return ret;
826+
}
827+
810828
// __kmp_task_finish: bookkeeping to do when a task finishes execution
811829
//
812830
// gtid: global thread ID for calling thread
@@ -933,12 +951,9 @@ static void __kmp_task_finish(kmp_int32 gtid, kmp_task_t *task,
933951
if (ompt)
934952
__ompt_task_finish(task, resumed_task, ompt_task_complete);
935953
#endif
936-
937-
// Only need to keep track of count if team parallel and tasking not
938-
// serialized, or task is detachable and event has already been fulfilled
939-
if (!(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser) ||
940-
taskdata->td_flags.detachable == TASK_DETACHABLE ||
941-
taskdata->td_flags.hidden_helper) {
954+
// TODO: What would be the balance between the conditions in the function
955+
// and an atomic operation?
956+
if (__kmp_track_children_task(taskdata)) {
942957
__kmp_release_deps(gtid, taskdata);
943958
// Predecrement simulated by "- 1" calculation
944959
#if KMP_DEBUG
@@ -1376,11 +1391,9 @@ kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, kmp_int32 gtid,
13761391
if (UNLIKELY(ompt_enabled.enabled))
13771392
__ompt_task_init(taskdata, gtid);
13781393
#endif
1379-
// Only need to keep track of child task counts if team parallel and tasking
1380-
// not serialized or if it is a proxy or detachable or hidden helper task
1381-
if (flags->proxy == TASK_PROXY || flags->detachable == TASK_DETACHABLE ||
1382-
flags->hidden_helper ||
1383-
!(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser)) {
1394+
// TODO: What would be the balance between the conditions in the function and
1395+
// an atomic operation?
1396+
if (__kmp_track_children_task(taskdata)) {
13841397
KMP_ATOMIC_INC(&parent_task->td_incomplete_child_tasks);
13851398
if (parent_task->td_taskgroup)
13861399
KMP_ATOMIC_INC(&parent_task->td_taskgroup->count);

openmp/runtime/test/tasking/hidden_helper_task/depend.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %libomp-cxx-compile-and-run
2+
// RUN: %libomp-cxx-compile && env OMP_NUM_THREADS=1 %libomp-run
23

34
/*
45
* This test aims to check whether hidden helper task can work with regular task

openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// RUN: %libomp-cxx-compile-and-run
2+
// RUN: %libomp-cxx-compile && env OMP_NUM_THREADS=1 %libomp-run
23

34
/*
45
* This test aims to check whether hidden helper thread has right gtid. We also

0 commit comments

Comments
 (0)