Adjust TAS assembly as per recent discussions: use "+m"(*lock) everywhere

to reference the spinlock variable, and specify "memory" as a clobber
operand to be sure gcc does not try to keep shared-memory values in
registers across a spinlock acquisition.  Also tighten the S/390 asm
sequence, which was apparently written with only minimal study of the
gcc asm documentation.  I have personally tested i386, ia64, ppc, hppa,
and s390 variants --- there is some small chance that I broke the others,
but I doubt it.

diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index f8bcf5e96a9cf2773cac4c41e6591d6f2d953489..0725fd06d24caed01607a74bd1b9dbc511e0e8cf 100644 (file)
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -66,7 +66,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *   $PostgreSQL: pgsql/src/include/storage/s_lock.h,v 1.125 2004/01/03 05:47:44 tgl Exp $
+ *   $PostgreSQL: pgsql/src/include/storage/s_lock.h,v 1.126 2004/06/19 23:02:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -85,16 +85,26 @@
  * Other compilers use __cpu or __cpu__ so we test for both in those cases.
  */
 
-/*
- * Standard gcc asm format is:
- *
+/*----------
+ * Standard gcc asm format (assuming "volatile slock_t *lock"):
+
    __asm__ __volatile__(
-       "   command \n"
-       "   command \n"
-       "   command \n"
-:      "=r"(_res)          return value, in register
-:      "r"(lock)           argument, 'lock pointer', in register
-:      "r0");              inline code uses this register
+       "   instruction \n"
+       "   instruction \n"
+       "   instruction \n"
+:      "=r"(_res), "+m"(*lock)     // return register, in/out lock value
+:      "r"(lock)                   // lock pointer, in input register
+:      "memory", "cc");            // show clobbered registers here
+
+ * The output-operands list (after first colon) should always include
+ * "+m"(*lock), whether or not the asm code actually refers to this
+ * operand directly.  This ensures that gcc believes the value in the
+ * lock variable is used and set by the asm code.  Also, the clobbers
+ * list (after third colon) should always include "memory"; this prevents
+ * gcc from thinking it can cache the values of shared-memory fields
+ * across the asm code.  Add "cc" if your asm code changes the condition
+ * code register, and also list any temp registers the code uses.
+ *----------
  */
 
 
@@ -117,8 +127,9 @@ tas(volatile slock_t *lock)
        "   lock            \n"
        "   xchgb   %0,%1   \n"
        "1: \n"
-:      "=q"(_res), "=m"(*lock)
-:      "0"(_res));
+:      "+q"(_res), "+m"(*lock)
+:
+:      "memory", "cc");
    return (int) _res;
 }
 
@@ -128,8 +139,7 @@ static __inline__ void
 spin_delay(void)
 {
    __asm__ __volatile__(
-       " rep; nop          \n"
-       : : : "memory");
+       " rep; nop          \n");
 }
 
 #endif  /* __i386__ || __x86_64__ */
@@ -150,10 +160,9 @@ tas(volatile slock_t *lock)
 
    __asm__ __volatile__(
        "   xchg4   %0=%1,%2    \n"
-:      "=r"(ret), "=m"(*lock)
-:      "r"(1), "1"(*lock)
+:      "=r"(ret), "+m"(*lock)
+:      "r"(1)
 :      "memory");
-
    return (int) ret;
 }
 
@@ -173,46 +182,18 @@ tas(volatile slock_t *lock)
    register slock_t _res = 1;
 
    __asm__ __volatile__(
-       "   swpb    %0, %0, [%3]    \n"
-:      "=r"(_res), "=m"(*lock)
-:      "0"(_res), "r"(lock));
+       "   swpb    %0, %0, [%2]    \n"
+:      "+r"(_res), "+m"(*lock)
+:      "r"(lock)
+:      "memory");
    return (int) _res;
 }
 
 #endif  /* __arm__ */
 
 
-#if defined(__s390__) && !defined(__s390x__)
-/* S/390 Linux */
-#define HAS_TEST_AND_SET
-
-typedef unsigned int slock_t;
-
-#define TAS(lock)     tas(lock)
-
-static __inline__ int
-tas(volatile slock_t *lock)
-{
-   int         _res;
-
-   __asm__ __volatile__(
-       "   la  1,1         \n"
-       "   l   2,%2        \n"
-       "   slr 0,0         \n"
-       "   cs  0,1,0(2)    \n"
-       "   lr  %1,0        \n"
-:      "=m"(lock), "=d"(_res)
-:      "m"(lock)
-:      "0", "1", "2");
-
-   return (_res);
-}
-
-#endif  /* __s390__ */
-
-
-#if defined(__s390x__)
-/* S/390x Linux (64-bit zSeries) */
+#if defined(__s390__) || defined(__s390x__)
+/* S/390 and S/390x Linux (32- and 64-bit zSeries) */
 #define HAS_TEST_AND_SET
 
 typedef unsigned int slock_t;
@@ -222,22 +203,17 @@ typedef unsigned int slock_t;
 static __inline__ int
 tas(volatile slock_t *lock)
 {
-   int         _res;
+   int         _res = 0;
 
    __asm__ __volatile__(
-       "   la  1,1         \n"
-       "   lg  2,%2        \n"
-       "   slr 0,0         \n"
-       "   cs  0,1,0(2)    \n"
-       "   lr  %1,0        \n"
-:      "=m"(lock), "=d"(_res)
-:      "m"(lock)
-:      "0", "1", "2");
-
-   return (_res);
+       "   cs  %0,%3,0(%2)     \n"
+:      "+d"(_res), "+m"(*lock)
+:      "a"(lock), "d"(1)
+:      "memory", "cc");
+   return _res;
 }
 
-#endif  /* __s390x__ */
+#endif  /* __s390__ || __s390x__ */
 
 
 #if defined(__sparc__)
@@ -250,12 +226,13 @@ typedef unsigned char slock_t;
 static __inline__ int
 tas(volatile slock_t *lock)
 {
-   register slock_t _res = 1;
+   register slock_t _res;
 
    __asm__ __volatile__(
        "   ldstub  [%2], %0    \n"
-:      "=r"(_res), "=m"(*lock)
-:      "r"(lock));
+:      "=r"(_res), "+m"(*lock)
+:      "r"(lock)
+:      "memory");
    return (int) _res;
 }
 
@@ -283,11 +260,11 @@ tas(volatile slock_t *lock)
    int _res;
 
    __asm__ __volatile__(
-"  lwarx   %0,0,%2     \n"
+"  lwarx   %0,0,%3     \n"
 "  cmpwi   %0,0        \n"
 "  bne     1f          \n"
 "  addi    %0,%0,1     \n"
-"  stwcx.  %0,0,%2     \n"
+"  stwcx.  %0,0,%3     \n"
 "  beq     2f          \n"
 "1:    li      %1,1        \n"
 "  b       3f          \n"
@@ -296,10 +273,9 @@ tas(volatile slock_t *lock)
 "  li      %1,0        \n"
 "3:                        \n"
 
-:  "=&r" (_t), "=r" (_res)
-:  "r" (lock)
-:  "cc", "memory"
-   );
+:  "=&r"(_t), "=r"(_res), "+m"(*lock)
+:  "r"(lock)
+:  "memory", "cc");
    return _res;
 }
 
@@ -330,10 +306,9 @@ tas(volatile slock_t *lock)
        "   clrl    %0      \n"
        "   tas     %1      \n"
        "   sne     %0      \n"
-:      "=d"(rv), "=m"(*lock)
-:      "1"(*lock)
-:      "cc");
-
+:      "=d"(rv), "+m"(*lock)
+:
+:      "memory", "cc");
    return rv;
 }
 
@@ -357,13 +332,13 @@ tas(volatile slock_t *lock)
    register int    _res;
 
    __asm__ __volatile__(
-       "   movl    $1, r0          \n"
-       "   bbssi   $0, (%1), 1f    \n"
-       "   clrl    r0              \n"
-       "1: movl    r0, %0          \n"
-:      "=r"(_res)
+       "   movl    $1, %0          \n"
+       "   bbssi   $0, (%2), 1f    \n"
+       "   clrl    %0              \n"
+       "1: \n"
+:      "=&r"(_res), "+m"(*lock)
 :      "r"(lock)
-:      "r0");
+:      "memory");
    return _res;
 }
 
@@ -383,9 +358,11 @@ tas(volatile slock_t *lock)
    register int    _res;
 
    __asm__ __volatile__(
-       "   sbitb   0, %0   \n"
-       "   sfsd    %1      \n"
-:      "=m"(*lock), "=r"(_res));
+       "   sbitb   0, %1   \n"
+       "   sfsd    %0      \n"
+:      "=r"(_res), "+m"(*lock)
+:
+:      "memory");
    return _res;
 }
 
@@ -404,12 +381,6 @@ tas(volatile slock_t *lock)
 typedef unsigned long slock_t;
 
 #define TAS(lock)  tas(lock)
-#define S_UNLOCK(lock) \
-do \
-{\
-   __asm__ __volatile__ (" mb \n"); \
-   *((volatile slock_t *) (lock)) = 0; \
-} while (0)
 
 static __inline__ int
 tas(volatile slock_t *lock)
@@ -417,24 +388,30 @@ tas(volatile slock_t *lock)
    register slock_t _res;
 
    __asm__ __volatile__(
-       "   ldq     $0, %0  \n"
+       "   ldq     $0, %1  \n"
        "   bne     $0, 2f  \n"
-       "   ldq_l   %1, %0  \n"
-       "   bne     %1, 2f  \n"
+       "   ldq_l   %0, %1  \n"
+       "   bne     %0, 2f  \n"
        "   mov     1,  $0  \n"
-       "   stq_c   $0, %0  \n"
+       "   stq_c   $0, %1  \n"
        "   beq     $0, 2f  \n"
        "   mb              \n"
        "   br      3f      \n"
-       "2: mov     1, %1   \n"
+       "2: mov     1, %0   \n"
        "3:                 \n"
-:      "=m"(*lock), "=r"(_res)
+:      "=&r"(_res), "+m"(*lock)
 :
-:      "0");
-
+:      "memory", "0");
    return (int) _res;
 }
 
+#define S_UNLOCK(lock) \
+do \
+{\
+   __asm__ __volatile__ (" mb \n"); \
+   *((volatile slock_t *) (lock)) = 0; \
+} while (0)
+
 #endif /* __alpha || __alpha__ */
 
 
@@ -540,8 +517,9 @@ tas(volatile slock_t *lock)
 
    __asm__ __volatile__(
        "   ldcwx   0(0,%2),%0  \n"
-:      "=r"(lockval), "=m"(*lockword)
-:      "r"(lockword));
+:      "=r"(lockval), "+m"(*lockword)
+:      "r"(lockword)
+:      "memory");
    return (lockval == 0);
 }