Skip to content

[OpenMP] Fix td_tdg_task_id underflow when taskloop and taskgraph #150877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

[OpenMP] Fix td_tdg_task_id underflow when taskloop and taskgraph #150877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4b49380
[OpenMP] Fix td_tdg_task_id underflow with taskloop and taskgraph
jpinot Jul 17, 2025
c8fdb64
[openmp] Remove taskgraph successors alloction before is needed
jpinot Sep 3, 2025
8936d9b
[openmp] Fix perent_task unitialization when allocating kmp_tdg_info
jpinot Sep 3, 2025
bf1f0df
[openmp] Fix locking when expanding recorded tdg
jpinot Sep 9, 2025
163b1eb
[openmp] Add tdg node pointer in kmp_taskdata_t
jpinot Sep 19, 2025
d18675e
[OpenMP] Add kmp_node_vector for TDG successors
jpinot Sep 19, 2025
32edeb6
[openmp] Delete graph_lock
jpinot Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions openmp/runtime/src/kmp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2649,6 +2649,14 @@ typedef struct kmp_node_info {
kmp_taskdata_t *parent_task; // Parent implicit task
} kmp_node_info_t;

// Representation of recorded nodes
typedef struct kmp_node_vector {
kmp_node_info_t **blocks;
kmp_int32 block_size;
std::atomic<kmp_int32> num_of_blocks;
kmp_bootstrap_lock_t lock;
} kmp_node_vector_t;

/// Represent a TDG's current status
typedef enum kmp_tdg_status {
KMP_TDG_NONE = 0,
Expand All @@ -2660,15 +2668,14 @@ typedef enum kmp_tdg_status {
typedef struct kmp_tdg_info {
kmp_int32 tdg_id; // Unique idenfifier of the TDG
kmp_taskgraph_flags_t tdg_flags; // Flags related to a TDG
kmp_int32 map_size; // Number of allocated TDG nodes
/* kmp_int32 map_size; // Number of allocated TDG nodes */
kmp_int32 num_roots; // Number of roots tasks int the TDG
kmp_int32 *root_tasks; // Array of tasks identifiers that are roots
kmp_node_info_t *record_map; // Array of TDG nodes
kmp_node_vector_t *record_map; // Array of TDG nodes
kmp_tdg_status_t tdg_status =
KMP_TDG_NONE; // Status of the TDG (recording, ready...)
std::atomic<kmp_int32> num_tasks; // Number of TDG nodes
kmp_bootstrap_lock_t
graph_lock; // Protect graph attributes when updated via taskloop_recur
std::atomic<kmp_int32> tdg_task_id_next; // Task id of next node
// Taskloop reduction related
void *rec_taskred_data; // Data to pass to __kmpc_task_reduction_init or
// __kmpc_taskred_init
Expand Down Expand Up @@ -2804,6 +2811,7 @@ struct kmp_taskdata { /* aligned during dynamic allocation */
#if OMPX_TASKGRAPH
bool is_taskgraph = 0; // whether the task is within a TDG
kmp_tdg_info_t *tdg; // used to associate task with a TDG
kmp_node_info_t *td_tdg_node_info; // node representing the task's in the TDG
kmp_int32 td_tdg_task_id; // local task id in its TDG
#endif
kmp_target_data_t td_target_data;
Expand Down
56 changes: 17 additions & 39 deletions openmp/runtime/src/kmp_taskdeps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,7 @@ static inline void __kmp_track_dependence(kmp_int32 gtid, kmp_depnode_t *source,
}
if (task_sink->is_taskgraph &&
__kmp_tdg_is_recording(task_sink->tdg->tdg_status)) {
kmp_node_info_t *source_info =
&task_sink->tdg->record_map[task_source->td_tdg_task_id];
kmp_node_info_t *source_info = task_source->td_tdg_node_info;
bool exists = false;
for (int i = 0; i < source_info->nsuccessors; i++) {
if (source_info->successors[i] == task_sink->td_tdg_task_id) {
Expand All @@ -244,21 +243,24 @@ static inline void __kmp_track_dependence(kmp_int32 gtid, kmp_depnode_t *source,
if (!exists) {
if (source_info->nsuccessors >= source_info->successors_size) {
kmp_uint old_size = source_info->successors_size;
source_info->successors_size = 2 * source_info->successors_size;
source_info->successors_size = old_size == 0
? __kmp_successors_size
: 2 * source_info->successors_size;
kmp_int32 *old_succ_ids = source_info->successors;
kmp_int32 *new_succ_ids = (kmp_int32 *)__kmp_allocate(
source_info->successors_size * sizeof(kmp_int32));
KMP_MEMCPY(new_succ_ids, old_succ_ids, old_size * sizeof(kmp_int32));
if (old_succ_ids) {
KMP_MEMCPY(new_succ_ids, old_succ_ids, old_size * sizeof(kmp_int32));
__kmp_free(old_succ_ids);
}
source_info->successors = new_succ_ids;
__kmp_free(old_succ_ids);
}

source_info->successors[source_info->nsuccessors] =
task_sink->td_tdg_task_id;
source_info->nsuccessors++;

kmp_node_info_t *sink_info =
&(task_sink->tdg->record_map[task_sink->td_tdg_task_id]);
kmp_node_info_t *sink_info = task_sink->td_tdg_node_info;
sink_info->npredecessors++;
}
}
Expand Down Expand Up @@ -700,39 +702,15 @@ kmp_int32 __kmpc_omp_task_with_deps(ident_t *loc_ref, kmp_int32 gtid,
__kmp_tdg_is_recording(new_taskdata->tdg->tdg_status)) {
kmp_tdg_info_t *tdg = new_taskdata->tdg;
// extend record_map if needed
if (new_taskdata->td_tdg_task_id >= tdg->map_size) {
__kmp_acquire_bootstrap_lock(&tdg->graph_lock);
if (new_taskdata->td_tdg_task_id >= tdg->map_size) {
kmp_uint old_size = tdg->map_size;
kmp_uint new_size = old_size * 2;
kmp_node_info_t *old_record = tdg->record_map;
kmp_node_info_t *new_record = (kmp_node_info_t *)__kmp_allocate(
new_size * sizeof(kmp_node_info_t));
KMP_MEMCPY(new_record, tdg->record_map,
old_size * sizeof(kmp_node_info_t));
tdg->record_map = new_record;

__kmp_free(old_record);

for (kmp_int i = old_size; i < new_size; i++) {
kmp_int32 *successorsList = (kmp_int32 *)__kmp_allocate(
__kmp_successors_size * sizeof(kmp_int32));
new_record[i].task = nullptr;
new_record[i].successors = successorsList;
new_record[i].nsuccessors = 0;
new_record[i].npredecessors = 0;
new_record[i].successors_size = __kmp_successors_size;
KMP_ATOMIC_ST_REL(&new_record[i].npredecessors_counter, 0);
}
// update the size at the end, so that we avoid other
// threads use old_record while map_size is already updated
tdg->map_size = new_size;
}
__kmp_release_bootstrap_lock(&tdg->graph_lock);
kmp_node_info_t *node =
kmp_node_vector_get(tdg->record_map, new_taskdata->td_tdg_task_id);
if (node == nullptr) {
kmp_node_vector_resize(tdg->record_map, new_taskdata->td_tdg_task_id * 2);
node = kmp_node_vector_get(tdg->record_map, new_taskdata->td_tdg_task_id);
}
tdg->record_map[new_taskdata->td_tdg_task_id].task = new_task;
tdg->record_map[new_taskdata->td_tdg_task_id].parent_task =
new_taskdata->td_parent;
node->task = new_task;
node->parent_task = new_taskdata->td_parent;
new_taskdata->td_tdg_node_info = node;
KMP_ATOMIC_INC(&tdg->num_tasks);
}
#endif
Expand Down
14 changes: 12 additions & 2 deletions openmp/runtime/src/kmp_taskdeps.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,26 @@ static inline void __kmp_dephash_free(kmp_info_t *thread, kmp_dephash_t *h) {
}

extern void __kmpc_give_task(kmp_task_t *ptask, kmp_int32 start);
#if OMPX_TASKGRAPH
extern kmp_node_vector_t *kmp_alloc_tdg_vector(kmp_int32 block_size);
extern kmp_node_info_t *kmp_node_vector_get(kmp_node_vector_t *vector,
kmp_int32 id);
extern void kmp_node_vector_resize(kmp_node_vector_t *vector, kmp_int32 size);
extern void kmp_node_vector_free(kmp_node_vector_t *vector);
#endif

static inline void __kmp_release_deps(kmp_int32 gtid, kmp_taskdata_t *task) {

#if OMPX_TASKGRAPH
if (task->is_taskgraph && !(__kmp_tdg_is_recording(task->tdg->tdg_status))) {
kmp_node_info_t *TaskInfo = &(task->tdg->record_map[task->td_tdg_task_id]);
kmp_node_info_t *TaskInfo = task->td_tdg_node_info;

for (int i = 0; i < TaskInfo->nsuccessors; i++) {
kmp_int32 successorNumber = TaskInfo->successors[i];
kmp_node_info_t *successor = &(task->tdg->record_map[successorNumber]);
kmp_node_info_t *successor =
kmp_node_vector_get(task->tdg->record_map, successorNumber);
/* kmp_node_info_t *successor = &(task->tdg->record_map[successorNumber]);
*/
kmp_int32 npredecessors = KMP_ATOMIC_DEC(&successor->npredecessors_counter) - 1;
if (successor->task != nullptr && npredecessors == 0) {
__kmp_omp_task(gtid, successor->task, false);
Expand Down
Loading
Loading