[llvm-branch-commits] [openmp] 2792379 - [OpenMP] libomp: taskwait depend implementation fixed.

Tom Stellard via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Aug 5 10:13:54 PDT 2021


Author: AndreyChurbanov
Date: 2021-08-05T10:13:27-07:00
New Revision: 279237937b330d8ca570ffa7b0cb7e1bbda57fca

URL: https://github.com/llvm/llvm-project/commit/279237937b330d8ca570ffa7b0cb7e1bbda57fca
DIFF: https://github.com/llvm/llvm-project/commit/279237937b330d8ca570ffa7b0cb7e1bbda57fca.diff

LOG: [OpenMP] libomp: taskwait depend implementation fixed.

Fix for https://bugs.llvm.org/show_bug.cgi?id=49723.
Eliminated references from task dependency hash to node allocated on stack,
thus eliminated accesses to stale memory. So the node now never freed.
Uncommented assertion which triggered when stale memory accessed.
Removed unneeded ref count increment for stack allocated node.

Differential Revision: https://reviews.llvm.org/D106705

(cherry picked from commit 8e29b4b323b87f3855dc71abf1e3f3d48952a4e4)

Added: 
    openmp/runtime/test/tasking/kmp_taskwait_depend_in.c

Modified: 
    openmp/runtime/src/kmp_taskdeps.cpp
    openmp/runtime/src/kmp_taskdeps.h

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/src/kmp_taskdeps.cpp b/openmp/runtime/src/kmp_taskdeps.cpp
index 162fb38e1eedd..dd3e7688d33f7 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -344,6 +344,13 @@ __kmp_process_deps(kmp_int32 gtid, kmp_depnode_t *node, kmp_dephash_t **hash,
         // link node as successor of all nodes in the prev_set if any
         npredecessors +=
             __kmp_depnode_link_successor(gtid, thread, task, node, prev_set);
+        if (dep_barrier) {
+          // clean last_out and prev_set if any; don't touch last_set
+          __kmp_node_deref(thread, last_out);
+          info->last_out = NULL;
+          __kmp_depnode_list_free(thread, prev_set);
+          info->prev_set = NULL;
+        }
       } else { // last_set is of 
diff erent dep kind, make it prev_set
         // link node as successor of all nodes in the last_set
         npredecessors +=
@@ -353,13 +360,21 @@ __kmp_process_deps(kmp_int32 gtid, kmp_depnode_t *node, kmp_dephash_t **hash,
         info->last_out = NULL;
         // clean prev_set if any
         __kmp_depnode_list_free(thread, prev_set);
-        // move last_set to prev_set, new last_set will be allocated
-        info->prev_set = last_set;
+        if (!dep_barrier) {
+          // move last_set to prev_set, new last_set will be allocated
+          info->prev_set = last_set;
+        } else {
+          info->prev_set = NULL;
+          info->last_flag = 0;
+        }
         info->last_set = NULL;
       }
-      info->last_flag = dep->flag; // store dep kind of the last_set
-      info->last_set = __kmp_add_node(thread, info->last_set, node);
-
+      // for dep_barrier last_flag value should remain:
+      // 0 if last_set is empty, unchanged otherwise
+      if (!dep_barrier) {
+        info->last_flag = dep->flag; // store dep kind of the last_set
+        info->last_set = __kmp_add_node(thread, info->last_set, node);
+      }
       // check if we are processing MTX dependency
       if (dep->flag == KMP_DEP_MTX) {
         if (info->mtx_lock == NULL) {
@@ -756,8 +771,6 @@ void __kmpc_omp_wait_deps(ident_t *loc_ref, kmp_int32 gtid, kmp_int32 ndeps,
 
   kmp_depnode_t node = {0};
   __kmp_init_node(&node);
-  // the stack owns the node
-  __kmp_node_ref(&node);
 
   if (!__kmp_check_deps(gtid, &node, NULL, &current_task->td_dephash,
                         DEP_BARRIER, ndeps, dep_list, ndeps_noalias,

diff  --git a/openmp/runtime/src/kmp_taskdeps.h b/openmp/runtime/src/kmp_taskdeps.h
index d1576dd5b7910..73abf07018f31 100644
--- a/openmp/runtime/src/kmp_taskdeps.h
+++ b/openmp/runtime/src/kmp_taskdeps.h
@@ -23,8 +23,7 @@ static inline void __kmp_node_deref(kmp_info_t *thread, kmp_depnode_t *node) {
     return;
 
   kmp_int32 n = KMP_ATOMIC_DEC(&node->dn.nrefs) - 1;
-  // TODO: temporarily disable assertion until the bug with dependences is fixed
-  //  KMP_DEBUG_ASSERT(n >= 0);
+  KMP_DEBUG_ASSERT(n >= 0);
   if (n == 0) {
     KMP_ASSERT(node->dn.nrefs == 0);
 #if USE_FAST_MEMORY

diff  --git a/openmp/runtime/test/tasking/kmp_taskwait_depend_in.c b/openmp/runtime/test/tasking/kmp_taskwait_depend_in.c
new file mode 100644
index 0000000000000..fef29ea60b487
--- /dev/null
+++ b/openmp/runtime/test/tasking/kmp_taskwait_depend_in.c
@@ -0,0 +1,68 @@
+// RUN: %libomp-compile-and-run
+
+// test checks IN dep kind in depend clause on taskwait construct
+// uses codegen emulation
+#include <stdio.h>
+#include <omp.h>
+// ---------------------------------------------------------------------------
+// internal data to emulate compiler codegen
+typedef struct DEP {
+  size_t addr;
+  size_t len;
+  unsigned char flags;
+} _dep;
+typedef struct ID {
+  int reserved_1;
+  int flags;
+  int reserved_2;
+  int reserved_3;
+  char *psource;
+} _id;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+extern int __kmpc_global_thread_num(_id*);
+extern void __kmpc_omp_wait_deps(_id *, int, int, _dep *, int, _dep *);
+#ifdef __cplusplus
+} // extern "C"
+#endif
+
+int main()
+{
+  int i1,i2,i3;
+  omp_set_num_threads(2);
+  printf("addresses: %p %p %p\n", &i1, &i2, &i3);
+  #pragma omp parallel
+  {
+    int t = omp_get_thread_num();
+    printf("thread %d enters parallel\n", t);
+    #pragma omp single
+    {
+      #pragma omp task depend(in: i3)
+      {
+        int th = omp_get_thread_num();
+        printf("task 0 created by th %d, executed by th %d\n", t, th);
+      }
+      #pragma omp task depend(in: i2)
+      {
+        int th = omp_get_thread_num();
+        printf("task 1 created by th %d, executed by th %d\n", t, th);
+      }
+//      #pragma omp taskwait depend(in: i1, i2)
+      {
+        _dep sdep[2];
+        static _id loc = {0, 2, 0, 0, ";test9.c;func;60;0;;"};
+        int gtid = __kmpc_global_thread_num(&loc);
+        sdep[0].addr = (size_t)&i2;
+        sdep[0].flags = 1; // 1-in, 2-out, 3-inout, 4-mtx, 8-inoutset
+        sdep[1].addr = (size_t)&i1;
+        sdep[1].flags = 1; // in
+        __kmpc_omp_wait_deps(&loc, gtid, 2, sdep, 0, NULL);
+      }
+      printf("single done\n");
+    }
+  }
+  printf("passed\n");
+  return 0;
+}


        


More information about the llvm-branch-commits mailing list