This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Patch for PR libgomp/37938, IA64 specific.
On Fri, Nov 7, 2008 at 11:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Nov 7, 2008 at 10:57 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Nov 7, 2008 at 10:35 AM, Steve Ellcey <sje@cup.hp.com> wrote:
>>>
>>> On Thu, 2008-11-06 at 17:12 -0800, H.J. Lu wrote:
>>>
>>>> static inline void gomp_mutex_unlock (gomp_mutex_t *mutex)
>>>> {
>>>> int val = __sync_lock_test_and_set (mutex, 0);
>>>> if (__builtin_expect (val > 1, 0))
>>>> gomp_mutex_unlock_slow (mutex);
>>>> }
>>>>
>>>> should work as long as mutex isn't updated with release or
>>>> unordered semantics prior to this function call. Who else
>>>> have updated mutux? What semantics are they using?
>>>
>>> Isn't the lock done with release semantics? gomp_mutex_lock
>>> calls __sync_bool_compare_and_swap and sync_compare_and_swap<mode> in
>>> sync.md generates a cmpxchg.rel instruction.
>>>
>>
>> According to ia64 psABI, __sync_bool_compare_and_swap and
>> sync_compare_and_swap<mode>
>> should have full barrier. That means they should use cmpxchg.acq,
>> which is icc 11.0 generates.
>>
>
> I think cmpxchg.rel is typo:
>
>
> (define_insn "cmpxchg_rel_<mode>"
> [(set (match_operand:DI 0 "gr_register_operand" "=r")
> (zero_extend:DI
> (match_operand:I124MODE 1 "not_postinc_memory_operand" "+S")))
> (set (match_dup 1)
> (unspec:I124MODE
> [(match_dup 1)
> (match_operand:DI 2 "ar_ccv_reg_operand" "")
> (match_operand:I124MODE 3 "gr_register_operand" "r")]
> UNSPEC_CMPXCHG_ACQ))]
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ""
> "cmpxchg<modesuffix>.rel %0 = %1, %3, %2"
> [(set_attr "itanium_class" "sem")])
>
> (define_insn "cmpxchg_rel_di"
> [(set (match_operand:DI 0 "gr_register_operand" "=r")
> (match_operand:DI 1 "not_postinc_memory_operand" "+S"))
> (set (match_dup 1)
> (unspec:DI [(match_dup 1)
> (match_operand:DI 2 "ar_ccv_reg_operand" "")
> (match_operand:DI 3 "gr_register_operand" "r")]
> UNSPEC_CMPXCHG_ACQ))]
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ""
> "cmpxchg8.rel %0 = %1, %3, %2"
> [(set_attr "itanium_class" "sem")])
>
I am testing this patch. There is no need for memory barrier with acq semantics.
H.J.
---
--- ./sync.md.sync 2007-08-23 09:22:02.000000000 -0700
+++ ./sync.md 2008-11-07 11:29:22.000000000 -0800
@@ -125,15 +125,14 @@
if (GET_MODE (dst) != DImode)
dst = gen_reg_rtx (DImode);
- emit_insn (gen_memory_barrier ());
- emit_insn (gen_cmpxchg_rel_<mode> (dst, operands[1], ccv, operands[3]));
+ emit_insn (gen_cmpxchg_acq_<mode> (dst, operands[1], ccv, operands[3]));
if (dst != operands[0])
emit_move_insn (operands[0], gen_lowpart (<MODE>mode, dst));
DONE;
})
-(define_insn "cmpxchg_rel_<mode>"
+(define_insn "cmpxchg_acq_<mode>"
[(set (match_operand:DI 0 "gr_register_operand" "=r")
(zero_extend:DI
(match_operand:I124MODE 1 "not_postinc_memory_operand" "+S")))
@@ -144,10 +143,10 @@
(match_operand:I124MODE 3 "gr_register_operand" "r")]
UNSPEC_CMPXCHG_ACQ))]
""
- "cmpxchg<modesuffix>.rel %0 = %1, %3, %2"
+ "cmpxchg<modesuffix>.acq %0 = %1, %3, %2"
[(set_attr "itanium_class" "sem")])
-(define_insn "cmpxchg_rel_di"
+(define_insn "cmpxchg_acq_di"
[(set (match_operand:DI 0 "gr_register_operand" "=r")
(match_operand:DI 1 "not_postinc_memory_operand" "+S"))
(set (match_dup 1)
@@ -156,7 +155,7 @@
(match_operand:DI 3 "gr_register_operand" "r")]
UNSPEC_CMPXCHG_ACQ))]
""
- "cmpxchg8.rel %0 = %1, %3, %2"
+ "cmpxchg8.acq %0 = %1, %3, %2"
[(set_attr "itanium_class" "sem")])
(define_insn "sync_lock_test_and_set<mode>"
--
H.J.