[OMPT] Adapt OMPT callback structure for tasks to handle untied tasks correctly (+ adapt testcase)

[sconvent]

Summary

The ompt/tasks/task_types.c testcase did not test untied tasks properly. Now, frame addresses are tested and two scheduling points are added at which the task can switch to another thread. Due to scheduling effects, the frame address could be NULL.

This needed a restructure of the way OMPT callbacks are called. __ompt_task_finish() now as an extra parameter, whether a task is completed. Its invocation has been moved into __kmp_task_finish(). Thus, the order of the writes to the frame addresses is not subject to scheduling effects anymore.

[hbae]

runtime/src/kmp_tasking.cpp

1327 ↗

(On Diff #154972)

This should be removed.

1423 ↗

(On Diff #154972)

I am not sure if the comment still corresponds to the code here.

[protze.joachim]

I apply suggested changes by hbae on commit.

LGTM

openmp/trunk/runtime/src/kmp_tasking.cpp

ompt_callbacks.ompt_callback(ompt_callback_task_schedule)(
         &(current_task->ompt_task_info.task_data), status,
         &(taskdata->ompt_task_info.task_data));
   }
   taskdata->ompt_task_info.scheduling_parent = current_task;
 }
 
 // __ompt_task_finish:
 //   Build and trigger final task-schedule event
-static inline void __ompt_task_finish(kmp_task_t *task,
-                                      kmp_taskdata_t *resumed_task) {
+static inline void
+__ompt_task_finish(kmp_task_t *task, kmp_taskdata_t *resumed_task,
+                   ompt_task_status_t status = ompt_task_complete) {
   kmp_taskdata_t *taskdata = KMP_TASK_TO_TASKDATA(task);
-  ompt_task_status_t status = ompt_task_complete;
-  if (taskdata->td_flags.tiedness == TASK_UNTIED &&
-      KMP_TEST_THEN_ADD32(&(taskdata->td_untied_count), 0) > 1)
-    status = ompt_task_others;
   if (__kmp_omp_cancellation && taskdata->td_taskgroup &&
       taskdata->td_taskgroup->cancel_request == cancel_taskgroup) {
     status = ompt_task_cancel;
   }
 
   /* let OMPT know that we're returning to the callee task */
   if (ompt_enabled.ompt_callback_task_schedule) {
     ompt_callbacks.ompt_callback(ompt_callback_task_schedule)(
KA_TRACE(
            gtid, taskdata, children));
 }
 
 // __kmp_task_finish: bookkeeping to do when a task finishes execution
 //
 // gtid: global thread ID for calling thread
 // task: task to be finished
 // resumed_task: task to be resumed.  (may be NULL if task is serialized)
+template <bool ompt>
 static void __kmp_task_finish(kmp_int32 gtid, kmp_task_t *task,
                               kmp_taskdata_t *resumed_task) {
   kmp_taskdata_t *taskdata = KMP_TASK_TO_TASKDATA(task);
   kmp_info_t *thread = __kmp_threads[gtid];
   kmp_task_team_t *task_team =
       thread->th.th_task_team; // might be NULL for serial teams...
   kmp_int32 children = 0;
 
if (counter > 0) {
       thread->th.th_current_task = resumed_task; // restore current_task
       resumed_task->td_flags.executing = 1; // resume previous task
       KA_TRACE(10, ("__kmp_task_finish(exit): T#%d partially done task %p, "
                     "resuming task %p\n",
                     gtid, taskdata, resumed_task));
       return;
     }
   }
+#if OMPT_SUPPORT
+  if (ompt)
+    __ompt_task_finish(task, resumed_task);
+#endif
 
   KMP_DEBUG_ASSERT(taskdata->td_flags.complete == 0);
   taskdata->td_flags.complete = 1; // mark the task as completed
   KMP_DEBUG_ASSERT(taskdata->td_flags.started == 1);
   KMP_DEBUG_ASSERT(taskdata->td_flags.freed == 0);
 
   // Only need to keep track of count if team parallel and tasking not
   // serialized

 
 template <bool ompt>
 static void __kmpc_omp_task_complete_if0_template(ident_t *loc_ref,
                                                   kmp_int32 gtid,
                                                   kmp_task_t *task) {
   KA_TRACE(10, ("__kmpc_omp_task_complete_if0(enter): T#%d loc=%p task=%p\n",
                 gtid, loc_ref, KMP_TASK_TO_TASKDATA(task)));
   // this routine will provide task to resume
-  __kmp_task_finish(gtid, task, NULL);
+  __kmp_task_finish<ompt>(gtid, task, NULL);
 
   KA_TRACE(10, ("__kmpc_omp_task_complete_if0(exit): T#%d loc=%p task=%p\n",
                 gtid, loc_ref, KMP_TASK_TO_TASKDATA(task)));
 
 #if OMPT_SUPPORT
   if (ompt) {
-    __ompt_task_finish(task, NULL);
     omp_frame_t *ompt_frame;
     __ompt_get_task_info_internal(0, NULL, NULL, &ompt_frame, NULL, NULL);
     ompt_frame->enter_frame = NULL;
   }
 #endif
 
   return;
 }

 #ifdef TASK_UNUSED
 // __kmpc_omp_task_complete: report that a task has completed execution
 // NEVER GENERATED BY COMPILER, DEPRECATED!!!
 void __kmpc_omp_task_complete(ident_t *loc_ref, kmp_int32 gtid,
                               kmp_task_t *task) {
   KA_TRACE(10, ("__kmpc_omp_task_complete(enter): T#%d loc=%p task=%p\n", gtid,
                 loc_ref, KMP_TASK_TO_TASKDATA(task)));
 
-  __kmp_task_finish(gtid, task, NULL); // Not sure how to find task to resume
+  __kmp_task_finish<false>(gtid, task,
+                           NULL); // Not sure how to find task to resume
 
   KA_TRACE(10, ("__kmpc_omp_task_complete(exit): T#%d loc=%p task=%p\n", gtid,
                 loc_ref, KMP_TASK_TO_TASKDATA(task)));
   return;
 }
 #endif // TASK_UNUSED
 
 // __kmp_init_implicit_task: Initialize the appropriate fields in the implicit
#if OMP_40_ENABLED
   taskdata->td_dephash = NULL;
   taskdata->td_depnode = NULL;
 #endif
   if (flags->tiedness == TASK_UNTIED)
     taskdata->td_last_tied = NULL; // will be set when the task is scheduled
   else
     taskdata->td_last_tied = taskdata;
 
+#if OMPT_SUPPORT
+  if (UNLIKELY(ompt_enabled.enabled))
+    __ompt_task_init(taskdata, gtid);
+#endif
 // Only need to keep track of child task counts if team parallel and tasking not
 // serialized or if it is a proxy task
 #if OMP_45_ENABLED
   if (flags->proxy == TASK_PROXY ||
       !(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser))
 #else
   if (!(taskdata->td_flags.team_serial || taskdata->td_flags.tasking_ser))
 #endif
if (taskdata->td_parent->td_flags.tasktype == TASK_EXPLICIT) {
       KMP_ATOMIC_INC(&taskdata->td_parent->td_allocated_child_tasks);
     }
   }
 
   KA_TRACE(20, ("__kmp_task_alloc(exit): T#%d created task %p parent=%p\n",
                 gtid, taskdata, taskdata->td_parent));
   ANNOTATE_HAPPENS_BEFORE(task);
 
-#if OMPT_SUPPORT
-  if (UNLIKELY(ompt_enabled.enabled))
-    __ompt_task_init(taskdata, gtid);
-#endif
-
   return task;
 }
 
 kmp_task_t *__kmpc_omp_task_alloc(ident_t *loc_ref, kmp_int32 gtid,
                                   kmp_int32 flags, size_t sizeof_kmp_task_t,
                                   size_t sizeof_shareds,
                                   kmp_routine_entry_t task_entry) {
   kmp_task_t *retval;

 #if USE_ITT_BUILD && USE_ITT_NOTIFY
   if (__kmp_forkjoin_frames_mode == 3) {
     // Get the current time stamp to measure task execution time to correct
     // barrier imbalance time
     cur_time = __itt_get_timestamp();
   }
 #endif
 
-#if OMP_45_ENABLED
-  // Proxy tasks are not handled by the runtime
-  if (taskdata->td_flags.proxy != TASK_PROXY) {
-#endif
-    ANNOTATE_HAPPENS_AFTER(task);
-    __kmp_task_start(gtid, task, current_task); // OMPT only if not discarded
-#if OMP_45_ENABLED
-  }
-#endif
-
 #if OMPT_SUPPORT
+  // For untied tasks, the first task executed only calls __kmpc_omp_task and
+  // does not execute code.
   ompt_thread_info_t oldInfo;
   kmp_info_t *thread;
   if (UNLIKELY(ompt_enabled.enabled)) {
     // Store the threads states and restore them after the task
     thread = __kmp_threads[gtid];
     oldInfo = thread->th.ompt_thread_info;
     thread->th.ompt_thread_info.wait_id = 0;
     thread->th.ompt_thread_info.state = (thread->th.th_team_serialized)
                                             ? omp_state_work_serial
                                             : omp_state_work_parallel;
     taskdata->ompt_task_info.frame.exit_frame = OMPT_GET_FRAME_ADDRESS(0);
   }
 #endif
 
+#if OMP_45_ENABLED
+  // Proxy tasks are not handled by the runtime
+  if (taskdata->td_flags.proxy != TASK_PROXY) {
+#endif
+    ANNOTATE_HAPPENS_AFTER(task);
+    __kmp_task_start(gtid, task, current_task); // OMPT only if not discarded
+#if OMP_45_ENABLED
+  }
+#endif
+
 #if OMP_40_ENABLED
   // TODO: cancel tasks if the parallel region has also been cancelled
   // TODO: check if this sequence can be hoisted above __kmp_task_start
   // if cancellation has been enabled for this run ...
   if (__kmp_omp_cancellation) {
     kmp_info_t *this_thr = __kmp_threads[gtid];
     kmp_team_t *this_team = this_thr->th.th_team;
     kmp_taskgroup_t *taskgroup = taskdata->td_taskgroup;
if (taskdata->td_flags.native) {
       ((void (*)(void *))(*(task->routine)))(task->shareds);
     } else
 #endif /* KMP_GOMP_COMPAT */
     {
       (*(task->routine))(gtid, task);
     }
     KMP_POP_PARTITIONED_TIMER();
 
-#if OMPT_SUPPORT
-    if (UNLIKELY(ompt_enabled.enabled))
-      __ompt_task_finish(task, current_task);
-#endif
 #if OMP_40_ENABLED
   }
 #endif // OMP_40_ENABLED
 
-#if OMPT_SUPPORT
-  if (UNLIKELY(ompt_enabled.enabled)) {
-    thread->th.ompt_thread_info = oldInfo;
-    taskdata->ompt_task_info.frame.exit_frame = NULL;
-  }
-#endif
 
 #if OMP_45_ENABLED
   // Proxy tasks are not handled by the runtime
   if (taskdata->td_flags.proxy != TASK_PROXY) {
 #endif
     ANNOTATE_HAPPENS_BEFORE(taskdata->td_parent);
-    __kmp_task_finish(gtid, task, current_task); // OMPT only if not discarded
+#if OMPT_SUPPORT
+    if (UNLIKELY(ompt_enabled.enabled)) {
+      thread->th.ompt_thread_info = oldInfo;
+      if (taskdata->td_flags.tiedness == TASK_TIED) {
+        taskdata->ompt_task_info.frame.exit_frame = NULL;
+      }
+      __kmp_task_finish<true>(gtid, task, current_task);
+    } else
+#endif
+      __kmp_task_finish<false>(gtid, task, current_task);
 #if OMP_45_ENABLED
   }
 #endif
 
 #if USE_ITT_BUILD && USE_ITT_NOTIFY
   // Barrier imbalance - correct arrive time after the task finished
   if (__kmp_forkjoin_frames_mode == 3) {
     kmp_info_t *this_thr = __kmp_threads[gtid];

 #if KMP_DEBUG || OMPT_SUPPORT
   kmp_taskdata_t *new_taskdata = KMP_TASK_TO_TASKDATA(new_task);
 #endif
   KA_TRACE(10, ("__kmpc_omp_task(enter): T#%d loc=%p task=%p\n", gtid, loc_ref,
                 new_taskdata));
 
 #if OMPT_SUPPORT
   kmp_taskdata_t *parent = NULL;
-  if (UNLIKELY(ompt_enabled.enabled && !new_taskdata->td_flags.started)) {
+  if (UNLIKELY(ompt_enabled.enabled)) {
+    if (!new_taskdata->td_flags.started) {
       OMPT_STORE_RETURN_ADDRESS(gtid);
       parent = new_taskdata->td_parent;
-    if (!parent->ompt_task_info.frame.enter_frame)
+      if (!parent->ompt_task_info.frame.enter_frame) {
         parent->ompt_task_info.frame.enter_frame = OMPT_GET_FRAME_ADDRESS(1);
+      }
       if (ompt_enabled.ompt_callback_task_create) {
         ompt_data_t task_data = ompt_data_none;
         ompt_callbacks.ompt_callback(ompt_callback_task_create)(
             parent ? &(parent->ompt_task_info.task_data) : &task_data,
             parent ? &(parent->ompt_task_info.frame) : NULL,
             &(new_taskdata->ompt_task_info.task_data),
             ompt_task_explicit | TASK_TYPE_DETAILS_FORMAT(new_taskdata), 0,
             OMPT_LOAD_RETURN_ADDRESS(gtid));
       }
+    } else {
+      // We are scheduling the continuation of an UNTIED task.
+      // Scheduling back to the parent task.
+      __ompt_task_finish(new_task,
+                         new_taskdata->ompt_task_info.scheduling_parent,
+                         ompt_task_others);
+      new_taskdata->ompt_task_info.frame.exit_frame = NULL;
+    }
   }
 #endif
 
   res = __kmp_omp_task(gtid, new_task, true);
 
   KA_TRACE(10, ("__kmpc_omp_task(exit): T#%d returning "
                 "TASK_CURRENT_NOT_QUEUED: loc=%p task=%p\n",
                 gtid, loc_ref, new_taskdata));
KA_TRACE(40,
               next_task_bounds.get_lower_offset(),
               next_task_bounds.get_upper_offset()));
     __kmp_omp_task(gtid, next_task, true); // schedule new task
     lower = upper + st; // adjust lower bound for the next iteration
   }
   // free the pattern task and exit
   __kmp_task_start(gtid, task, current_task); // make internal bookkeeping
   // do not execute the pattern task, just do internal bookkeeping
-  __kmp_task_finish(gtid, task, current_task);
+  __kmp_task_finish<false>(gtid, task, current_task);
 }
 
 // Structure to keep taskloop parameters for auxiliary task
 // kept in the shareds of the task structure.
 typedef struct __taskloop_params {
   kmp_task_t *task;
   kmp_uint64 *lb;
   kmp_uint64 *ub;
#endif
   } else { // st > 0
     tc = (upper - lower) / st + 1;
   }
   if (tc == 0) {
     KA_TRACE(20, ("__kmpc_taskloop(exit): T#%d zero-trip loop\n", gtid));
     // free the pattern task and exit
     __kmp_task_start(gtid, task, current_task);
     // do not execute anything for zero-trip loop
-    __kmp_task_finish(gtid, task, current_task);
+    __kmp_task_finish<false>(gtid, task, current_task);
     return;
   }
   if (num_tasks_min == 0)
     // TODO: can we choose better default heuristic?
     num_tasks_min =
         KMP_MIN(thread->th.th_team_nproc * 10, INITIAL_TASK_DEQUE_SIZE);
 
   // compute num_tasks/grainsize based on the input provided

openmp/trunk/runtime/test/ompt/tasks/task_types.c

 
 // explicit task with untied
 #pragma omp single
 #pragma omp task untied
     {
       // Output of thread_id is needed to know on which thread task is executed
       printf("%" PRIu64 ": explicit_untied\n", ompt_get_thread_data()->value);
       print_ids(0);
+      print_frame(1);
+      x++;
+#pragma omp taskyield
+      printf("%" PRIu64 ": explicit_untied(2)\n",
+             ompt_get_thread_data()->value);
+      print_ids(0);
+      print_frame(1);
+      x++;
+#pragma omp taskwait
+      printf("%" PRIu64 ": explicit_untied(3)\n",
+             ompt_get_thread_data()->value);
+      print_ids(0);
+      print_frame(1);
       x++;
     }
 // explicit task with final
 #pragma omp single
 #pragma omp task final(1)
     {
       print_ids(0);
       x++;
#pragma omp task
   // CHECK-SAME: codeptr_ra={{0x[0-f]+}}
   // CHECK-SAME: task_type=ompt_task_explicit|ompt_task_untied=268435460
   // CHECK-SAME: has_dependences=no
 
   // Here the thread_id cannot be taken from a schedule event as there
   // may be multiple of those
   // CHECK: [[THREAD_ID_3:[0-9]+]]: explicit_untied
   // CHECK: [[THREAD_ID_3]]: task level 0: parallel_id=[[PARALLEL_ID]]
-  // CHECK-SAME: task_id=[[EXPLICIT_UNTIED_TASK_ID]], exit_frame={{[^\,]*}}
-  // CHECK-SAME: reenter_frame=[[NULL]]
+  // CHECK-SAME: task_id=[[EXPLICIT_UNTIED_TASK_ID]]
+  // CHECK-SAME: exit_frame={{0x[0-f]+}}, reenter_frame=[[NULL]]
+  // CHECK-SAME: task_type=ompt_task_explicit|ompt_task_untied=268435460
+  // CHECK-SAME: thread_num={{[01]}}
+
+  // after taskyield
+  // CHECK: [[THREAD_ID_3_2:[0-9]+]]: explicit_untied(2)
+  // CHECK: [[THREAD_ID_3_2]]: task level 0: parallel_id=[[PARALLEL_ID]]
+  // CHECK-SAME: task_id=[[EXPLICIT_UNTIED_TASK_ID]]
+  // CHECK-SAME: exit_frame={{0x[0-f]+}}, reenter_frame=[[NULL]]
+  // CHECK-SAME: task_type=ompt_task_explicit|ompt_task_untied=268435460
+  // CHECK-SAME: thread_num={{[01]}}
+
+  // after taskwait
+  // CHECK: [[THREAD_ID_3_3:[0-9]+]]: explicit_untied(3)
+  // CHECK: [[THREAD_ID_3_3]]: task level 0: parallel_id=[[PARALLEL_ID]]
+  // CHECK-SAME: task_id=[[EXPLICIT_UNTIED_TASK_ID]]
+  // CHECK-SAME: exit_frame={{0x[0-f]+}}, reenter_frame=[[NULL]]
   // CHECK-SAME: task_type=ompt_task_explicit|ompt_task_untied=268435460
   // CHECK-SAME: thread_num={{[01]}}
 
   // explicit task with final
   // CHECK: {{^[0-9]+}}: ompt_event_task_create: parent_task_id={{[0-9]+}}
   // CHECK-SAME: parent_task_frame.exit={{0x[0-f]+}}
   // CHECK-SAME: parent_task_frame.reenter={{0x[0-f]+}}
   // CHECK-SAME: new_task_id=[[EXPLICIT_FINAL_TASK_ID:[0-9]+]]