re PR libgomp/51298 (libgomp team_barrier locking failures)

	PR libgomp/51298
	* config/linux/bar.h: Use atomic rather than sync builtins.
	* config/linux/bar.c: Likewise.  Add missing acquire
	synchronisation on generation field.
	* task.c (gomp_barrier_handle_tasks): Regain lock so as to not
	double unlock.

From-SVN: r181833

diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index faa6749d8248d3306fd8551a110a322d3c0fe90b..f6278b771591b516b94c6602238a7688e43bd5cc 100644 (file)
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,12 @@
+2011-11-30  Alan Modra  <amodra@gmail.com>
+
+       PR libgomp/51298
+       * config/linux/bar.h: Use atomic rather than sync builtins.
+       * config/linux/bar.c: Likewise.  Add missing acquire
+       synchronisation on generation field.
+       * task.c (gomp_barrier_handle_tasks): Regain lock so as to not
+       double unlock.
+
 2011-11-30  Alan Modra  <amodra@gmail.com>
 
        * ordered.c (gomp_ordered_sync): Add MEMMODEL_ACQ_REL fence.

diff --git a/libgomp/config/linux/bar.c b/libgomp/config/linux/bar.c
index 3e0379eb695958be0688ff3c441df9a3f1a58328..223a2aec432579f833e3f09499cab745ff2b89ea 100644 (file)
--- a/libgomp/config/linux/bar.c
+++ b/libgomp/config/linux/bar.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2005, 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2005, 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU OpenMP Library (libgomp).
@@ -37,17 +37,15 @@ gomp_barrier_wait_end (gomp_barrier_t *bar, gomp_barrier_state_t state)
     {
       /* Next time we'll be awaiting TOTAL threads again.  */
       bar->awaited = bar->total;
-      atomic_write_barrier ();
-      bar->generation += 4;
+      __atomic_store_n (&bar->generation, bar->generation + 4,
+                       MEMMODEL_RELEASE);
       futex_wake ((int *) &bar->generation, INT_MAX);
     }
   else
     {
-      unsigned int generation = state;
-
       do
-       do_wait ((int *) &bar->generation, generation);
-      while (bar->generation == generation);
+       do_wait ((int *) &bar->generation, state);
+      while (__atomic_load_n (&bar->generation, MEMMODEL_ACQUIRE) == state);
     }
 }
 
@@ -81,15 +79,15 @@ gomp_team_barrier_wake (gomp_barrier_t *bar, int count)
 void
 gomp_team_barrier_wait_end (gomp_barrier_t *bar, gomp_barrier_state_t state)
 {
-  unsigned int generation;
+  unsigned int generation, gen;
 
   if (__builtin_expect ((state & 1) != 0, 0))
     {
       /* Next time we'll be awaiting TOTAL threads again.  */
       struct gomp_thread *thr = gomp_thread ();
       struct gomp_team *team = thr->ts.team;
+
       bar->awaited = bar->total;
-      atomic_write_barrier ();
       if (__builtin_expect (team->task_count, 0))
        {
          gomp_barrier_handle_tasks (state);
@@ -97,7 +95,7 @@ gomp_team_barrier_wait_end (gomp_barrier_t *bar, gomp_barrier_state_t state)
        }
       else
        {
-         bar->generation = state + 3;
+         __atomic_store_n (&bar->generation, state + 3, MEMMODEL_RELEASE);
          futex_wake ((int *) &bar->generation, INT_MAX);
          return;
        }
@@ -107,12 +105,16 @@ gomp_team_barrier_wait_end (gomp_barrier_t *bar, gomp_barrier_state_t state)
   do
     {
       do_wait ((int *) &bar->generation, generation);
-      if (__builtin_expect (bar->generation & 1, 0))
-       gomp_barrier_handle_tasks (state);
-      if ((bar->generation & 2))
+      gen = __atomic_load_n (&bar->generation, MEMMODEL_ACQUIRE);
+      if (__builtin_expect (gen & 1, 0))
+       {
+         gomp_barrier_handle_tasks (state);
+         gen = __atomic_load_n (&bar->generation, MEMMODEL_ACQUIRE);
+       }
+      if ((gen & 2) != 0)
        generation |= 2;
     }
-  while (bar->generation != state + 4);
+  while (gen != state + 4);
 }
 
 void

diff --git a/libgomp/config/linux/bar.h b/libgomp/config/linux/bar.h
index 9c1a8a07687485d5b73d2e9985bbf7f0c3ccd3f8..58bd40d698964b458d022136012ac3d5c42f7485 100644 (file)
--- a/libgomp/config/linux/bar.h
+++ b/libgomp/config/linux/bar.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2005, 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 2005, 2008, 2009, 2011 Free Software Foundation, Inc.
    Contributed by Richard Henderson <rth@redhat.com>.
 
    This file is part of the GNU OpenMP Library (libgomp).
@@ -50,7 +50,7 @@ static inline void gomp_barrier_init (gomp_barrier_t *bar, unsigned count)
 
 static inline void gomp_barrier_reinit (gomp_barrier_t *bar, unsigned count)
 {
-  __sync_fetch_and_add (&bar->awaited, count - bar->total);
+  __atomic_add_fetch (&bar->awaited, count - bar->total, MEMMODEL_ACQ_REL);
   bar->total = count;
 }
 
@@ -69,10 +69,13 @@ extern void gomp_team_barrier_wake (gomp_barrier_t *, int);
 static inline gomp_barrier_state_t
 gomp_barrier_wait_start (gomp_barrier_t *bar)
 {
-  unsigned int ret = bar->generation & ~3;
-  /* Do we need any barrier here or is __sync_add_and_fetch acting
-     as the needed LoadLoad barrier already?  */
-  ret += __sync_add_and_fetch (&bar->awaited, -1) == 0;
+  unsigned int ret = __atomic_load_n (&bar->generation, MEMMODEL_ACQUIRE) & ~3;
+  /* A memory barrier is needed before exiting from the various forms
+     of gomp_barrier_wait, to satisfy OpenMP API version 3.1 section
+     2.8.6 flush Construct, which says there is an implicit flush during
+     a barrier region.  This is a convenient place to add the barrier,
+     so we use MEMMODEL_ACQ_REL here rather than MEMMODEL_ACQUIRE.  */
+  ret += __atomic_add_fetch (&bar->awaited, -1, MEMMODEL_ACQ_REL) == 0;
   return ret;
 }
 

diff --git a/libgomp/task.c b/libgomp/task.c
index b93f77a38167e613235b3f5a14ef3588a9e2e5d1..d3d72663f35c0762a2f4c0557240350eb08c9dd5 100644 (file)
--- a/libgomp/task.c
+++ b/libgomp/task.c
@@ -273,6 +273,7 @@ gomp_barrier_handle_tasks (gomp_barrier_state_t state)
              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);
            }
        }
     }