This is the mail archive of the cygwin-patches mailing list for the Cygwin 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?] Separate pthread patches, #2 take 2.


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


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