Skip to content

Commit 8e29b4b

Browse files
[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
1 parent 946fd4e commit 8e29b4b

File tree

3 files changed

+89
-9
lines changed

3 files changed

+89
-9
lines changed

openmp/runtime/src/kmp_taskdeps.cpp

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,13 @@ __kmp_process_deps(kmp_int32 gtid, kmp_depnode_t *node, kmp_dephash_t **hash,
344344
// link node as successor of all nodes in the prev_set if any
345345
npredecessors +=
346346
__kmp_depnode_link_successor(gtid, thread, task, node, prev_set);
347+
if (dep_barrier) {
348+
// clean last_out and prev_set if any; don't touch last_set
349+
__kmp_node_deref(thread, last_out);
350+
info->last_out = NULL;
351+
__kmp_depnode_list_free(thread, prev_set);
352+
info->prev_set = NULL;
353+
}
347354
} else { // last_set is of different dep kind, make it prev_set
348355
// link node as successor of all nodes in the last_set
349356
npredecessors +=
@@ -353,13 +360,21 @@ __kmp_process_deps(kmp_int32 gtid, kmp_depnode_t *node, kmp_dephash_t **hash,
353360
info->last_out = NULL;
354361
// clean prev_set if any
355362
__kmp_depnode_list_free(thread, prev_set);
356-
// move last_set to prev_set, new last_set will be allocated
357-
info->prev_set = last_set;
363+
if (!dep_barrier) {
364+
// move last_set to prev_set, new last_set will be allocated
365+
info->prev_set = last_set;
366+
} else {
367+
info->prev_set = NULL;
368+
info->last_flag = 0;
369+
}
358370
info->last_set = NULL;
359371
}
360-
info->last_flag = dep->flag; // store dep kind of the last_set
361-
info->last_set = __kmp_add_node(thread, info->last_set, node);
362-
372+
// for dep_barrier last_flag value should remain:
373+
// 0 if last_set is empty, unchanged otherwise
374+
if (!dep_barrier) {
375+
info->last_flag = dep->flag; // store dep kind of the last_set
376+
info->last_set = __kmp_add_node(thread, info->last_set, node);
377+
}
363378
// check if we are processing MTX dependency
364379
if (dep->flag == KMP_DEP_MTX) {
365380
if (info->mtx_lock == NULL) {
@@ -756,8 +771,6 @@ void __kmpc_omp_wait_deps(ident_t *loc_ref, kmp_int32 gtid, kmp_int32 ndeps,
756771

757772
kmp_depnode_t node = {0};
758773
__kmp_init_node(&node);
759-
// the stack owns the node
760-
__kmp_node_ref(&node);
761774

762775
if (!__kmp_check_deps(gtid, &node, NULL, &current_task->td_dephash,
763776
DEP_BARRIER, ndeps, dep_list, ndeps_noalias,

openmp/runtime/src/kmp_taskdeps.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ static inline void __kmp_node_deref(kmp_info_t *thread, kmp_depnode_t *node) {
2323
return;
2424

2525
kmp_int32 n = KMP_ATOMIC_DEC(&node->dn.nrefs) - 1;
26-
// TODO: temporarily disable assertion until the bug with dependences is fixed
27-
// KMP_DEBUG_ASSERT(n >= 0);
26+
KMP_DEBUG_ASSERT(n >= 0);
2827
if (n == 0) {
2928
KMP_ASSERT(node->dn.nrefs == 0);
3029
#if USE_FAST_MEMORY
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// RUN: %libomp-compile-and-run
2+
3+
// test checks IN dep kind in depend clause on taskwait construct
4+
// uses codegen emulation
5+
#include <stdio.h>
6+
#include <omp.h>
7+
// ---------------------------------------------------------------------------
8+
// internal data to emulate compiler codegen
9+
typedef struct DEP {
10+
size_t addr;
11+
size_t len;
12+
unsigned char flags;
13+
} _dep;
14+
typedef struct ID {
15+
int reserved_1;
16+
int flags;
17+
int reserved_2;
18+
int reserved_3;
19+
char *psource;
20+
} _id;
21+
22+
#ifdef __cplusplus
23+
extern "C" {
24+
#endif
25+
extern int __kmpc_global_thread_num(_id*);
26+
extern void __kmpc_omp_wait_deps(_id *, int, int, _dep *, int, _dep *);
27+
#ifdef __cplusplus
28+
} // extern "C"
29+
#endif
30+
31+
int main()
32+
{
33+
int i1,i2,i3;
34+
omp_set_num_threads(2);
35+
printf("addresses: %p %p %p\n", &i1, &i2, &i3);
36+
#pragma omp parallel
37+
{
38+
int t = omp_get_thread_num();
39+
printf("thread %d enters parallel\n", t);
40+
#pragma omp single
41+
{
42+
#pragma omp task depend(in: i3)
43+
{
44+
int th = omp_get_thread_num();
45+
printf("task 0 created by th %d, executed by th %d\n", t, th);
46+
}
47+
#pragma omp task depend(in: i2)
48+
{
49+
int th = omp_get_thread_num();
50+
printf("task 1 created by th %d, executed by th %d\n", t, th);
51+
}
52+
// #pragma omp taskwait depend(in: i1, i2)
53+
{
54+
_dep sdep[2];
55+
static _id loc = {0, 2, 0, 0, ";test9.c;func;60;0;;"};
56+
int gtid = __kmpc_global_thread_num(&loc);
57+
sdep[0].addr = (size_t)&i2;
58+
sdep[0].flags = 1; // 1-in, 2-out, 3-inout, 4-mtx, 8-inoutset
59+
sdep[1].addr = (size_t)&i1;
60+
sdep[1].flags = 1; // in
61+
__kmpc_omp_wait_deps(&loc, gtid, 2, sdep, 0, NULL);
62+
}
63+
printf("single done\n");
64+
}
65+
}
66+
printf("passed\n");
67+
return 0;
68+
}

0 commit comments

Comments
 (0)