[OMPT] Fix OMPT callbacks for the taskloop construct and add testcase

[sconvent]

Summary

Fix the order of callbacks related to the taskloop construct.
Add the iteration_count to work callbacks (according to the spec).
Use kmpc_omp_task() instead of kmp_omp_task() to include OMPT callbacks.
Add a testcase.

[sconvent]

Use kmpc_omp_task() instead of changing kmp_omp_task().

[protze.joachim]

runtime/src/kmp_tasking.cpp

3754

same as below

3910

We somehow need to pass a return-address in here. So probably a new variant of __kmpc_omp_task, which takes the address and adds it to the ompt-callback invocation?

3955

Shouldn't there be st(ride) in the formula?

[AndreyChurbanov]

runtime/src/kmp_tasking.cpp

3955

Why not use calculated below iterations count - tc? It is computed at lines 3980 - 3995, including the check against zero.

[sconvent]

runtime/src/kmp_tasking.cpp

3955

There is a difference between the iteration count and the task count.
From the spec (TR6 page 423):

For a taskloop construct, count represents the number of iterations in the iteration space, which may be the result of collapsing several associated loops.

tc seems to be the task count, not the iteration count. And st(ride) is not needed for the calculation of the iteration count, right?

[AndreyChurbanov]

runtime/src/kmp_tasking.cpp

3955

No, the tc states for "trace count" which is exactly iterations count. And the stride is important here.
E.g. for(i=0; i<10; i+=3) causes the iteration count to be 4 not 10, because of stride=3.
For task count another parameter used - it is grainsize.

[sconvent]

Use tc for the iteration count

runtime/src/kmp_tasking.cpp

3955

You're right, I updated the diff to use tc.

[Hahnfeld]

In D47709#1123519, @sconvent wrote:

Use tc for the iteration count

Can you add another loop in the test case where this makes a difference? Maybe just Andrey's example

[sconvent]

Use a stride of 3 in testcase loop

[protze.joachim]

taskloop is actually a tasking construct and explicitly no worksharing construct. So, please move the test in the task directory.

[sconvent]

Add support for OMPT return addresses.
Also test them.

[sconvent]

In D47709#1125178, @protze.joachim wrote:

taskloop is actually a tasking construct and explicitly no worksharing construct. So, please move the test in the task directory.

Done

[hbae]

runtime/src/kmp_tasking.cpp

1602

Can we change the function name to __kmp_omp_taskloop_task?
__kmpc_* is usually for ABI functions.

3918

I think it is better to keep the original code (__kmp_omp_task() call) at this line.

[protze.joachim]

I apply changes requested by hbae on commit.

LGTM

[Hahnfeld]

openmp/trunk/runtime/src/kmp_tasking.cpp

4018 ↗

(On Diff #157717)

This call wasn't correctly reverted prior to commit, @gtbercea is fixing this in D50001

runtime/src/kmp_tasking.cpp

static kmp_int32 __kmpc_omp_taskwait_template(ident_t *loc_ref, kmp_int32 gtid,
   kmp_info_t *thread;
   int thread_finished = FALSE;
   KMP_SET_THREAD_STATE_BLOCK(TASKWAIT);
 
   KA_TRACE(10, ("__kmpc_omp_taskwait(enter): T#%d loc=%p\n", gtid, loc_ref));
 
   if (__kmp_tasking_mode != tskm_immediate_exec) {
     thread = __kmp_threads[gtid];
     taskdata = thread->th.th_current_task;
 
 #if OMPT_SUPPORT && OMPT_OPTIONAL
     ompt_data_t *my_task_data;
     ompt_data_t *my_parallel_data;
 
     if (ompt) {
       my_task_data = &(taskdata->ompt_task_info.task_data);
       my_parallel_data = OMPT_CUR_TEAM_DATA(thread);

 
 #ifdef BUILD_TIED_TASK_STACK
         // GEH: Figure out if this is the right thing to do
         for (i = maxthreads; i < nthreads; i++) {
           kmp_thread_data_t *thread_data = &(*threads_data_p)[i];
           __kmp_init_task_stack(__kmp_gtid_from_thread(thread), thread_data);
         }
 #endif // BUILD_TIED_TASK_STACK
-        // Install the new data and free the old data
         (*threads_data_p) = new_data;
         __kmp_free(old_data);
       } else {
         KE_TRACE(10, ("__kmp_realloc_task_threads_data: T#%d allocating "
                       "threads data for task_team %p, size = %d\n",
                       __kmp_gtid_from_thread(thread), task_team, nthreads));
         // Make the initial allocate for threads_data array, and zero entries
         // Cannot use __kmp_thread_calloc() because threads not around for
for (i = 0; i < num_tasks; ++i) {
     if (ptask_dup != NULL) // set lastprivate flag, construct fistprivates, etc.
       ptask_dup(next_task, task, lastpriv);
     KA_TRACE(40,
              ("__kmp_taskloop_linear: T#%d; task #%llu: task %p: lower %lld, "
               "upper %lld stride %lld, (offsets %p %p)\n",
               gtid, i, next_task, lower, upper, st,
               next_task_bounds.get_lower_offset(),
               next_task_bounds.get_upper_offset()));
-    __kmp_omp_task(gtid, next_task, true); // schedule new task
+    __kmpc_omp_task(NULL, gtid, next_task); // 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);
 }
 
#endif
   p->task_dup = task_dup;
   p->st = st;
   p->ub_glob = ub_glob;
   p->num_tasks = n_tsk1;
   p->grainsize = grainsize;
   p->extras = ext1;
   p->tc = tc1;
   p->num_t_min = num_t_min;
-  __kmp_omp_task(gtid, new_task, true); // schedule new task
+  __kmpc_omp_task(NULL, gtid, new_task); // schedule new task
 
   // execute the 1st half of current subrange
   if (n_tsk0 > num_t_min)
     __kmp_taskloop_recur(loc, gtid, task, lb, ub, st, ub_glob, n_tsk0, gr_size0,
                          ext0, tc0, num_t_min, task_dup);
   else
     __kmp_taskloop_linear(loc, gtid, task, lb, ub, st, ub_glob, n_tsk0,
                           gr_size0, ext0, tc0, task_dup);
 
   KA_TRACE(40, ("__kmpc_taskloop_recur(exit): T#%d\n", gtid));
 }
 
 /*!
 @ingroup TASKING
 @param loc       Source location information
 @param gtid      Global thread ID

 Execute the taskloop construct.
 */
 void __kmpc_taskloop(ident_t *loc, int gtid, kmp_task_t *task, int if_val,
                      kmp_uint64 *lb, kmp_uint64 *ub, kmp_int64 st, int nogroup,
                      int sched, kmp_uint64 grainsize, void *task_dup) {
   kmp_taskdata_t *taskdata = KMP_TASK_TO_TASKDATA(task);
   KMP_DEBUG_ASSERT(task != NULL);
 
+  if (nogroup == 0) {
+#if OMPT_SUPPORT && OMPT_OPTIONAL
+    OMPT_STORE_RETURN_ADDRESS(gtid);
+#endif
+    __kmpc_taskgroup(loc, gtid);
+  }
+
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   ompt_team_info_t *team_info = __ompt_get_teaminfo(0, NULL);
   ompt_task_info_t *task_info = __ompt_get_task_info_object(0);
+  kmp_uint64 iteration_count = *lb < *ub ? *ub - *lb + 1 : *lb - *ub + 1;
   if (ompt_enabled.ompt_callback_work) {
     ompt_callbacks.ompt_callback(ompt_callback_work)(
         ompt_work_taskloop, ompt_scope_begin, &(team_info->parallel_data),
-        &(task_info->task_data), 0, OMPT_GET_RETURN_ADDRESS(0));
+        &(task_info->task_data), iteration_count, OMPT_GET_RETURN_ADDRESS(0));
   }
 #endif
 
-  if (nogroup == 0) {
-#if OMPT_SUPPORT && OMPT_OPTIONAL
-    OMPT_STORE_RETURN_ADDRESS(gtid);
-#endif
-    __kmpc_taskgroup(loc, gtid);
-  }
-
   // =========================================================================
   // calculate loop parameters
   kmp_taskloop_bounds_t task_bounds(task, lb, ub);
   kmp_uint64 tc;
   // compiler provides global bounds here
   kmp_uint64 lower = task_bounds.get_lb();
   kmp_uint64 upper = task_bounds.get_ub();
   kmp_uint64 ub_glob = upper; // global upper used to calc lastprivate flag
KA_TRACE(20, ("__kmpc_taskloop: T#%d, go linear: tc %llu, #tasks %llu"
                   gtid, tc, num_tasks, num_tasks_min, grainsize, extras));
 #if OMPT_SUPPORT && OMPT_OPTIONAL
     OMPT_STORE_RETURN_ADDRESS(gtid);
 #endif
     __kmp_taskloop_linear(loc, gtid, task, lb, ub, st, ub_glob, num_tasks,
                           grainsize, extras, tc, task_dup);
   }
 
-  if (nogroup == 0) {
-#if OMPT_SUPPORT && OMPT_OPTIONAL
-    OMPT_STORE_RETURN_ADDRESS(gtid);
-#endif
-    __kmpc_end_taskgroup(loc, gtid);
-  }
 #if OMPT_SUPPORT && OMPT_OPTIONAL
   if (ompt_enabled.ompt_callback_work) {
     ompt_callbacks.ompt_callback(ompt_callback_work)(
         ompt_work_taskloop, ompt_scope_end, &(team_info->parallel_data),
-        &(task_info->task_data), 0, OMPT_GET_RETURN_ADDRESS(0));
+        &(task_info->task_data), iteration_count, OMPT_GET_RETURN_ADDRESS(0));
   }
 #endif
+
+  if (nogroup == 0) {
+#if OMPT_SUPPORT && OMPT_OPTIONAL
+    OMPT_STORE_RETURN_ADDRESS(gtid);
+#endif
+    __kmpc_end_taskgroup(loc, gtid);
+  }
   KA_TRACE(20, ("__kmpc_taskloop(exit): T#%d\n", gtid));
 }
 
 #endif

runtime/test/ompt/worksharing/taskloop.c

+// RUN: %libomp-compile && %libomp-run | FileCheck %s
+// REQUIRES: ompt
+#include "callback.h"
+#include <omp.h>
+
+int main() {
+  unsigned int i, j, x;
+
+#pragma omp parallel num_threads(2)
+#pragma omp master
+#pragma omp taskloop
+  for (j = 0; j < 2; j++) {
+    x++;
+  }
+
+  // CHECK: 0: NULL_POINTER=[[NULL:.*$]]
+
+  // CHECK: {{^}}[[MASTER_ID:[0-9]+]]: ompt_event_parallel_begin:
+  // CHECK-SAME: parent_task_id={{[0-9]+}}
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID:[0-9]+]]
+  // CHECK-SAME: requested_team_size=2
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_implicit_task_begin:
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID]]
+  // CHECK-SAME: task_id=[[IMPLICIT_TASK_ID1:[0-9]+]]
+  // CHECK-SAME: team_size=2, thread_num=0
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_taskgroup_begin:
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID]], task_id=[[IMPLICIT_TASK_ID1]]
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_taskloop_begin:
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID]]
+  // CHECK-SAME: parent_task_id=[[IMPLICIT_TASK_ID1]]
+  // CHECK-SAME: codeptr_ra=0x{{[0-f]+}}, count=2
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_task_create:
+  // CHECK-SAME: parent_task_id=[[IMPLICIT_TASK_ID1]]
+  // CHECK-SAME: new_task_id=[[TASK_ID1:[0-9]+]]
+  // CHECK-SAME: task_type=ompt_task_explicit=4
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_task_create:
+  // CHECK-SAME: parent_task_id=[[IMPLICIT_TASK_ID1]]
+  // CHECK-SAME: new_task_id=[[TASK_ID2:[0-9]+]]
+  // CHECK-SAME: task_type=ompt_task_explicit=4
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_taskloop_end:
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID]]
+  // CHECK-SAME: parent_task_id=[[IMPLICIT_TASK_ID1]]
+  // CHECK-SAME: count=2
+  // CHECK-DAG: {{^}}[[MASTER_ID]]: ompt_event_wait_taskgroup_begin:
+  // CHECK-DAG: {{^.*}}first_task_id={{[0-9]+}}, second_task_id=[[TASK_ID1]]
+  // CHECK-DAG: {{^.*}}first_task_id=[[TASK_ID1]], second_task_id={{[0-9]+}}
+  // CHECK-DAG: {{^.*}}first_task_id={{[0-9]+}}, second_task_id=[[TASK_ID2]]
+  // CHECK-DAG: {{^.*}}first_task_id=[[TASK_ID2]], second_task_id={{[0-9]+}}
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_wait_taskgroup_end:
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID]], task_id=[[IMPLICIT_TASK_ID1]]
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_taskgroup_end:
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID]], task_id=[[IMPLICIT_TASK_ID1]]
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_implicit_task_end: parallel_id=0
+  // CHECK-SAME: task_id=[[IMPLICIT_TASK_ID1]], team_size=2, thread_num=0
+  // CHECK: {{^}}[[MASTER_ID]]: ompt_event_parallel_end:
+  // CHECK-SAME: parallel_id=[[PARALLEL_ID]]
+
+  return 0;
+}