On Thu, Nov 17, 2011 at 02:56:31PM -1000, Richard Henderson wrote: > On 11/17/2011 01:56 AM, Alan Modra wrote: > > This is part of work in progress getting locking back into shape on > > powerpc. (If we ever were in shape, which is doubtful.) Using the > > ia64 version means we have a needless sync at the start of > > gomp_mutex_lock (courtesy of __sync_val_compare_and_swap) and a > > needless isync at the end of gomp_mutex_unlock (slightly odd use > > of __sync_lock_test_and_set to release a lock). It would be nice to > > get rid of the __sync_val_compare_and_swap in config/linux/mutex.c > > too. > > We should be able to use this new file as a replacement for all > targets, not just PPC. > > And, yes, updating all of the __sync uses in libgomp would be a > good thing. > > > + __atomic_compare_exchange_4 (mutex, &oldval, 1, false, > > + MEMMODEL_ACQUIRE, MEMMODEL_RELAXED); > > + if (__builtin_expect (oldval != 0, 0)) > > + gomp_mutex_lock_slow (mutex, oldval); > > __atomic_compare_exchange_4 returns a boolean success: > > if (__builtin_expect (!__atomic_compare_exchange_4 > (mutex, &oldval, 1, false, > MEMMODEL_ACQUIRE, MEMMODEL_RELAXED), 0)) > > In theory that should avoid an extra comparison on some targets.
That was the way I wrote it at first. This way gives three fewer insns on powerpc, because insns setting up the boolean are elided as is the comparison of the boolean against zero. An oldval != 0 comparison insn is also not needed because __atomic_compare_exchange already did that for us. BTW, we're left with dead stores into oldval's stack slot. Is it legal for the first two parameters of __atomic_compare_exchange to alias? I'm guessing that the stores might disappear if we tell gcc that they can't alias. -- Alan Modra Australia Development Lab, IBM
