Dave Korn wrote:
>   The attached patch implements ilockexch and ilockcmpexch, using the inline
> asm definition from __arch_compare_and_exchange_val_32_acq in
> glibc-2.10.1/sysdeps/i386/i486/bits/atomic.h, trivially expanded inline rather
> than in its original preprocessor macro form.
> 
>   It generates incorrect code.

  This much looks like it's probably a compiler bug.  This version, compiled
with current GCC HEAD, generates the same results as in the take 3 version,
without needing the explicit memory clobber added:

L469:
        .loc 2 127 0
        movl    __ZN13pthread_mutex7mutexesE+8, %eax     # mutexes.head, D.30413
        movl    %eax, 36(%ebx)   # D.30413, <variable>.next
        .loc 5 58 0
/APP
 # 58 "/gnu/winsup/src/winsup/cygwin/winbase.h" 1
        lock cmpxchgl %ebx, __ZN13pthread_mutex7mutexesE+8       # this,
 # 0 "" 2
/NO_APP
        movl    %eax, -12(%ebp)  # tmp76, ret
        .loc 5 59 0
        movl    -12(%ebp), %eax  # ret, D.30414
        .loc 2 126 0
        cmpl    %eax, 36(%ebx)   # D.30414, <variable>.next
        jne     L469     #,

... right down to the unoptimised register motion.  This is what we would have
hoped to see: the "memory" clobber ought to be superfluous, and the
write-output operand in *t should have told GCC not to move the store to
node->next after the loop.

  I checked 4.3.3; it behaves the same as 4.3.2, i.e. it incorrectly lowers
the store without a memory clobber present.

    cheers,
      DaveK

Reply via email to