Improvements based on feedback from Anthony Williams
authorSam Lantinga <slouken@libsdl.org>
Tue, 25 Jan 2011 17:40:06 -0800
changeset 5095dceec93471e7
parent 5094 124cda437b07
child 5096 e4301cde4de1
Improvements based on feedback from Anthony Williams
configure.in
include/SDL_atomic.h
src/atomic/SDL_atomic.c
src/atomic/SDL_spinlock.c
     1.1 --- a/configure.in	Mon Jan 24 23:54:21 2011 -0600
     1.2 +++ b/configure.in	Tue Jan 25 17:40:06 2011 -0800
     1.3 @@ -301,6 +301,7 @@
     1.4      int a;
     1.5      void *x, *y, *z;
     1.6      __sync_lock_test_and_set(&a, 4);
     1.7 +    __sync_lock_test_and_set(&x, y);
     1.8      __sync_fetch_and_add(&a, 1);
     1.9      __sync_bool_compare_and_swap(&a, 5, 10);
    1.10      __sync_bool_compare_and_swap(&x, y, z);
    1.11 @@ -317,6 +318,7 @@
    1.12          ],[
    1.13          int a;
    1.14          __sync_lock_test_and_set(&a, 1);
    1.15 +        __sync_lock_release(&a);
    1.16          ],[
    1.17          have_gcc_sync_lock_test_and_set=yes
    1.18          ])
     2.1 --- a/include/SDL_atomic.h	Mon Jan 24 23:54:21 2011 -0600
     2.2 +++ b/include/SDL_atomic.h	Tue Jan 25 17:40:06 2011 -0800
     2.3 @@ -38,8 +38,13 @@
     2.4   *  SDL_AtomicDecRef()
     2.5   * 
     2.6   * Seriously, here be dragons!
     2.7 + * ^^^^^^^^^^^^^^^^^^^^^^^^^^^
     2.8   *
     2.9 - * These operations may, or may not, actually be implemented using
    2.10 + * You can find out a little more about lockless programming and the 
    2.11 + * subtle issues that can arise here:
    2.12 + * http://msdn.microsoft.com/en-us/library/ee418650%28v=vs.85%29.aspx
    2.13 + *
    2.14 + * These operations may or may not actually be implemented using
    2.15   * processor specific atomic operations. When possible they are
    2.16   * implemented as true processor specific atomic operations. When that
    2.17   * is not possible the are implemented using locks that *do* use the
    2.18 @@ -114,66 +119,54 @@
    2.19  
    2.20  /*@}*//*SDL AtomicLock*/
    2.21  
    2.22 +
    2.23 +/* The compiler barrier prevents the compiler from reordering
    2.24 +   reads and writes to globally visible variables across the call.
    2.25 +*/
    2.26 +#ifdef _MSC_VER
    2.27 +void _ReadWriteBarrier(void);
    2.28 +#pragma intrinsic(_ReadWriteBarrier)
    2.29 +#define SDL_CompilerBarrier()   _ReadWriteBarrier()
    2.30 +#elif __GNUC__
    2.31 +#define SDL_CompilerBarrier()   __asm__ __volatile__ ("" : : : "memory")
    2.32 +#else
    2.33 +#define SDL_CompilerBarrier()   \
    2.34 +({ SDL_SpinLock _tmp = 0; SDL_AtomicLock(&_tmp); SDL_AtomicUnlock(&_tmp); })
    2.35 +#endif
    2.36 +
    2.37  /* Platform specific optimized versions of the atomic functions,
    2.38   * you can disable these by defining SDL_DISABLE_ATOMIC_INLINE
    2.39   */
    2.40  #ifndef SDL_DISABLE_ATOMIC_INLINE
    2.41  
    2.42 -#if defined(HAVE_MSC_ATOMICS)
    2.43 +#if HAVE_MSC_ATOMICS
    2.44  
    2.45  #define SDL_AtomicSet(a, v)     _InterlockedExchange((long*)&(a)->value, (v))
    2.46 -#define SDL_AtomicGet(a)        ((a)->value)
    2.47  #define SDL_AtomicAdd(a, v)     _InterlockedExchangeAdd((long*)&(a)->value, (v))
    2.48  #define SDL_AtomicCAS(a, oldval, newval) (_InterlockedCompareExchange((long*)&(a)->value, (newval), (oldval)) == (oldval))
    2.49 -#define SDL_AtomicSetPtr(a, v)  (void)_InterlockedExchangePointer((a), (v))
    2.50 -#define SDL_AtomicGetPtr(a)     (*(a))
    2.51 +#define SDL_AtomicSetPtr(a, v)  _InterlockedExchangePointer((a), (v))
    2.52  #if _M_IX86
    2.53  #define SDL_AtomicCASPtr(a, oldval, newval) (_InterlockedCompareExchange((long*)(a), (long)(newval), (long)(oldval)) == (long)(oldval))
    2.54  #else
    2.55  #define SDL_AtomicCASPtr(a, oldval, newval) (_InterlockedCompareExchangePointer((a), (newval), (oldval)) == (oldval))
    2.56  #endif
    2.57  
    2.58 -#elif defined(__MACOSX__)
    2.59 +#elif __MACOSX__
    2.60  #include <libkern/OSAtomic.h>
    2.61  
    2.62 -#define SDL_AtomicSet(a, v) \
    2.63 -({                          \
    2.64 -    int oldvalue;           \
    2.65 -                            \
    2.66 -    do {                    \
    2.67 -        oldvalue = (a)->value; \
    2.68 -    } while (!OSAtomicCompareAndSwap32Barrier(oldvalue, v, &(a)->value)); \
    2.69 -                            \
    2.70 -    oldvalue;               \
    2.71 -})
    2.72 -#define SDL_AtomicGet(a)        ((a)->value)
    2.73 -#define SDL_AtomicAdd(a, v) \
    2.74 -({                          \
    2.75 -    int oldvalue;           \
    2.76 -                            \
    2.77 -    do {                    \
    2.78 -        oldvalue = (a)->value; \
    2.79 -    } while (!OSAtomicCompareAndSwap32Barrier(oldvalue, oldvalue+v, &(a)->value)); \
    2.80 -                            \
    2.81 -    oldvalue;               \
    2.82 -})
    2.83 -#define SDL_AtomicCAS(a, oldval, newval) OSAtomicCompareAndSwap32Barrier(oldval, newval, &(a)->value)
    2.84 -#define SDL_AtomicSetPtr(a, v)  (*(a) = v, OSMemoryBarrier())
    2.85 -#define SDL_AtomicGetPtr(a)     (*(a))
    2.86 +#define SDL_AtomicCAS(a, oldval, newval) OSAtomicCompareAndSwap32Barrier((oldval), (newval), &(a)->value)
    2.87  #if SIZEOF_VOIDP == 4
    2.88  #define SDL_AtomicCASPtr(a, oldval, newval) OSAtomicCompareAndSwap32Barrier((int32_t)(oldval), (int32_t)(newval), (int32_t*)(a))
    2.89  #elif SIZEOF_VOIDP == 8
    2.90  #define SDL_AtomicCASPtr(a, oldval, newval) OSAtomicCompareAndSwap64Barrier((int64_t)(oldval), (int64_t)(newval), (int64_t*)(a))
    2.91  #endif
    2.92  
    2.93 -#elif defined(HAVE_GCC_ATOMICS)
    2.94 +#elif HAVE_GCC_ATOMICS
    2.95  
    2.96  #define SDL_AtomicSet(a, v)     __sync_lock_test_and_set(&(a)->value, v)
    2.97 -#define SDL_AtomicGet(a)        ((a)->value)
    2.98  #define SDL_AtomicAdd(a, v)     __sync_fetch_and_add(&(a)->value, v)
    2.99 +#define SDL_AtomicSetPtr(a, v)  __sync_lock_test_and_set(a, v)
   2.100  #define SDL_AtomicCAS(a, oldval, newval) __sync_bool_compare_and_swap(&(a)->value, oldval, newval)
   2.101 -#define SDL_AtomicSetPtr(a, v)  (*(a) = v, __sync_synchronize())
   2.102 -#define SDL_AtomicGetPtr(a)     (*(a))
   2.103  #define SDL_AtomicCASPtr(a, oldval, newval) __sync_bool_compare_and_swap(a, oldval, newval)
   2.104  
   2.105  #endif
   2.106 @@ -195,40 +188,61 @@
   2.107   * \return The previous value of the atomic variable.
   2.108   */
   2.109  #ifndef SDL_AtomicSet
   2.110 -extern DECLSPEC int SDLCALL SDL_AtomicSet(SDL_atomic_t *a, int value);
   2.111 +#define SDL_AtomicSet(a, v) \
   2.112 +({                              \
   2.113 +    int _value;                 \
   2.114 +    do {                        \
   2.115 +        _value = (a)->value;    \
   2.116 +    } while (!SDL_AtomicCAS(a, _value, (v))); \
   2.117 +    _value;                     \
   2.118 +})
   2.119  #endif
   2.120  
   2.121  /**
   2.122   * \brief Get the value of an atomic variable
   2.123   */
   2.124  #ifndef SDL_AtomicGet
   2.125 -extern DECLSPEC int SDLCALL SDL_AtomicGet(SDL_atomic_t *a);
   2.126 +#define SDL_AtomicGet(a)        \
   2.127 +({                              \
   2.128 +    int _value = (a)->value;    \
   2.129 +    SDL_CompilerBarrier();      \
   2.130 +    _value;                     \
   2.131 +})
   2.132  #endif
   2.133  
   2.134  /**
   2.135 - * \brief  Add to an atomic variable.
   2.136 + * \brief Add to an atomic variable.
   2.137   *
   2.138   * \return The previous value of the atomic variable.
   2.139 + *
   2.140 + * \note This same style can be used for any number operation
   2.141   */
   2.142  #ifndef SDL_AtomicAdd
   2.143 -extern DECLSPEC int SDLCALL SDL_AtomicAdd(SDL_atomic_t *a, int value);
   2.144 +#define SDL_AtomicAdd(a, v)     \
   2.145 +({                              \
   2.146 +    int _value;                 \
   2.147 +    do {                        \
   2.148 +        _value = (a)->value;    \
   2.149 +    } while (!SDL_AtomicCAS(a, _value, (_value + (v)))); \
   2.150 +    _value;                     \
   2.151 +})
   2.152  #endif
   2.153  
   2.154  /**
   2.155   * \brief Increment an atomic variable used as a reference count.
   2.156   */
   2.157  #ifndef SDL_AtomicIncRef
   2.158 -extern DECLSPEC void SDLCALL SDL_AtomicIncRef(SDL_atomic_t *a);
   2.159 +#define SDL_AtomicIncRef(a)    SDL_AtomicAdd(a, 1)
   2.160  #endif
   2.161  
   2.162  /**
   2.163   * \brief Decrement an atomic variable used as a reference count.
   2.164   *
   2.165 - * \return SDL_TRUE if the variable has reached zero after decrementing,
   2.166 + * \return SDL_TRUE if the variable reached zero after decrementing,
   2.167   *         SDL_FALSE otherwise
   2.168   */
   2.169  #ifndef SDL_AtomicDecRef
   2.170 -extern DECLSPEC SDL_bool SDLCALL SDL_AtomicDecRef(SDL_atomic_t *a);
   2.171 +#define SDL_AtomicDecRef(a)    (SDL_AtomicAdd(a, -1) == 1)
   2.172  #endif
   2.173  
   2.174  /**
   2.175 @@ -239,21 +253,36 @@
   2.176   * \note If you don't know what this function is for, you shouldn't use it!
   2.177  */
   2.178  #ifndef SDL_AtomicCAS
   2.179 -extern DECLSPEC SDL_bool SDLCALL SDL_AtomicCAS(SDL_atomic_t *a, int oldval, int newval);
   2.180 +#define SDL_AtomicCAS SDL_AtomicCAS_
   2.181  #endif
   2.182 +extern DECLSPEC SDL_bool SDLCALL SDL_AtomicCAS_(SDL_atomic_t *a, int oldval, int newval);
   2.183  
   2.184  /**
   2.185   * \brief Set a pointer to a value atomically.
   2.186 + *
   2.187 + * \return The previous value of the pointer.
   2.188   */
   2.189  #ifndef SDL_AtomicSetPtr
   2.190 -extern DECLSPEC void SDLCALL SDL_AtomicSetPtr(void** a, void* value);
   2.191 +#define SDL_AtomicSetPtr(a, v)  \
   2.192 +({                              \
   2.193 +    void* _value;               \
   2.194 +    do {                        \
   2.195 +        _value = *(a);          \
   2.196 +    } while (!SDL_AtomicCASPtr(a, _value, (v))); \
   2.197 +    _value;                     \
   2.198 +})
   2.199  #endif
   2.200  
   2.201  /**
   2.202   * \brief Get the value of a pointer atomically.
   2.203   */
   2.204  #ifndef SDL_AtomicGetPtr
   2.205 -extern DECLSPEC void* SDLCALL SDL_AtomicGetPtr(void** a);
   2.206 +#define SDL_AtomicGetPtr(a)     \
   2.207 +({                              \
   2.208 +    void* _value = *(a);        \
   2.209 +    SDL_CompilerBarrier();      \
   2.210 +    _value;                     \
   2.211 +})
   2.212  #endif
   2.213  
   2.214  /**
   2.215 @@ -264,8 +293,9 @@
   2.216   * \note If you don't know what this function is for, you shouldn't use it!
   2.217  */
   2.218  #ifndef SDL_AtomicCASPtr
   2.219 -extern DECLSPEC SDL_bool SDLCALL SDL_AtomicCASPtr(void **a, void *oldval, void *newval);
   2.220 +#define SDL_AtomicCASPtr SDL_AtomicCASPtr_
   2.221  #endif
   2.222 +extern DECLSPEC SDL_bool SDLCALL SDL_AtomicCASPtr_(void **a, void *oldval, void *newval);
   2.223  
   2.224  /* Ends C function definitions when using C++ */
   2.225  #ifdef __cplusplus
     3.1 --- a/src/atomic/SDL_atomic.c	Mon Jan 24 23:54:21 2011 -0600
     3.2 +++ b/src/atomic/SDL_atomic.c	Tue Jan 25 17:40:06 2011 -0800
     3.3 @@ -70,61 +70,8 @@
     3.4      SDL_AtomicUnlock(&locks[index]);
     3.5  }
     3.6  
     3.7 -#undef SDL_AtomicSet
     3.8 -int
     3.9 -SDL_AtomicSet(SDL_atomic_t *a, int value)
    3.10 -{
    3.11 -    int oldvalue;
    3.12 -
    3.13 -    enterLock(a);
    3.14 -    oldvalue = a->value;
    3.15 -    a->value = value;
    3.16 -    leaveLock(a);
    3.17 -
    3.18 -    return oldvalue;
    3.19 -}
    3.20 -
    3.21 -#undef SDL_AtomicGet
    3.22 -int
    3.23 -SDL_AtomicGet(SDL_atomic_t *a)
    3.24 -{
    3.25 -    /* Assuming integral reads on this platform, we're safe here since the
    3.26 -       functions that set the variable have the necessary memory barriers.
    3.27 -    */
    3.28 -    return a->value;
    3.29 -}
    3.30 -
    3.31 -#undef SDL_AtomicAdd
    3.32 -int
    3.33 -SDL_AtomicAdd(SDL_atomic_t *a, int value)
    3.34 -{
    3.35 -    int oldvalue;
    3.36 -
    3.37 -    enterLock(a);
    3.38 -    oldvalue = a->value;
    3.39 -    a->value += value;
    3.40 -    leaveLock(a);
    3.41 -
    3.42 -    return oldvalue;
    3.43 -}
    3.44 -
    3.45 -#undef SDL_AtomicIncRef
    3.46 -void
    3.47 -SDL_AtomicIncRef(SDL_atomic_t *a)
    3.48 -{
    3.49 -    SDL_AtomicAdd(a, 1);
    3.50 -}
    3.51 -
    3.52 -#undef SDL_AtomicDecRef
    3.53  SDL_bool
    3.54 -SDL_AtomicDecRef(SDL_atomic_t *a)
    3.55 -{
    3.56 -    return SDL_AtomicAdd(a, -1) == 1;
    3.57 -}
    3.58 -
    3.59 -#undef SDL_AtomicCAS
    3.60 -SDL_bool
    3.61 -SDL_AtomicCAS(SDL_atomic_t *a, int oldval, int newval)
    3.62 +SDL_AtomicCAS_(SDL_atomic_t *a, int oldval, int newval)
    3.63  {
    3.64      SDL_bool retval = SDL_FALSE;
    3.65  
    3.66 @@ -138,28 +85,8 @@
    3.67      return retval;
    3.68  }
    3.69  
    3.70 -#undef SDL_AtomicSetPtr
    3.71 -void
    3.72 -SDL_AtomicSetPtr(void** a, void* value)
    3.73 -{
    3.74 -    void *prevval;
    3.75 -    do {
    3.76 -        prevval = *a;
    3.77 -    } while (!SDL_AtomicCASPtr(a, prevval, value));
    3.78 -}
    3.79 -
    3.80 -#undef SDL_AtomicGetPtr
    3.81 -void*
    3.82 -SDL_AtomicGetPtr(void** a)
    3.83 -{
    3.84 -    /* Assuming integral reads on this platform, we're safe here since the
    3.85 -       functions that set the pointer have the necessary memory barriers.
    3.86 -    */
    3.87 -    return *a;
    3.88 -}
    3.89 -
    3.90 -#undef SDL_AtomicCASPtr
    3.91 -SDL_bool SDL_AtomicCASPtr(void **a, void *oldval, void *newval)
    3.92 +SDL_bool
    3.93 +SDL_AtomicCASPtr_(void **a, void *oldval, void *newval)
    3.94  {
    3.95      SDL_bool retval = SDL_FALSE;
    3.96  
     4.1 --- a/src/atomic/SDL_spinlock.c	Mon Jan 24 23:54:21 2011 -0600
     4.2 +++ b/src/atomic/SDL_spinlock.c	Tue Jan 25 17:40:06 2011 -0800
     4.3 @@ -37,20 +37,20 @@
     4.4      SDL_COMPILE_TIME_ASSERT(locksize, sizeof(*lock) == sizeof(long));
     4.5      return (InterlockedExchange((long*)lock, 1) == 0);
     4.6  
     4.7 -#elif defined(__MACOSX__)
     4.8 +#elif __MACOSX__
     4.9      return OSAtomicCompareAndSwap32Barrier(0, 1, lock);
    4.10  
    4.11 -#elif defined(HAVE_GCC_ATOMICS) || defined(HAVE_GCC_SYNC_LOCK_TEST_AND_SET)
    4.12 +#elif HAVE_GCC_ATOMICS || HAVE_GCC_SYNC_LOCK_TEST_AND_SET
    4.13      return (__sync_lock_test_and_set(lock, 1) == 0);
    4.14  
    4.15 -#elif defined(__GNUC__) && defined(__arm__) && defined(__ARM_ARCH_5__)
    4.16 +#elif __GNUC__ && __arm__ && __ARM_ARCH_5__
    4.17      int result;
    4.18      __asm__ __volatile__ (
    4.19          "swp %0, %1, [%2]\n"
    4.20          : "=&r,&r" (result) : "r,0" (1), "r,r" (lock) : "memory");
    4.21      return (result == 0);
    4.22  
    4.23 -#elif defined(__GNUC__) && defined(__arm__)
    4.24 +#elif __GNUC__ && __arm__
    4.25      int result;
    4.26      __asm__ __volatile__ (
    4.27          "ldrex %0, [%2]\nteq   %0, #0\nstrexeq %0, %1, [%2]"
    4.28 @@ -75,8 +75,16 @@
    4.29  void
    4.30  SDL_AtomicUnlock(SDL_SpinLock *lock)
    4.31  {
    4.32 -    /* Assuming atomic assignment operation and full memory barrier in lock */
    4.33 +#if defined(_MSC_VER)
    4.34 +    _ReadWriteBarrier();
    4.35      *lock = 0;
    4.36 +
    4.37 +#elif HAVE_GCC_ATOMICS || HAVE_GCC_SYNC_LOCK_TEST_AND_SET
    4.38 +    __sync_lock_release(lock);
    4.39 +
    4.40 +#else
    4.41 +    *lock = 0;
    4.42 +#endif
    4.43  }
    4.44  
    4.45  /* vi: set ts=4 sw=4 expandtab: */