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