Fix performance regression in SPEC kdtree test

[jlpeyton]

Summary

Make __ompt_implicit_task_end a static function and remove the inline part. This fixes small regression in SPEC kdtree benchmark. Also reformat some of __ompt_implicit_task_end.

[Hahnfeld]

runtime/src/kmp_wait_release.h

254–261

Are you sure that you want to remove this lookup? This means always passing NULL AFAICS

[AndreyChurbanov]

runtime/src/kmp_wait_release.h

254–261

Once the value of pId is never used it should be OK to have it NULL for now. Increased code size impacts performance of some tests, so we try to eliminate unused code when possible.

[Hahnfeld]

runtime/src/kmp_wait_release.h

264

Well, it's used in this event.

[AndreyChurbanov]

runtime/src/kmp_wait_release.h

264

Here is the complete code of the event you mentioned:

static inline void __ompt_implicit_task_end(kmp_info_t *this_thr,
                                          ompt_state_t ompt_state,
                                          ompt_data_t *tId,
                                          ompt_data_t *pId) {
int ds_tid = this_thr->th.th_info.ds.ds_tid;
if (ompt_state == ompt_state_wait_barrier_implicit) {
  this_thr->th.ompt_thread_info.state = ompt_state_overhead;
  void *codeptr = NULL;
  if (ompt_enabled.ompt_callback_sync_region_wait) {
    ompt_callbacks.ompt_callback(ompt_callback_sync_region_wait)(
        ompt_sync_region_barrier, ompt_scope_end, NULL, tId, codeptr);
  }
  if (ompt_enabled.ompt_callback_sync_region) {
    ompt_callbacks.ompt_callback(ompt_callback_sync_region)(
        ompt_sync_region_barrier, ompt_scope_end, NULL, tId, codeptr);
  }
  if (!KMP_MASTER_TID(ds_tid)) {
    if (ompt_enabled.ompt_callback_implicit_task) {
      ompt_callbacks.ompt_callback(ompt_callback_implicit_task)(
          ompt_scope_end, NULL, tId, 0, ds_tid);
    }
    // return to idle state
    this_thr->th.ompt_thread_info.state = ompt_state_idle;
  } else {
    this_thr->th.ompt_thread_info.state = ompt_state_overhead;
  }
}
}

The pId parameter is not used here.

[Hahnfeld]

runtime/src/kmp_wait_release.h

264

Ah, didn't look into the function. Can we then remove the argument and the variable completely?

[protze.joachim]

runtime/src/kmp_wait_release.h

264

I would also suggest to completely remove pId here.
The final OpenMP 5.0 spec says, that parallel_data here is always NULL, so it is not needed anymore.

[jlpeyton]

Removed pId variable

[AndreyChurbanov]

LGTM

[protze.joachim]

LGTM

runtime/src/kmp_wait_release.h

public:
   int execute_tasks(kmp_info_t *this_thr, kmp_int32 gtid, int final_spin,
                     int *thread_finished
                     USE_ITT_BUILD_ARG(void * itt_sync_obj), kmp_int32
                     is_constrained);
   */
 };
 
 #if OMPT_SUPPORT
-static inline void __ompt_implicit_task_end(kmp_info_t *this_thr,
-                                            omp_state_t omp_state,
-                                            ompt_data_t *tId,
-                                            ompt_data_t *pId) {
+OMPT_NOINLINE
+static void __ompt_implicit_task_end(kmp_info_t *this_thr,
+                                     omp_state_t omp_state, ompt_data_t *tId,
   int ds_tid = this_thr->th.th_info.ds.ds_tid;
   if (omp_state == omp_state_wait_barrier_implicit) {
     this_thr->th.ompt_thread_info.state = omp_state_overhead;
 #if OMPT_OPTIONAL
     void *codeptr = NULL;
     if (ompt_enabled.ompt_callback_sync_region_wait) {
       ompt_callbacks.ompt_callback(ompt_callback_sync_region_wait)(
           ompt_sync_region_barrier, ompt_scope_end, NULL, tId, codeptr);
#if OMPT_SUPPORT
   ompt_data_t *tId;
   if (ompt_enabled.enabled) {
     ompt_entry_state = this_thr->th.ompt_thread_info.state;
     if (!final_spin || ompt_entry_state != omp_state_wait_barrier_implicit ||
         KMP_MASTER_TID(this_thr->th.th_info.ds.ds_tid)) {
       ompt_lw_taskteam_t *team =
           this_thr->th.th_team->t.ompt_serialized_team_info;
       if (team) {
-        pId = &(team->ompt_team_info.parallel_data);
         tId = &(team->ompt_task_info.task_data);
       } else {
-        pId = OMPT_CUR_TEAM_DATA(this_thr);
         tId = OMPT_CUR_TASK_DATA(this_thr);
       }
     } else {
-      pId = NULL;
       tId = &(this_thr->th.ompt_thread_info.task_data);
     }
-        if (final_spin && (__kmp_tasking_mode == tskm_immediate_exec ||
-                           this_thr->th.th_task_team == NULL)) {
       // implicit task is done. Either no taskqueue, or task-team finished
       __ompt_implicit_task_end(this_thr, ompt_entry_state, tId, pId);
     }
   }
 #endif
 
   // Setup for waiting
   KMP_INIT_YIELD(spins);
 
   if (__kmp_dflt_blocktime != KMP_MAX_BLOCKTIME) {