[Openmp-commits] [openmp] Draft: [OpenMP] Fix datarace with taskgraph dependencies and task stealing (PR #150880)

[Josep Pinot via Openmp-commits]

https://github.com/jpinot updated https://github.com/llvm/llvm-project/pull/150880

>From 276c7d07b372eb2305defc754697ea370b9ed138 Mon Sep 17 00:00:00 2001
From: jpinot <josep.pinot at bsc.es>
Date: Mon, 7 Jul 2025 10:53:46 +0200
Subject: [PATCH 1/3] [OpenMP] Fix datarace with taskgraph dependencies and
 task stealing

This commit resolves a datarace that could occur when taskgraph with
dependencies was enabled alongside task stealing. The problem occur
because, during task stealing, the stealing thread could incorrectly add
an extra node to the task dependency graph, while the original thread
might fail to create the correct node, leading to a corrupted graph and
potential execution issues.
---
 openmp/runtime/src/kmp_taskdeps.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/openmp/runtime/src/kmp_taskdeps.cpp b/openmp/runtime/src/kmp_taskdeps.cpp
index abbca752f0587..70443d7ff9507 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -329,6 +329,12 @@ __kmp_depnode_link_successor(kmp_int32 gtid, kmp_info_t *thread,
           if (!(__kmp_tdg_is_recording(tdg_status)) && task)
 #endif
             __kmp_track_dependence(gtid, dep, node, task);
+#if OMPX_TASKGRAPH
+          else if (KMP_TASK_TO_TASKDATA(dep->dn.task)->td_flags.onced) {
+            KMP_RELEASE_DEPNODE(gtid, dep);
+            continue;
+          }
+#endif
           dep->dn.successors = __kmp_add_node(thread, dep->dn.successors, node);
           KA_TRACE(40, ("__kmp_process_deps: T#%d adding dependence from %p to "
                         "%p\n",

>From 6be40d031a85b1fb8b085f699ace4709ab11d72f Mon Sep 17 00:00:00 2001
From: jpinot <jsp.pinot at gmail.com>
Date: Tue, 7 Oct 2025 12:18:54 +0200
Subject: [PATCH 2/3] Revert "[OpenMP] Fix datarace with taskgraph dependencies
 and task stealing"

This reverts commit cd9a91992a069416a5a2a82dae66eec7d434c31b.
---
 openmp/runtime/src/kmp_taskdeps.cpp | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/openmp/runtime/src/kmp_taskdeps.cpp b/openmp/runtime/src/kmp_taskdeps.cpp
index 70443d7ff9507..abbca752f0587 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -329,12 +329,6 @@ __kmp_depnode_link_successor(kmp_int32 gtid, kmp_info_t *thread,
           if (!(__kmp_tdg_is_recording(tdg_status)) && task)
 #endif
             __kmp_track_dependence(gtid, dep, node, task);
-#if OMPX_TASKGRAPH
-          else if (KMP_TASK_TO_TASKDATA(dep->dn.task)->td_flags.onced) {
-            KMP_RELEASE_DEPNODE(gtid, dep);
-            continue;
-          }
-#endif
           dep->dn.successors = __kmp_add_node(thread, dep->dn.successors, node);
           KA_TRACE(40, ("__kmp_process_deps: T#%d adding dependence from %p to "
                         "%p\n",

>From f5979238331d9032a5edf6b32af48f5285532a30 Mon Sep 17 00:00:00 2001
From: jpinot <jsp.pinot at gmail.com>
Date: Tue, 7 Oct 2025 12:08:35 +0200
Subject: [PATCH 3/3] [OpenMP] Fix successor reassignment race in taskgraph

Fix race condition when the same thread re-set a successor node for a
task that had already completed execution and released its dependencies.
This caused the successor to remain waiting indefinitely:

Thread 0 sets Task1 as successor of Task0
Thread 1 executes Task0 and releases successors
Thread 0 re-sets Task1 as successor of Task0
-> Task1 never executes because Task0 was already completed
---
 openmp/runtime/src/kmp_taskdeps.cpp | 14 ++++++++++++--
 openmp/runtime/src/kmp_taskdeps.h   |  4 ++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/openmp/runtime/src/kmp_taskdeps.cpp b/openmp/runtime/src/kmp_taskdeps.cpp
index abbca752f0587..409948b9a85b6 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -321,9 +321,19 @@ __kmp_depnode_link_successor(kmp_int32 gtid, kmp_info_t *thread,
         __kmp_track_dependence(gtid, dep, node, task);
     }
 #endif
-    if (dep->dn.task) {
+    if (dep->dn.task
+#if OMPX_TASKGRAPH
+        && __kmp_tdg_is_recording(tdg_status) &&
+        !KMP_TASK_TO_TASKDATA(dep->dn.task)->td_flags.onced
+#endif
+    ) {
       KMP_ACQUIRE_DEPNODE(gtid, dep);
-      if (dep->dn.task) {
+      if (dep->dn.task
+#if OMPX_TASKGRAPH
+          && __kmp_tdg_is_recording(tdg_status) &&
+          !KMP_TASK_TO_TASKDATA(dep->dn.task)->td_flags.onced
+#endif
+      ) {
         if (!dep->dn.successors || dep->dn.successors->node != node) {
 #if OMPX_TASKGRAPH
           if (!(__kmp_tdg_is_recording(tdg_status)) && task)
diff --git a/openmp/runtime/src/kmp_taskdeps.h b/openmp/runtime/src/kmp_taskdeps.h
index f6bfb39218a21..04161aca7795e 100644
--- a/openmp/runtime/src/kmp_taskdeps.h
+++ b/openmp/runtime/src/kmp_taskdeps.h
@@ -146,6 +146,10 @@ static inline void __kmp_release_deps(kmp_int32 gtid, kmp_taskdata_t *task) {
 #endif
     node->dn.task =
         NULL; // mark this task as finished, so no new dependencies are generated
+#if OMPX_TASKGRAPH
+  else
+    KMP_TASK_TO_TASKDATA(node->dn.task)->td_flags.onced = 1;
+#endif
   KMP_RELEASE_DEPNODE(gtid, node);
 
   kmp_depnode_list_t *next;