Fix assertion that arises when waiting for proxy tasks on runtime shutdown

[Hahnfeld]

Summary

This only triggered when built in debug mode with OMPT enabled:
__kmp_wait_template expected the state of the current thread to be either ompt_state_idle or ompt_state_wait_barrier{,_implicit,_explicit}.

[AndreyChurbanov]

LGTM

[jmellorcrummey]

The problem is not the OMPT callbacks; the problem was the state in which they were reached.

Not making the OMPT callbacks while waiting for shutdown is a mistake. If there is waiting happening, tools want to know.

I'm not sure how the state of the runtime should be classified at kmp_runtime.c:3992 (e.g., overhead, task wait, barrier?), but undefined is not a good option. I think that there needs to be a thoughtful choice for the state in kmp_runtime.c:3992 and then corresponding changes to kmp_wait_template.h.

If the state should not be idle or barrier at kmp_runtime.c:3992, then another if/else should be added in the changed regions of kmp_wait_template.h to properly handle another state in a meaningful fashion.

[Hahnfeld]

In D12754#243824, @jmellorcrummey wrote:

The problem is not the OMPT callbacks; the problem was the state in which they were reached.

Not making the OMPT callbacks while waiting for shutdown is a mistake. If there is waiting happening, tools want to know.

I'm not sure how the state of the runtime should be classified at kmp_runtime.c:3992 (e.g., overhead, task wait, barrier?), but undefined is not a good option. I think that there needs to be a thoughtful choice for the state in kmp_runtime.c:3992 and then corresponding changes to kmp_wait_template.h.

If the state should not be idle or barrier at kmp_runtime.c:3992, then another if/else should be added in the changed regions of kmp_wait_template.h to properly handle another state in a meaningful fashion.

Sorry for not giving reasons about the decision:
First, this case only happens when a #pragma omp target nowait exceeds the runtime of the master thread:

int main()
{
	#pragma omp target nowait
	{
		sleep(10);
	}

	return EXIT_SUCCESS;
}

(In my opinion this can't do anything meaningful in real code because the results of that target region cannot be handled anywhere on the host)

When leaving the main function the runtime library begins shutdown. So in my understanding the state is neither

• ompt_state_wait_barrier because there is only one thread waiting for the proxy tasks to finish nor
• ompt_state_idle because the thread isn't "waiting to work on an OpenMP parallel region" as the TR describes

In the standard's view the thread is already finished because it won't do any user work in its remaining lifetime. That's also why I think tools might not be really interested in those possible events.

But maybe I've overlooked a matching state. Or we could define a new one...

runtime/src/kmp_runtime.c

__kmp_unregister_root_current_thread( int gtid )
     KMP_MB();
 
 #if OMP_41_ENABLED
    kmp_info_t * thread = __kmp_threads[gtid];
    kmp_team_t * team = thread->th.th_team;
    kmp_task_team_t *   task_team = thread->th.th_task_team;
 
    // we need to wait for the proxy tasks before finishing the thread
-   if ( task_team != NULL && task_team->tt.tt_found_proxy_tasks )
+   if ( task_team != NULL && task_team->tt.tt_found_proxy_tasks ) {
+#if OMPT_SUPPORT
+        // the runtime is shutting down so we won't report any events
+        thread->th.ompt_thread_info.state = ompt_state_undefined;
+#endif
         __kmp_task_team_wait(thread, team, NULL );
+   }
 #endif
 
     __kmp_reset_root(gtid, root);
 
     /* free up this thread slot */
     __kmp_gtid_set_specific( KMP_GTID_DNE );
 #ifdef KMP_TDATA_GTID
     __kmp_gtid = KMP_GTID_DNE;

runtime/src/kmp_wait_release.h

static inline void __kmp_wait_template(kmp_info_t *this_thr, C *flag, int final_spin
     if (flag->done_check()) {
         KMP_FSYNC_SPIN_ACQUIRED(spin);
         return;
     }
     th_gtid = this_thr->th.th_info.ds.ds_gtid;
     KA_TRACE(20, ("__kmp_wait_sleep: T#%d waiting for flag(%p)\n", th_gtid, flag));
 
 #if OMPT_SUPPORT && OMPT_BLAME
-    if (ompt_status == ompt_status_track_callback) {
-        if (this_thr->th.ompt_thread_info.state == ompt_state_idle){
+    ompt_state_t ompt_state = this_thr->th.ompt_thread_info.state;
+    if (ompt_status == ompt_status_track_callback &&
+        ompt_state != ompt_state_undefined) {
+        if (ompt_state == ompt_state_idle) {
             if (ompt_callbacks.ompt_callback(ompt_event_idle_begin)) {
                 ompt_callbacks.ompt_callback(ompt_event_idle_begin)(th_gtid + 1);
             }
         } else if (ompt_callbacks.ompt_callback(ompt_event_wait_barrier_begin)) {
-            KMP_DEBUG_ASSERT(this_thr->th.ompt_thread_info.state == ompt_state_wait_barrier ||
-                             this_thr->th.ompt_thread_info.state == ompt_state_wait_barrier_implicit ||
-                             this_thr->th.ompt_thread_info.state == ompt_state_wait_barrier_explicit);
+            KMP_DEBUG_ASSERT(ompt_state == ompt_state_wait_barrier ||
+                             ompt_state == ompt_state_wait_barrier_implicit ||
+                             ompt_state == ompt_state_wait_barrier_explicit);
 
             ompt_lw_taskteam_t* team = this_thr->th.th_team->t.ompt_serialized_team_info;
             ompt_parallel_id_t pId;
             ompt_task_id_t tId;
             if (team){
                 pId = team->ompt_team_info.parallel_id;
                 tId = team->ompt_task_info.task_id;
             } else {
while (flag->notdone_check()) {
             if (__kmp_global.g.g_abort)
                 __kmp_abort_thread();
             break;
         }
         // TODO: If thread is done with work and times out, disband/free
     }
 
 #if OMPT_SUPPORT && OMPT_BLAME
-    if (ompt_status == ompt_status_track_callback) {
-        if (this_thr->th.ompt_thread_info.state == ompt_state_idle){
+    if (ompt_status == ompt_status_track_callback &&
+        ompt_state != ompt_state_undefined) {
+        if (ompt_state == ompt_state_idle) {
             if (ompt_callbacks.ompt_callback(ompt_event_idle_end)) {
                 ompt_callbacks.ompt_callback(ompt_event_idle_end)(th_gtid + 1);
             }
         } else if (ompt_callbacks.ompt_callback(ompt_event_wait_barrier_end)) {
-            KMP_DEBUG_ASSERT(this_thr->th.ompt_thread_info.state == ompt_state_wait_barrier ||
-                             this_thr->th.ompt_thread_info.state == ompt_state_wait_barrier_implicit ||
-                             this_thr->th.ompt_thread_info.state == ompt_state_wait_barrier_explicit);
+            KMP_DEBUG_ASSERT(ompt_state == ompt_state_wait_barrier ||
+                             ompt_state == ompt_state_wait_barrier_implicit ||
+                             ompt_state == ompt_state_wait_barrier_explicit);
 
             ompt_lw_taskteam_t* team = this_thr->th.th_team->t.ompt_serialized_team_info;
             ompt_parallel_id_t pId;
             ompt_task_id_t tId;
             if (team){
                 pId = team->ompt_team_info.parallel_id;
                 tId = team->ompt_task_info.task_id;
             } else {