re PR libgomp/51376 (libgomp taskwait failure)

	PR libgomp/51376
	PR libgomp/56073
	* task.c (GOMP_task): Revert 2011-12-09 change.
	(GOMP_taskwait): Likewise.  Instead use atomic load with acquire
	barrier to read task->children..
	(gomp_barrier_handle_tasks): ..and matching atomic store with
	release barrier here when setting parent->children to NULL.

From-SVN: r195756

2 files changed, 36 insertions, 5 deletions

diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index 25a5994469f..7422a81e671 100644
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,13 @@
+2013-01-22  Alan Modra  <amodra@gmail.com>
+
+	PR libgomp/51376
+	PR libgomp/56073
+	* task.c (GOMP_task): Revert 2011-12-09 change.
+	(GOMP_taskwait): Likewise.  Instead use atomic load with acquire
+	barrier to read task->children..
+	(gomp_barrier_handle_tasks): ..and matching atomic store with
+	release barrier here when setting parent->children to NULL.
+
 2012-11-21  Jakub Jelinek  <jakub@redhat.com>
 
 	PR libgomp/55411
diff --git a/libgomp/task.c b/libgomp/task.c
index 4b75850072b..13e6605a99f 100644
--- a/libgomp/task.c
+++ b/libgomp/task.c
@@ -116,11 +116,19 @@ GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
 	}
       else
 	fn (data);
-      if (team != NULL)
+      /* Access to "children" is normally done inside a task_lock
+	 mutex region, but the only way this particular task.children
+	 can be set is if this thread's task work function (fn)
+	 creates children.  So since the setter is *this* thread, we
+	 need no barriers here when testing for non-NULL.  We can have
+	 task.children set by the current thread then changed by a
+	 child thread, but seeing a stale non-NULL value is not a
+	 problem.  Once past the task_lock acquisition, this thread
+	 will see the real value of task.children.  */
+      if (task.children != NULL)
 	{
 	  gomp_mutex_lock (&team->task_lock);
-	  if (task.children != NULL)
-	    gomp_clear_parent (task.children);
+	  gomp_clear_parent (task.children);
 	  gomp_mutex_unlock (&team->task_lock);
 	}
       gomp_end_task ();
@@ -258,7 +266,13 @@ gomp_barrier_handle_tasks (gomp_barrier_state_t state)
 		    parent->children = child_task->next_child;
 		  else
 		    {
-		      parent->children = NULL;
+		      /* We access task->children in GOMP_taskwait
+			 outside of the task lock mutex region, so
+			 need a release barrier here to ensure memory
+			 written by child_task->fn above is flushed
+			 before the NULL is written.  */
+		      __atomic_store_n (&parent->children, NULL,
+					MEMMODEL_RELEASE);
 		      if (parent->in_taskwait)
 			gomp_sem_post (&parent->taskwait_sem);
 		    }
@@ -291,7 +305,14 @@ GOMP_taskwait (void)
   struct gomp_task *child_task = NULL;
   struct gomp_task *to_free = NULL;
 
-  if (task == NULL || team == NULL)
+  /* The acquire barrier on load of task->children here synchronizes
+     with the write of a NULL in gomp_barrier_handle_tasks.  It is
+     not necessary that we synchronize with other non-NULL writes at
+     this point, but we must ensure that all writes to memory by a
+     child thread task work function are seen before we exit from
+     GOMP_taskwait.  */
+  if (task == NULL
+      || __atomic_load_n (&task->children, MEMMODEL_ACQUIRE) == NULL)
     return;
 
   gomp_mutex_lock (&team->task_lock);