PR libgomp/51376 fix

The simple one-line fix in GOMP_taskwait took many hours to find.
Shared memory problems are a pain to debug, especially when adding
code to dump some state turns a testcase that fails every hundred or
so runs into one that takes thousands of times longer to fail.

What happens here is that GOMP_taskwait is called in the parent thread
some time after gomp_barrier_handle_tasks has run in the child to the
point of clearing the parent's "children" field.  However, since there
is no acquire barrier in the parent and the child may or may not have
reached the release barrier in the mutex unlock, the memory stores in
the child are not guaranteed to be seen in order in the parent thread.
Thus the parent can see "task->children" clear but not yet see stores
done as part of the real work of the child, ie. to "a" and "n" in the
testcase.

The GOMP_task change fixes a similar potential problem.  Bootstrapped
and regression tested powerpc-linux.  OK to apply?

	PR libgomp/51376
	* task.c (GOMP_taskwait): Don't access task->children outside of
	task_lock mutex region.
	(GOMP_task): Likewise.

On Thu, Dec 01, 2011 at 09:58:08PM +1030, Alan Modra wrote:
> The GOMP_task change fixes a similar potential problem.  Bootstrapped
> and regression tested powerpc-linux.  OK to apply?
> 
> 	PR libgomp/51376
> 	* task.c (GOMP_taskwait): Don't access task->children outside of
> 	task_lock mutex region.
> 	(GOMP_task): Likewise.

Can't this be solved just by adding a barrier?  The access to the var
outside of the lock has been quite intentional, to avoid locking in the
common case where there are no children.

> --- libgomp/task.c	(revision 181833)
> +++ libgomp/task.c	(working copy)
> @@ -116,12 +116,12 @@ GOMP_task (void (*fn) (void *), void *da
>  	}
>        else
>  	fn (data);
> -      if (task.children)
> -	{
> -	  gomp_mutex_lock (&team->task_lock);
> -	  gomp_clear_parent (task.children);
> -	  gomp_mutex_unlock (&team->task_lock);
> -	}
> +      if (team != NULL)
> +	gomp_mutex_lock (&team->task_lock);
> +      if (task.children != NULL)
> +	gomp_clear_parent (task.children);
> +      if (team != NULL)
> +	gomp_mutex_unlock (&team->task_lock);
>        gomp_end_task ();
>      }
>    else
> @@ -290,8 +290,9 @@ GOMP_taskwait (void)
>    struct gomp_task *child_task = NULL;
>    struct gomp_task *to_free = NULL;
>  
> -  if (task == NULL || task->children == NULL)
> +  if (task == NULL)
>      return;
> +
>    gomp_mutex_lock (&team->task_lock);
>    while (1)
>      {

	Jakub