This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch for PR libgomp/37938, IA64 specific.


On Thu, Nov 06, 2008 at 02:44:55PM -0800, Steve Ellcey wrote:
> So I put a call to __sync_synchronize into gomp_mutex_unlock and that
> seems to work fine, my only complaint is that this requires us to have
> an ia64 specific version of mutex.h.  Now if ia64 was doing something
> wrong, I would understand that.  But the IA64 __sync_lock_test_and_set
> instruction has acquire semantics which is what is documented in
> extend.texi.  If gomp_mutex_unlock needs something more then that
> shouldn't the standard linux version of gomp_mutex_unlock be changed to
> call __sync_synchronize and/or use something else instead of
> __sync_lock_test_and_set?

This is libgomp internal header and something that has to be as fast as
possible, it is inlined in many places inside of libgomp.
IMHO if we put a big comment in the generic header that it relies on 
release semantics (or full barrier) of the builtin and that targets which
don't offer this for __sync_lock_test_and_set should #include "ia64/mutex.h"
in their config/linux/<arch>/mutex.h, it is acceptable.
>From my brief look at the targets that are currently supported by libgomp's
config/linux:

Doesn't care or only tiny bit worse code: x86
Worse code: powerpc, s390, sparc, alpha, mips   
Needs __sync_synchronize: ia64

I really think that using __sync_synchronize in config/linux/mutex.h and
have all targets but ia64 override this header is worse than having
just special config/linux/ia64/mutex.h.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]