On Mon, Nov 28, 2011 at 10:37:09AM +1030, Alan Modra wrote:
> On Mon, Nov 28, 2011 at 09:27:20AM +1030, Alan Modra wrote:
> > This is the mutex part. Depends on
> > http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02454.html for MEMMODEL_*
> > values.
>
> Arrgh, I posted the wrong patch. I know it needs work elsewhere in
> libgomp to comply with the OMP spec, which says of flush:
>
> 14 A flush region without a list is implied at the following locations:
> 15 • During a barrier region.
> 16 • At entry to and exit from parallel, critical, and ordered regions.
> [snip]
>
> Yet COMP_critical_start simply calls gomp_mutex_lock, relying on the
> lock acquisition to do the flush. This won't necessarily happen with
> MEMMODEL_ACQUIRE.
>
> This bit in gomp_mutex_lock
> > + __atomic_compare_exchange_4 (mutex, &oldval, 1, false,
> > + MEMMODEL_ACQUIRE, MEMMODEL_RELAXED);
> should be
> __atomic_compare_exchange_4 (mutex, &oldval, 1, false,
> MEMMODEL_ACQ_REL, MEMMODEL_ACQ_REL);
>
> and once you do that it's hard to justify the patch for stage3 as
> fixing a bug (unnecessary sync instructions emitted by __sync
> builtins).
If it is just GOMP_critical_start, couldn't it use an extra barrier
before resp. after it calls mutex_lock?
Jakub
