[llvm-commits] [llvm] r72087 - /llvm/trunk/include/llvm/System/Atomic.h

[Sebastian Redl]

Owen Anderson wrote:
> Author: resistor
> Date: Mon May 18 20:07:40 2009
> New Revision: 72087
>
> URL: http://llvm.org/viewvc/llvm-project?rev=72087&view=rev
> Log:
> Fix up the Windows portion of Atomic.h.  This is untested, but it is my best understanding of what should work.
> I'd be much obliged if someone on MSVC++ could try this out and let me know if it works.
>
> Modified:
>     llvm/trunk/include/llvm/System/Atomic.h
>
> Modified: llvm/trunk/include/llvm/System/Atomic.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/System/Atomic.h?rev=72087&r1=72086&r2=72087&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/System/Atomic.h (original)
> +++ llvm/trunk/include/llvm/System/Atomic.h Mon May 18 20:07:40 2009
> @@ -62,11 +62,24 @@
>      }
>  #elif defined(_MSC_VER)
>      typedef LONG cas_flag;
> +    template<typename T>
>      inline T CompareAndSwap(volatile T* ptr,
>  			    T new_value,
>  			    T old_value) {
> -      return InterlockedCompareExchange(addr, new_value, old_value);
> +      if (sizeof(T) == 4)
> +	return InterlockedCompareExchange(ptr, new_value, old_value);
> +      else
> +	return InterlockedCompareExchange64(ptr, new_value, old_value);
>      }
>   
I'm not booted into Windows, but this cannot possibly compile. An if
must have all branches valid even if the condition is a compile-time
constant. You could force some casts, or you could specialize the
function on 4-byte types. Eh, you'll need the casts anyway, because the
WinAPI sucks.

> +      else
> +	assert(0 && "Unsupported compare-and-swap size!");
>   

However, it would be really nice to catch this at compile time.
And another thing, the function is called InterlockedCompareExchangePointer.

The code below is untested, but should do the Right Thing.

template <typename T, size_t N>
struct CompareAndSwapImpl;

template <typename T>
struct CompareAndSwapImpl<T, 4> {
  static T DoCAS(volatile T* ptr, T n, T o) {
    return InterlockedCompareExchange(reinterpret_cast<LONG
volatile*>(ptr), n, o);
  }
}

template <typename T>
struct CompareAndSwapImpl<T, 8> {
  static T DoCAS(volatile T* ptr, T n, T o) {
    return InterlockedCompareExchange64(reinterpret_cast<LONGLONG
volatile*>(ptr), n, o);
  }
}

template <typename T>
struct CompareAndSwapImpl<T*, sizeof(T*)> {
  static T DoCAS(T* volatile* ptr, T* n, T* o) {
    return InterlockedCompareExchangePointer(reinterpret_cast<void*
volatile*>(ptr), n, o);
  }
}

template <typename T>
inline T CompareAndSwap(T volatile *ptr, T new_value, T old_value) {
  return CompareAndSwapImpl<T, sizeof(T)>::DoCAS(ptr, new_value, old_value);
}

Sebastian