[OpenMP] Clean up for hidden helper task

[tianshilei1992]

Summary

This patch makes some clean up for code of hidden helper task.

[protze.joachim]

openmp/runtime/src/kmp_taskdeps.h

147–151

This shouldn't be the encountering tid, but any random number in [0,teamsize).
The value is used as an index to the array of threads for the team to which the task binds.

openmp/runtime/src/kmp_tasking.cpp

3953

Either add an assertion (start<nthreads) or limit the value to nthreads

[tianshilei1992]

rebase and fix comments

openmp/runtime/src/kmp_taskdeps.h

147–151

__kmp_tid_from_gtid can make sure we get the right tid, aka [0, team size).

[tianshilei1992]

ping

[tianshilei1992]

rebase

[protze.joachim]

lgtm.
Please run git clang-format before committing to address the per-merge checks.

[tianshilei1992]

git clang-format

openmp/runtime/src/kmp.h

std::atomic<kmp_int32>
       td_incomplete_child_tasks; /* Child tasks not yet complete */
   kmp_taskgroup_t
       *td_taskgroup; // Each task keeps pointer to its current taskgroup
   kmp_dephash_t
       *td_dephash; // Dependencies for children tasks are tracked from here
   kmp_depnode_t
       *td_depnode; // Pointer to graph node if this task has dependencies
   kmp_task_team_t *td_task_team;
-  // The global thread id of the encountering thread. We need it because when a
-  // regular task depends on a hidden helper task, and the hidden helper task
-  // is finished on a hidden helper thread, it will call __kmp_release_deps to
-  // release all dependences. If now the task is a regular task, we need to pass
-  // the encountering gtid such that the task will be picked up and executed by
-  // its encountering team instead of hidden helper team.
-  kmp_int32 encountering_gtid;
   size_t td_size_alloc; // Size of task structure, including shareds etc.
 #if defined(KMP_GOMP_COMPAT)
   // 4 or 8 byte integers for the loop bounds in GOMP_taskloop
   kmp_int32 td_size_loop_bounds;
 #endif
   kmp_taskdata_t *td_last_tied; // keep tied task for task scheduling constraint
 #if defined(KMP_GOMP_COMPAT)
   // GOMP sends in a copy function for copy constructors

openmp/runtime/src/kmp_taskdeps.h

if (npredecessors == 0) {
         if (KMP_HIDDEN_HELPER_THREAD(gtid)) {
           // Hidden helper thread can only execute hidden helper tasks
           KMP_ASSERT(task->td_flags.hidden_helper);
           next_taskdata = KMP_TASK_TO_TASKDATA(successor->dn.task);
           // If the dependent task is a regular task, we need to push to its
           // encountering thread's queue; otherwise, it can be pushed to its own
           // queue.
           if (!next_taskdata->td_flags.hidden_helper) {
-            __kmpc_give_task(
-                successor->dn.task,
-                __kmp_tid_from_gtid(next_taskdata->encountering_gtid));
+            kmp_int32 encountering_gtid =
+                next_taskdata->td_alloc_thread->th.th_info.ds.ds_gtid;
+            kmp_int32 encountering_tid = __kmp_tid_from_gtid(encountering_gtid);
+            __kmpc_give_task(successor->dn.task, encountering_tid);
           } else {
             __kmp_omp_task(gtid, successor->dn.task, false);
           }
         } else {
           __kmp_omp_task(gtid, successor->dn.task, false);
         }
       }
     }
 

openmp/runtime/src/kmp_tasking.cpp

 // returns: a pointer to the allocated kmp_task_t structure (task).
 kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, kmp_int32 gtid,
                              kmp_tasking_flags_t *flags,
                              size_t sizeof_kmp_task_t, size_t sizeof_shareds,
                              kmp_routine_entry_t task_entry) {
   kmp_task_t *task;
   kmp_taskdata_t *taskdata;
   kmp_info_t *thread = __kmp_threads[gtid];
-  kmp_info_t *encountering_thread = thread;
   kmp_team_t *team = thread->th.th_team;
   kmp_taskdata_t *parent_task = thread->th.th_current_task;
   size_t shareds_offset;
 
   if (UNLIKELY(!TCR_4(__kmp_init_middle)))
     __kmp_middle_initialize();
 
   if (flags->hidden_helper) {
     if (__kmp_enable_hidden_helper) {
       if (!TCR_4(__kmp_init_hidden_helper))
         __kmp_hidden_helper_initialize();
-
-      // For a hidden helper task encountered by a regular thread, we will push
-      // the task to the (gtid%__kmp_hidden_helper_threads_num)-th hidden helper
-      // thread.
-      if (!KMP_HIDDEN_HELPER_THREAD(gtid)) {
-        thread = __kmp_threads[KMP_GTID_TO_SHADOW_GTID(gtid)];
-        // We don't change the parent-child relation for hidden helper task as
-        // we need that to do per-task-region synchronization.
-      }
     } else {
       // If the hidden helper task is not enabled, reset the flag to FALSE.
       flags->hidden_helper = FALSE;
     }
   }
 
   KA_TRACE(10, ("__kmp_task_alloc(enter): T#%d loc=%p, flags=(0x%x) "
                 "sizeof_task=%ld sizeof_shared=%ld entry=%p\n",
                 gtid, loc_ref, *((kmp_int32 *)flags), sizeof_kmp_task_t,
                 sizeof_shareds, task_entry));
 
   KMP_DEBUG_ASSERT(parent_task);
   if (parent_task->td_flags.final) {
     if (flags->merged_if0) {
     }
     flags->final = 1;
   }
 
   if (flags->tiedness == TASK_UNTIED && !team->t.t_serialized) {
     // Untied task encountered causes the TSC algorithm to check entire deque of
     // the victim thread. If no untied task encountered, then checking the head
     // of the deque should be enough.
-    KMP_CHECK_UPDATE(
-        encountering_thread->th.th_task_team->tt.tt_untied_task_encountered, 1);
+    KMP_CHECK_UPDATE(thread->th.th_task_team->tt.tt_untied_task_encountered, 1);
   }
 
   // Detachable tasks are not proxy tasks yet but could be in the future. Doing
   // the tasking setup
   // when that happens is too late.
   if (UNLIKELY(flags->proxy == TASK_PROXY ||
                flags->detachable == TASK_DETACHABLE || flags->hidden_helper)) {
     if (flags->proxy == TASK_PROXY) {
       flags->tiedness = TASK_UNTIED;
       flags->merged_if0 = 1;
     }
     /* are we running in a sequential parallel or tskm_immediate_exec... we need
        tasking support enabled */
-    if ((encountering_thread->th.th_task_team) == NULL) {
+    if ((thread->th.th_task_team) == NULL) {
       /* This should only happen if the team is serialized
           setup a task team and propagate it to the thread */
       KMP_DEBUG_ASSERT(team->t.t_serialized);
       KA_TRACE(30,
                ("T#%d creating task team in __kmp_task_alloc for proxy task\n",
                 gtid));
-      __kmp_task_team_setup(
-          encountering_thread, team,
-          1); // 1 indicates setup the current team regardless of nthreads
-      encountering_thread->th.th_task_team =
-          team->t.t_task_team[encountering_thread->th.th_task_state];
+      // 1 indicates setup the current team regardless of nthreads
+      __kmp_task_team_setup(thread, team, 1);
+      thread->th.th_task_team = team->t.t_task_team[thread->th.th_task_state];
     }
-    kmp_task_team_t *task_team = encountering_thread->th.th_task_team;
+    kmp_task_team_t *task_team = thread->th.th_task_team;
 
     /* tasking must be enabled now as the task might not be pushed */
     if (!KMP_TASKING_ENABLED(task_team)) {
       KA_TRACE(
           30,
           ("T#%d enabling tasking in __kmp_task_alloc for proxy task\n", gtid));
-      __kmp_enable_tasking(task_team, encountering_thread);
-      kmp_int32 tid = encountering_thread->th.th_info.ds.ds_tid;
+      __kmp_enable_tasking(task_team, thread);
+      kmp_int32 tid = thread->th.th_info.ds.ds_tid;
       kmp_thread_data_t *thread_data = &task_team->tt.tt_threads_data[tid];
       // No lock needed since only owner can allocate
       if (thread_data->td.td_deque == NULL) {
-        __kmp_alloc_task_deque(encountering_thread, thread_data);
+        __kmp_alloc_task_deque(thread, thread_data);
       }
     }
 
     if ((flags->proxy == TASK_PROXY || flags->detachable == TASK_DETACHABLE) &&
         task_team->tt.tt_found_proxy_tasks == FALSE)
       TCW_4(task_team->tt.tt_found_proxy_tasks, TRUE);
     if (flags->hidden_helper &&
         task_team->tt.tt_hidden_helper_task_encountered == FALSE)
       TCW_4(task_team->tt.tt_hidden_helper_task_encountered, TRUE);
   }
 
   // Calculate shared structure offset including padding after kmp_task_t struct
   // to align pointers in shared struct
   shareds_offset = sizeof(kmp_taskdata_t) + sizeof_kmp_task_t;
   shareds_offset = __kmp_round_up_to_val(shareds_offset, sizeof(void *));
 
   // Allocate a kmp_taskdata_t block and a kmp_task_t block.
   KA_TRACE(30, ("__kmp_task_alloc: T#%d First malloc size: %ld\n", gtid,
                 shareds_offset));
   KA_TRACE(30, ("__kmp_task_alloc: T#%d Second malloc size: %ld\n", gtid,
                 sizeof_shareds));
 
   // Avoid double allocation here by combining shareds with taskdata
 #if USE_FAST_MEMORY
-  taskdata = (kmp_taskdata_t *)__kmp_fast_allocate(
-      encountering_thread, shareds_offset + sizeof_shareds);
+  taskdata = (kmp_taskdata_t *)__kmp_fast_allocate(thread, shareds_offset +
+                                                               sizeof_shareds);
 #else /* ! USE_FAST_MEMORY */
-  taskdata = (kmp_taskdata_t *)__kmp_thread_malloc(
-      encountering_thread, shareds_offset + sizeof_shareds);
+  taskdata = (kmp_taskdata_t *)__kmp_thread_malloc(thread, shareds_offset +
+                                                               sizeof_shareds);
 #endif /* USE_FAST_MEMORY */
 
   task = KMP_TASKDATA_TO_TASK(taskdata);
 
 // Make sure task & taskdata are aligned appropriately
 #if KMP_ARCH_X86 || KMP_ARCH_PPC64 || !KMP_HAVE_QUAD
   KMP_DEBUG_ASSERT((((kmp_uintptr_t)taskdata) & (sizeof(double) - 1)) == 0);
   KMP_DEBUG_ASSERT((((kmp_uintptr_t)task) & (sizeof(double) - 1)) == 0);
#endif
   } else {
     task->shareds = NULL;
   }
   task->routine = task_entry;
   task->part_id = 0; // AC: Always start with 0 part id
 
   taskdata->td_task_id = KMP_GEN_TASK_ID();
   taskdata->td_team = thread->th.th_team;
-  taskdata->td_alloc_thread = encountering_thread;
+  taskdata->td_alloc_thread = thread;
   taskdata->td_parent = parent_task;
   taskdata->td_level = parent_task->td_level + 1; // increment nesting level
   KMP_ATOMIC_ST_RLX(&taskdata->td_untied_count, 0);
   taskdata->td_ident = loc_ref;
   taskdata->td_taskwait_ident = NULL;
   taskdata->td_taskwait_counter = 0;
   taskdata->td_taskwait_thread = 0;
   KMP_DEBUG_ASSERT(taskdata->td_parent != NULL);
   // avoid copying icvs for proxy tasks
   if (flags->proxy == TASK_FULL)
     copy_icvs(&taskdata->td_icvs, &taskdata->td_parent->td_icvs);
 
   taskdata->td_flags = *flags;
-  taskdata->encountering_gtid = gtid;
   taskdata->td_task_team = thread->th.th_task_team;
   taskdata->td_size_alloc = shareds_offset + sizeof_shareds;
   taskdata->td_flags.tasktype = TASK_EXPLICIT;
+  // If it is hidden helper task, we need to set the team and task team
+  // correspondingly.
+  if (flags->hidden_helper) {
+    kmp_info_t *shadow_thread = __kmp_threads[KMP_GTID_TO_SHADOW_GTID(gtid)];
+    taskdata->td_team = shadow_thread->th.th_team;
+    taskdata->td_task_team = shadow_thread->th.th_task_team;
+  }
 
   // GEH - TODO: fix this to copy parent task's value of tasking_ser flag
   taskdata->td_flags.tasking_ser = (__kmp_tasking_mode == tskm_immediate_exec);
 
   // GEH - TODO: fix this to copy parent task's value of team_serial flag
   taskdata->td_flags.team_serial = (team->t.t_serialized) ? 1 : 0;
 
   // GEH - Note we serialize the task if the team is serialized to make sure
void __kmpc_give_task(kmp_task_t *ptask, kmp_int32 start = 0) {
   // Enqueue task to complete bottom half completion from a thread within the
   // corresponding team
   kmp_team_t *team = taskdata->td_team;
   kmp_int32 nthreads = team->t.t_nproc;
   kmp_info_t *thread;
 
   // This should be similar to start_k = __kmp_get_random( thread ) % nthreads
   // but we cannot use __kmp_get_random here
-  kmp_int32 start_k = start;
+  kmp_int32 start_k = start % nthreads;
   kmp_int32 pass = 1;
   kmp_int32 k = start_k;
 
   do {
     // For now we're just linearly trying to find a thread
     thread = team->t.t_threads[k];
     k = (k + 1) % nthreads;