On Mon, Jun 14, 2010 at 07:04:43PM +0200, Paolo Bonzini wrote:
> On 06/13/2010 11:28 PM, Paul E. McKenney wrote:
> >On Sun, Jun 13, 2010 at 05:20:34PM -0400, Mathieu Desnoyers wrote:
> >>* Paul E. McKenney ([email protected]) wrote:
> >>>On Thu, Jun 10, 2010 at 09:46:31PM -0400, Mathieu Desnoyers wrote:
> >>
> >>[...]
> >>
> >>(will reply to the rest in the individual patches)
> >>
> >>>>Can we trust __sync_lock_test_and_set/__sync_add_and_fetch given that
> >>>>__sync_synchronize is broken ?
> >>>
> >>>I don't know yet. If it turns out that we cannot, then I will use some
> >>>form of global locking. But the __sync_lock_test_and_set() do at least
> >>>generate instructions, unlike __sync_synchronize(). ;-)
> >>
> >>I'm concerned about the fact that their synchronization primitives might
> >>have
> >>the assembly all with, except for the memory barriers.
>
> The default implementation of these __sync_* builtins is based on
> cmpxchg, and will cause a link error unless cmpxchg is also
> available (either in libgcc or with a compiler-provided inline
> implementation).
>
> Instead, the default implementation of __sync_synchronize is to just
> do a compiler barrier. ARM implements __sync_synchronize only for
> Linux, so at least there it is not needed. Strange that Paul needs
> it too.
When I use __sync_synchronize(), I get failures from the rcutorture_urcu_mb
stress test. When I use a "dmb" instruction, I don't get any failures.
No idea why, to be honest.
> Anyway, a simple configure test is to compile this with -fdump-rtl-expand:
>
> int
> f()
> {
> __sync_synchronize();
> }
>
> If the assembly output includes "__sync_synchronize", or the dump
> file includes the text "unspec:BLK", it should be fine. In
> particular, ia64, mips, and Alpha are ok. Else you can use the
> pthreads trick. I can try to make a patch if you're interested.
> Or, more simply, it's possible to hardcode the above three platforms
> since it's unlikely that others will be added soon.
And I attached the input file, the .expand file, and the .s file.
I see neither __sync_synchronize in the .s file nor "unspec" in the
.expand file. Or was I confused about what to look for?
Thanx, Paul
int
f()
{
__sync_synchronize();
}
.arch armv6
.eabi_attribute 27, 3
.fpu vfp
.eabi_attribute 20, 1
.eabi_attribute 21, 1
.eabi_attribute 23, 3
.eabi_attribute 24, 1
.eabi_attribute 25, 1
.eabi_attribute 26, 2
.eabi_attribute 30, 6
.eabi_attribute 18, 4
.file "sync_sync.c"
.text
.align 2
.global f
.type f, %function
f:
@ args = 0, pretend = 0, frame = 0
@ frame_needed = 1, uses_anonymous_args = 0
@ link register save eliminated.
str fp, [sp, #-4]!
add fp, sp, #0
add sp, fp, #0
ldmfd sp!, {fp}
bx lr
.size f, .-f
.ident "GCC: (Ubuntu 4.4.1-4ubuntu9) 4.4.1"
.section .note.GNU-stack,"",%progbits
;; Function f (f)
;; Generating RTL for gimple basic block 2
;;
;; Full RTL generated for this function:
;;
(note 1 0 3 NOTE_INSN_DELETED)
(note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 2 3 4 2 NOTE_INSN_FUNCTION_BEG)
(note 4 2 5 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 5 4 14 3 sync_sync.c:4 (parallel [
(asm_operands/v ("") ("") 0 []
[] 477)
(clobber (mem:BLK (scratch) [0 A8]))
]) -1 (nil))
(note 14 5 8 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
(insn 8 14 9 4 sync_sync.c:5 (clobber (reg/i:SI 0 r0)) -1 (nil))
(insn 9 8 10 4 sync_sync.c:5 (clobber (reg:SI 133 [ <result> ])) -1 (nil))
(jump_insn 10 9 11 4 sync_sync.c:5 (set (pc)
(label_ref 12)) -1 (nil))
(barrier 11 10 6)
(code_label 6 11 15 5 1 "" [0 uses])
(note 15 6 7 5 [bb 5] NOTE_INSN_BASIC_BLOCK)
(insn 7 15 12 5 sync_sync.c:5 (set (reg/i:SI 0 r0)
(reg:SI 133 [ <result> ])) -1 (nil))
(code_label 12 7 16 6 2 "" [1 uses])
(note 16 12 13 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(insn 13 16 0 6 sync_sync.c:5 (use (reg/i:SI 0 r0)) -1 (nil))
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
