openmp: Fix intermittent hanging of task-detach-6 libgomp tests [PR98738]

This adds support for the task detach clause to taskwait, and fixes a
number of problems related to semaphores that may lead to a hang in
some circumstances.

2021-01-21  Kwok Cheung Yeung  <kcy@codesourcery.com>

	libgomp/

	PR libgomp/98738
	* libgomp.h (enum gomp_task_kind): Add GOMP_TASK_DETACHED.
	* task.c (task_fulfilled_p): Check detach field as well.
	(GOMP_task): Add thread to debug messages.  Use address of task as
	the event handle.
	(gomp_barrier_handle_tasks): Fix indentation.  Use address of task
	as event handle. Set kind of suspended detach task to
	GOMP_TASK_DETACHED and decrement task_running_count.  Move
	finish_cancelled block out of else branch.  Skip decrement of
	task_running_count if task kind is GOMP_TASK_DETACHED.
	(GOMP_taskwait): Finish fulfilled detach tasks.  Update comment.
	Queue detach tasks that have not been fulfilled.
	(omp_fulfill_event): Use address of task as event handle.  Post
	to taskwait_sem and taskgroup_sem if necessary.  Check
	task_running_count before calling gomp_team_barrier_wake.
	* testsuite/libgomp.c-c++-common/task-detach-5.c (main): Change
	data-sharing of detach events on enclosing parallel to private.
	* testsuite/libgomp.c-c++-common/task-detach-6.c (main): Likewise.
	* testsuite/libgomp.fortran/task-detach-5.f90 (task_detach_5):
	Likewise.
	* testsuite/libgomp.fortran/task-detach-6.f90 (task_detach_6):
	Likewise.

diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index dbaa30d3381d3fda01b660120c57ea6ba09c0e09..53c10fcfbd120dcabafffb8d1d072b6932fdcceb 100644 (file)
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,3 +1,28 @@
+2021-01-21  Kwok Cheung Yeung  <kcy@codesourcery.com>
+
+       PR libgomp/98738
+       * libgomp.h (enum gomp_task_kind): Add GOMP_TASK_DETACHED.
+       * task.c (task_fulfilled_p): Check detach field as well.
+       (GOMP_task): Add thread to debug messages.  Use address of task as
+       the event handle.
+       (gomp_barrier_handle_tasks): Fix indentation.  Use address of task
+       as event handle. Set kind of suspended detach task to
+       GOMP_TASK_DETACHED and decrement task_running_count.  Move
+       finish_cancelled block out of else branch.  Skip decrement of
+       task_running_count if task kind is GOMP_TASK_DETACHED.
+       (GOMP_taskwait): Finish fulfilled detach tasks.  Update comment.
+       Queue detach tasks that have not been fulfilled.
+       (omp_fulfill_event): Use address of task as event handle.  Post
+       to taskwait_sem and taskgroup_sem if necessary.  Check
+       task_running_count before calling gomp_team_barrier_wake.
+       * testsuite/libgomp.c-c++-common/task-detach-5.c (main): Change
+       data-sharing of detach events on enclosing parallel to private.
+       * testsuite/libgomp.c-c++-common/task-detach-6.c (main): Likewise.
+       * testsuite/libgomp.fortran/task-detach-5.f90 (task_detach_5):
+       Likewise.
+       * testsuite/libgomp.fortran/task-detach-6.f90 (task_detach_6):
+       Likewise.
+
 2021-01-22  Kwok Cheung Yeung  <kcy@codesourcery.com>
 
        Backport from mainline

diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 9b1378559bcea6ed30650c42aafb49cf2a469419..f541369f92bc76096fb89c452397123dac18efa2 100644 (file)
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -481,7 +481,10 @@ enum gomp_task_kind
      but not yet completed.  Once that completes, they will be readded
      into the queues as GOMP_TASK_WAITING in order to perform the var
      unmapping.  */
-  GOMP_TASK_ASYNC_RUNNING
+  GOMP_TASK_ASYNC_RUNNING,
+  /* Task that has finished executing but is waiting for its
+     completion event to be fulfilled.  */
+  GOMP_TASK_DETACHED
 };
 
 struct gomp_task_depend_entry

diff --git a/libgomp/task.c b/libgomp/task.c
index 78627726e0c813c30a1c3e7d308bf663cd5f5000..713138a10e76ac8440e72a122c08293dddc61aeb 100644 (file)
--- a/libgomp/task.c
+++ b/libgomp/task.c
@@ -330,7 +330,7 @@ gomp_task_handle_depend (struct gomp_task *task, struct gomp_task *parent,
 static bool
 task_fulfilled_p (struct gomp_task *task)
 {
-  return gomp_sem_getcount (&task->completion_sem) > 0;
+  return task->detach && gomp_sem_getcount (&task->completion_sem) > 0;
 }
 
 /* Called when encountering an explicit task directive.  If IF_CLAUSE is
@@ -419,11 +419,12 @@ GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
        {
          task.detach = true;
          gomp_sem_init (&task.completion_sem, 0);
-         *(void **) detach = &task.completion_sem;
+         *(void **) detach = &task;
          if (data)
-           *(void **) data = &task.completion_sem;
+           *(void **) data = &task;
 
-         gomp_debug (0, "New event: %p\n", &task.completion_sem);
+         gomp_debug (0, "Thread %d: new event: %p\n",
+                     thr->ts.team_id, &task);
        }
 
       if (thr->task)
@@ -488,11 +489,12 @@ GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
        {
          task->detach = true;
          gomp_sem_init (&task->completion_sem, 0);
-         *(void **) detach = &task->completion_sem;
+         *(void **) detach = task;
          if (data)
-           *(void **) data = &task->completion_sem;
+           *(void **) data = task;
 
-         gomp_debug (0, "New event: %p\n", &task->completion_sem);
+         gomp_debug (0, "Thread %d: new event: %p\n",
+                     thr->ts.team_id, task);
        }
       thr->task = task;
       if (cpyfn)
@@ -1372,14 +1374,14 @@ gomp_barrier_handle_tasks (gomp_barrier_state_t state)
                                 child_task, MEMMODEL_RELAXED);
          --team->task_detach_count;
          gomp_debug (0, "thread %d: found task with fulfilled event %p\n",
-                     thr->ts.team_id, &child_task->completion_sem);
+                     thr->ts.team_id, child_task);
 
-       if (to_free)
-         {
-           gomp_finish_task (to_free);
-           free (to_free);
-           to_free = NULL;
-         }
+         if (to_free)
+           {
+             gomp_finish_task (to_free);
+             free (to_free);
+             to_free = NULL;
+           }
          goto finish_cancelled;
        }
 
@@ -1452,41 +1454,43 @@ gomp_barrier_handle_tasks (gomp_barrier_state_t state)
        {
          if (child_task->detach && !task_fulfilled_p (child_task))
            {
+             child_task->kind = GOMP_TASK_DETACHED;
              priority_queue_insert (PQ_TEAM, &team->task_detach_queue,
                                     child_task, child_task->priority,
                                     PRIORITY_INSERT_END,
                                     false, false);
              ++team->task_detach_count;
-             gomp_debug (0, "thread %d: queueing task with event %p\n",
-                         thr->ts.team_id, &child_task->completion_sem);
+             --team->task_running_count;
+             gomp_debug (0,
+                         "thread %d: queuing detached task with event %p\n",
+                         thr->ts.team_id, child_task);
              child_task = NULL;
+             continue;
            }
-         else
+
+        finish_cancelled:;
+         size_t new_tasks
+           = gomp_task_run_post_handle_depend (child_task, team);
+         gomp_task_run_post_remove_parent (child_task);
+         gomp_clear_parent (&child_task->children_queue);
+         gomp_task_run_post_remove_taskgroup (child_task);
+         to_free = child_task;
+         if (!cancelled && child_task->kind != GOMP_TASK_DETACHED)
+           team->task_running_count--;
+         child_task = NULL;
+         if (new_tasks > 1)
            {
-            finish_cancelled:;
-             size_t new_tasks
-               = gomp_task_run_post_handle_depend (child_task, team);
-             gomp_task_run_post_remove_parent (child_task);
-             gomp_clear_parent (&child_task->children_queue);
-             gomp_task_run_post_remove_taskgroup (child_task);
-             to_free = child_task;
-             child_task = NULL;
-             if (!cancelled)
-               team->task_running_count--;
-             if (new_tasks > 1)
-               {
-                 do_wake = team->nthreads - team->task_running_count;
-                 if (do_wake > new_tasks)
-                   do_wake = new_tasks;
-               }
-             if (--team->task_count == 0
-                 && gomp_team_barrier_waiting_for_tasks (&team->barrier))
-               {
-                 gomp_team_barrier_done (&team->barrier, state);
-                 gomp_mutex_unlock (&team->task_lock);
-                 gomp_team_barrier_wake (&team->barrier, 0);
-                 gomp_mutex_lock (&team->task_lock);
-               }
+             do_wake = team->nthreads - team->task_running_count;
+             if (do_wake > new_tasks)
+               do_wake = new_tasks;
+           }
+         if (--team->task_count == 0
+             && gomp_team_barrier_waiting_for_tasks (&team->barrier))
+           {
+             gomp_team_barrier_done (&team->barrier, state);
+             gomp_mutex_unlock (&team->task_lock);
+             gomp_team_barrier_wake (&team->barrier, 0);
+             gomp_mutex_lock (&team->task_lock);
            }
        }
     }
@@ -1556,10 +1560,28 @@ GOMP_taskwait (void)
              goto finish_cancelled;
            }
        }
+      else if (next_task->kind == GOMP_TASK_DETACHED
+              && task_fulfilled_p (next_task))
+       {
+         child_task = next_task;
+         gomp_debug (0, "thread %d: found task with fulfilled event %p\n",
+                     thr->ts.team_id, &child_task);
+         priority_queue_remove (PQ_TEAM, &team->task_detach_queue,
+                                child_task, MEMMODEL_RELAXED);
+         --team->task_detach_count;
+         if (to_free)
+           {
+             gomp_finish_task (to_free);
+             free (to_free);
+             to_free = NULL;
+           }
+         goto finish_cancelled;
+       }
       else
        {
        /* All tasks we are waiting for are either running in other
-          threads, or they are tasks that have not had their
+          threads, are detached and waiting for the completion event to be
+          fulfilled, or they are tasks that have not had their
           dependencies met (so they're not even in the queue).  Wait
           for them.  */
          if (task->taskwait == NULL)
@@ -1614,6 +1636,21 @@ GOMP_taskwait (void)
       gomp_mutex_lock (&team->task_lock);
       if (child_task)
        {
+         if (child_task->detach && !task_fulfilled_p (child_task))
+           {
+             child_task->kind = GOMP_TASK_DETACHED;
+             priority_queue_insert (PQ_TEAM, &team->task_detach_queue,
+                                    child_task, child_task->priority,
+                                    PRIORITY_INSERT_END,
+                                    false, false);
+             ++team->task_detach_count;
+             gomp_debug (0,
+                         "thread %d: queuing detached task with event %p\n",
+                         thr->ts.team_id, child_task);
+             child_task = NULL;
+             continue;
+           }
+
         finish_cancelled:;
          size_t new_tasks
            = gomp_task_run_post_handle_depend (child_task, team);
@@ -2402,17 +2439,45 @@ ialias (omp_in_final)
 void
 omp_fulfill_event (omp_event_handle_t event)
 {
-  gomp_sem_t *sem = (gomp_sem_t *) event;
+  struct gomp_task *task = (struct gomp_task *) event;
+  struct gomp_task *parent = task->parent;
   struct gomp_thread *thr = gomp_thread ();
   struct gomp_team *team = thr ? thr->ts.team : NULL;
 
-  if (gomp_sem_getcount (sem) > 0)
-    gomp_fatal ("omp_fulfill_event: %p event already fulfilled!\n", sem);
+  if (gomp_sem_getcount (&task->completion_sem) > 0)
+    gomp_fatal ("omp_fulfill_event: %p event already fulfilled!\n", task);
 
-  gomp_debug (0, "omp_fulfill_event: %p\n", sem);
-  gomp_sem_post (sem);
-  if (team)
-    gomp_team_barrier_wake (&team->barrier, 1);
+  gomp_debug (0, "omp_fulfill_event: %p\n", task);
+  gomp_sem_post (&task->completion_sem);
+
+  /* Wake up any threads that may be waiting for the detached task
+     to complete.  */
+  gomp_mutex_lock (&team->task_lock);
+  if (parent && parent->taskwait)
+    {
+      if (parent->taskwait->in_taskwait)
+       {
+         parent->taskwait->in_taskwait = false;
+         gomp_sem_post (&parent->taskwait->taskwait_sem);
+       }
+      else if (parent->taskwait->in_depend_wait)
+       {
+         parent->taskwait->in_depend_wait = false;
+         gomp_sem_post (&parent->taskwait->taskwait_sem);
+       }
+    }
+  if (task->taskgroup && task->taskgroup->in_taskgroup_wait)
+    {
+      task->taskgroup->in_taskgroup_wait = false;
+      gomp_sem_post (&task->taskgroup->taskgroup_sem);
+    }
+  if (team && team->nthreads > team->task_running_count)
+    {
+      gomp_mutex_unlock (&team->task_lock);
+      gomp_team_barrier_wake (&team->barrier, 1);
+    }
+  else
+    gomp_mutex_unlock (&team->task_lock);
 }
 
 ialias (omp_fulfill_event)

diff --git a/libgomp/testsuite/libgomp.c-c++-common/task-detach-5.c b/libgomp/testsuite/libgomp.c-c++-common/task-detach-5.c
index 5a0151729efdecf9de39f6a7813086b61a25d0e7..71bcde9daea88c2fae53dff03a199b0307b7bfc9 100644 (file)
--- a/libgomp/testsuite/libgomp.c-c++-common/task-detach-5.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/task-detach-5.c
@@ -12,7 +12,7 @@ int main (void)
   int thread_count;
   omp_event_handle_t detach_event1, detach_event2;
 
-  #pragma omp parallel firstprivate(detach_event1, detach_event2)
+  #pragma omp parallel private(detach_event1, detach_event2)
   {
     #pragma omp single
       thread_count = omp_get_num_threads();

diff --git a/libgomp/testsuite/libgomp.c-c++-common/task-detach-6.c b/libgomp/testsuite/libgomp.c-c++-common/task-detach-6.c
index b5f68ccabdcc7471f02f6f57edf58d8fad795f1c..e7af05a9a7dd9461696d1ccc54b39ea096f23dd9 100644 (file)
--- a/libgomp/testsuite/libgomp.c-c++-common/task-detach-6.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/task-detach-6.c
@@ -14,7 +14,7 @@ int main (void)
   omp_event_handle_t detach_event1, detach_event2;
 
   #pragma omp target map(tofrom: x, y, z) map(from: thread_count)
-    #pragma omp parallel firstprivate(detach_event1, detach_event2)
+    #pragma omp parallel private(detach_event1, detach_event2)
       {
        #pragma omp single
          thread_count = omp_get_num_threads();

diff --git a/libgomp/testsuite/libgomp.fortran/task-detach-5.f90 b/libgomp/testsuite/libgomp.fortran/task-detach-5.f90
index 955d687ca8b53b48af8d4be87677c76ea9b25df1..8bebb5c506d8342e3cb940cf4ffcc02c47c3cd66 100644 (file)
--- a/libgomp/testsuite/libgomp.fortran/task-detach-5.f90
+++ b/libgomp/testsuite/libgomp.fortran/task-detach-5.f90
@@ -10,7 +10,7 @@ program task_detach_5
   integer :: x = 0, y = 0, z = 0
   integer :: thread_count
 
-  !$omp parallel firstprivate(detach_event1, detach_event2)
+  !$omp parallel private(detach_event1, detach_event2)
     !$omp single
       thread_count = omp_get_num_threads()
     !$omp end single

diff --git a/libgomp/testsuite/libgomp.fortran/task-detach-6.f90 b/libgomp/testsuite/libgomp.fortran/task-detach-6.f90
index 0fe21553494f6877087cd5c095d810c2d3add7d9..437ca66b13d58a9ecb0a6f7c247af852909b2c50 100644 (file)
--- a/libgomp/testsuite/libgomp.fortran/task-detach-6.f90
+++ b/libgomp/testsuite/libgomp.fortran/task-detach-6.f90
@@ -12,7 +12,7 @@ program task_detach_6
   integer :: thread_count
 
   !$omp target map(tofrom: x, y, z) map(from: thread_count)
-    !$omp parallel firstprivate(detach_event1, detach_event2)
+    !$omp parallel private(detach_event1, detach_event2)
       !$omp single
        thread_count = omp_get_num_threads()
       !$omp end single