[OMPT] Fix the lock id provided for ordered events

[protze.joachim]

Summary

With the current implementation, ompt_thread_info.wait_id is set to 0 once the thread got the lock. This value is used to provide the lock id for the event calls.

With this patch, the lock id as provided by the function argument is used. Now all event callbacks provide a consistent lock id.

[jmellorcrummey]

Neither this patch nor the original code address the problem that needs attention.

There is a synchronization object used to perform ordered synchronization. Neither loc nor the wait_id initialized to loc represent it. If you look at how __kmp_ordered is called from kmp_gsupport.c (which is used for code compiled with the GNU compilers), you will begin to understand the problem.

Here's how it is called in KMP_API_NAME_GOMP_ORDERED_START:

MKLOC(loc, "GOMP_ordered_start");
KA_TRACE(20, ("GOMP_ordered_start: T#%d\n", gtid));
__kmpc_ordered(&loc, gtid);

Below is the definition for MKLOC:

#define MKLOC(loc,routine) static ident_t (loc) = {0, KMP_IDENT_KMPC, 0, 0, ";unknown;unknown;0;0;;" };

Thus, in __ kmpc_ordered, loc refers to the address of a static variable in the runtime library. When __ kmp_ordered_end is called, a different loc is passed. Neother represents the synchronization object that is used to maintain ordered synchronization. That synchronization object is only visible inside &_&_kmp_dispatch_deo as the first argument to the __kmp_wait_yield template it calls

__kmp_wait_yield< UT >( &sh->u.s.ordered_iteration, lower, __kmp_ge< UT > USE_ITT_BUILD_ARG( NULL ) );

[jlpeyton]

Updating the status of this review to "Needs changes". The fundamental issue has apparently not been addressed according to John Mellor-Crummy.

[protze.joachim]

Subsumed by D38185

runtime/src/kmp_csupport.c

#if OMPT_SUPPORT && OMPT_TRACE
     if (ompt_enabled) {
         /* OMPT state update */
         th->th.ompt_thread_info.state = ompt_state_work_parallel;
         th->th.ompt_thread_info.wait_id = 0;
 
         /* OMPT event callback */
         if (ompt_callbacks.ompt_callback(ompt_event_acquired_ordered)) {
             ompt_callbacks.ompt_callback(ompt_event_acquired_ordered)(
-                th->th.ompt_thread_info.wait_id);
+                (uint64_t) loc);
         }
     }
 #endif
 
 #if USE_ITT_BUILD
     __kmp_itt_ordered_start( gtid );
 #endif /* USE_ITT_BUILD */
 }
if ( th -> th.th_dispatch -> th_dxo_fcn != 0 )
         (*th->th.th_dispatch->th_dxo_fcn)( & gtid, & cid, loc );
     else
         __kmp_parallel_dxo( & gtid, & cid, loc );
 
 #if OMPT_SUPPORT && OMPT_BLAME
     if (ompt_enabled &&
         ompt_callbacks.ompt_callback(ompt_event_release_ordered)) {
         ompt_callbacks.ompt_callback(ompt_event_release_ordered)(
-            th->th.ompt_thread_info.wait_id);
+            (uint64_t) loc);
     }
 #endif
 }
 
 #if KMP_USE_DYNAMIC_LOCK
 
 static __forceinline kmp_indirect_lock_t *
 __kmp_get_indirect_csptr(kmp_critical_name * crit, ident_t const * loc, kmp_int32 gtid, kmp_dyna_lockseq_t seq)