Re: [PATCH?] Separate pthread patches, #2 take 2.

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

Corinna Vinschen wrote:
> On Jun  4 02:52, Dave Korn wrote:
>> Dave Korn wrote:
>>> 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.  
>>   Let's see whether anyone else agrees:
>>
>>         http://gcc.gnu.org/ml/gcc/2009-06/msg00053.html
> 
> When you checked in this change, I'll create a 1.7.0-49 test release.

  This is the final version I committed.  It is the glibc version, with the
addition of the memory clobber, which that discussion revealed was absolutely
required, and the use of a register asm var to feed the inline asm, which is
in accordance with documented practice in the gcc manual.  (This now leaves
only one difference between the glibc version and the version I posted, which
is the use of a "+a" write-only output constraint paired with a numeric "0"
matching input constraint in glibc's version compared with a single output
operand using the read-write constraing "=a" in my version.  These should in
fact be exactly identical in terms of what they indicate to reload, in any case.)

  I have also manually inspected the generated assembly from thread.cc and
shared.cc in a cygwin DLL build and verified that it is correct and efficient,
and have installed the resulting DLL and retested all Thomas Stalder's
testcases and the previously intermittently failing pthread7-rope testcase
from libstdc++ testsuite.  Committed with this ChangeLog:

winsup/cygwin/ChangeLog

	* winbase.h (ilockexch):  Fix asm constraints.
	(ilockcmpexch):  Likewise.


  Libstdc++ plan after the weekend.  Cheers all!

    cheers,
      DaveK


? winsup/cygwin/cygwin-cxx.h
? winsup/cygwin/mutex
Index: winsup/cygwin/winbase.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/winbase.h,v
retrieving revision 1.14
diff -p -u -r1.14 winbase.h
--- winsup/cygwin/winbase.h	12 Jul 2008 18:09:17 -0000	1.14
+++ winsup/cygwin/winbase.h	5 Jun 2009 13:05:08 -0000
@@ -38,22 +38,31 @@ ilockdecr (volatile long *m)
 extern __inline__ long
 ilockexch (volatile long *t, long v)
 {
-  register int __res;
-  __asm__ __volatile__ ("\n\
-1:	lock	cmpxchgl %3,(%1)\n\
-	jne 1b\n\
- 	": "=a" (__res), "=q" (t): "1" (t), "q" (v), "0" (*t): "cc");
-  return __res;
+  return
+  ({
+    register __typeof (*t) ret __asm ("%eax");
+    __asm __volatile ("\n"
+	"1:	lock cmpxchgl %2, %1\n"
+	"	jne  1b\n"
+	: "=a" (ret), "=m" (*t)
+	: "r" (v), "m" (*t), "0" (*t)
+	: "memory");
+    ret;
+  });
 }
 
 extern __inline__ long
 ilockcmpexch (volatile long *t, long v, long c)
 {
-  register int __res;
-  __asm__ __volatile__ ("\n\
-	lock cmpxchgl %3,(%1)\n\
-	": "=a" (__res), "=q" (t) : "1" (t), "q" (v), "0" (c): "cc");
-  return __res;
+  return
+  ({
+    register __typeof (*t) ret __asm ("%eax");
+    __asm __volatile ("lock cmpxchgl %2, %1"
+	: "=a" (ret), "=m" (*t)
+	: "r" (v), "m" (*t), "0" (c)
+	: "memory");
+    ret;
+  });
 }
 
 #undef InterlockedIncrement