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 6, 2008 at 1:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Nov 06, 2008 at 10:33:11AM -0800, Steve Ellcey wrote:
>> On Thu, 2008-11-06 at 09:56 -0800, H.J. Lu wrote:
>>
>> > Shouldn't we use __sync_lock_test_and_set  to initialize mutux?
>> >
>> >
>> > H.J.
>> > --- libgomp/config/linux/mutex.h.sync       2006-11-18 06:22:18.000000000 -0800
>> > +++ libgomp/config/linux/mutex.h    2008-11-06 09:50:46.000000000 -0800
>> > @@ -38,7 +38,7 @@ typedef int gomp_mutex_t;
>> >
>> >  static inline void gomp_mutex_init (gomp_mutex_t *mutex)
>> >  {
>> > -  *mutex = 0;
>> > +  __sync_lock_test_and_set (mutex, 0);
>> >  }
>> >
>> >  extern void gomp_mutex_lock_slow (gomp_mutex_t *mutex);
>>
>> I was wondering about the initialization but this patch didn't seem to
>> fix the test case in PR 37938 when I added it and then removed my
>> patches.
>
> Why would mutex initialization need it?  A mutex is always initialized
> uncontended, so other threads either don't exist at that point, or are
> prevented from trying to look it, usually by some other mutex, barrier etc.
>
> I guess the problem is that the __sync_* builtins don't have explicit
> acquire vs. release semantics variants.  We need to use
> __sync_bool_compare_and_swap for locking the mutex, and I believe on ia64
> __sync_bool_compare_and_swap has release, not acquire semantics, which is
> desirable for mutex lock, and for unlocking the mutex we use
> __sync_lock_test_and_set (which has acquire semantics?), but want release
> semantics, right?  Putting __sync_synchronize into gomp_mutex_unlock
> would mean slowing down other targets, for which already
> __sync_lock_test_and_set (and __sync_bool_compare_and_swap) act as a full
> barrier.  So I'm afraid ia64 needs to define config/linux/ia64/mutex.[ch]
> which uses __asm instead of __sync builtins.
>

According to ia64 psABI, ia64 _sync_bool_compare_and_swap should
have full barrier.  __sync_lock_test_and_set  has acquire barrier.


-- 
H.J.


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