From d703c5893699b045adaf57d9cde324a24be965cd Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Mon, 27 Oct 2008 10:42:58 +0000 Subject: [PATCH] From Blasius Czink, "Among other things I added support for atomic operations on BSD-like systems and additional methods (for "and", "or", "xor"). " and a later post the same osg-submissions thread: "it's been a while since I have made the changes but I think it was due to problems with static builds of OpenThreads on windows. I was using OpenThreads in a communication/synchronisation library (without OpenSceneGraph). It seems I forgot to post a small change in the CMakeLists file of OpenThreads. If a user turns DYNAMIC_OPENTHREADS to OFF (static build) OT_LIBRARY_STATIC will be defined in the Config. Without these changes a windows user will always end up with a "__declspec(dllexport)" or "__declspec(dllimport)" which is a problem for static builds." And another post from Blasius on this topic: "I tested with VS2005 and VS2008. For 32 bit everything works as expected. For x64 and VS2008 I could successfully do the cmake-configure and then the compilation but I had occasional crashes of cmTryCompileExec.exe (during the cmake-configure phase) which seems to be a cmake bug. With VS2005 and 64bit cmake does not set _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED although the interlocked functionality should be there. If I place the source snippet from the CHECK_CXX_SOURCE_RUNS macro to a separate sourcefile I can compile and run the resulting executable successfully. Forcing OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED (on VS2005/x64) reveals a bug in "intrin.h" which seems to be fixed in VS2008 but not in VS2005. In case anyone is interested the lines: __MACHINEI(unsigned char _interlockedbittestandset(long *a, long b)) __MACHINEI(unsigned char _interlockedbittestandreset(long *a, long b)) __MACHINEX64(unsigned char _interlockedbittestandset64(__int64 *a, __int64 b)) __MACHINEX64(unsigned char _interlockedbittestandreset64(__int64 *a, __int64 b)) should be changed to: __MACHINEI(unsigned char _interlockedbittestandset(long volatile *a, long b)) __MACHINEI(unsigned char _interlockedbittestandreset(long volatile *a, long b)) __MACHINEX64(unsigned char _interlockedbittestandset64(__int64 volatile *a, __int64 b)) __MACHINEX64(unsigned char _interlockedbittestandreset64(__int64 volatile *a, __int64 b)) The worst thing that can happen is that interlocked funtionality is not detected during cmake-configure and the mutex fallback is used. Which reminds me another small glitch in the Atomic header so I attached a corrected version. Why is the OT_LIBRARY_STATIC added to the config file? It is not needed anywhere. OT_LIBRARY_STATIC is needed if you are doing static-builds on Windows. See my previous post on that. " --- CMakeModules/CheckAtomicOps.cmake | 22 ++++- include/OpenThreads/Atomic | 87 +++++++++++++++++++ include/OpenThreads/Exports | 1 + src/OpenThreads/CMakeLists.txt | 17 ++-- src/OpenThreads/common/Atomic.cpp | 76 +++++++++++++++- src/OpenThreads/common/Config.in | 2 + .../win32/Win32BarrierPrivateData.h | 1 + .../win32/Win32ConditionPrivateData.h | 33 +++---- 8 files changed, 212 insertions(+), 27 deletions(-) diff --git a/CMakeModules/CheckAtomicOps.cmake b/CMakeModules/CheckAtomicOps.cmake index 773130f37..043102322 100644 --- a/CMakeModules/CheckAtomicOps.cmake +++ b/CMakeModules/CheckAtomicOps.cmake @@ -90,6 +90,24 @@ int main(int, const char**) } " _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) -IF(NOT _OPENTHREADS_ATOMIC_USE_GCC_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_SUN AND NOT _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) +CHECK_CXX_SOURCE_RUNS(" +#include + +int main() +{ + volatile int32_t value = 0; + long data = 0; + long * volatile ptr = &data; + + OSAtomicIncrement32(&value); + OSMemoryBarrier(); + OSAtomicDecrement32(&value); + OSAtomicCompareAndSwapInt(value, 1, &value); + OSAtomicCompareAndSwapPtr(ptr, ptr, (void * volatile *)&ptr); +} +" _OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + + +IF(NOT _OPENTHREADS_ATOMIC_USE_GCC_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_SUN AND NOT _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED AND NOT _OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) SET(_OPENTHREADS_ATOMIC_USE_MUTEX 1) -ENDIF(NOT _OPENTHREADS_ATOMIC_USE_GCC_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_SUN AND NOT _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) +ENDIF(NOT _OPENTHREADS_ATOMIC_USE_GCC_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS AND NOT _OPENTHREADS_ATOMIC_USE_SUN AND NOT _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED AND NOT _OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) diff --git a/include/OpenThreads/Atomic b/include/OpenThreads/Atomic index f30aa9106..bd90cd306 100644 --- a/include/OpenThreads/Atomic +++ b/include/OpenThreads/Atomic @@ -23,6 +23,9 @@ #define _OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES #elif defined(_OPENTHREADS_ATOMIC_USE_SUN) # include +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) +# include +# define _OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES #elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) # include "Mutex" # include "ScopedLock" @@ -46,6 +49,10 @@ class OPENTHREAD_EXPORT_DIRECTIVE Atomic { { } _OPENTHREADS_ATOMIC_INLINE unsigned operator++(); _OPENTHREADS_ATOMIC_INLINE unsigned operator--(); + _OPENTHREADS_ATOMIC_INLINE unsigned AND(unsigned value); + _OPENTHREADS_ATOMIC_INLINE unsigned OR(unsigned value); + _OPENTHREADS_ATOMIC_INLINE unsigned XOR(unsigned value); + _OPENTHREADS_ATOMIC_INLINE unsigned exchange(unsigned value = 0); _OPENTHREADS_ATOMIC_INLINE operator unsigned() const; private: @@ -57,6 +64,8 @@ class OPENTHREAD_EXPORT_DIRECTIVE Atomic { #endif #if defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) volatile long _value; +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + volatile int32_t _value; #elif defined(_OPENTHREADS_ATOMIC_USE_SUN) volatile uint_t _value; #else @@ -125,6 +134,84 @@ Atomic::operator--() #endif } +_OPENTHREADS_ATOMIC_INLINE unsigned +Atomic::AND(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_fetch_and_and(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + return __and_and_fetch(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + return atomic_and_uint_nv(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + _value &= value; + return _value; +#else + _value &= value; + return _value; +#endif +} + +_OPENTHREADS_ATOMIC_INLINE unsigned +Atomic::OR(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_fetch_and_or(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + return __or_and_fetch(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + return atomic_or_uint_nv(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + _value |= value; + return _value; +#else + _value |= value; + return _value; +#endif +} + +_OPENTHREADS_ATOMIC_INLINE unsigned +Atomic::XOR(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_fetch_and_xor(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + return __xor_and_fetch(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + return atomic_xor_uint_nv(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + _value ^= value; + return _value; +#else + _value ^= value; + return _value; +#endif +} + +_OPENTHREADS_ATOMIC_INLINE unsigned +Atomic::exchange(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_lock_test_and_set(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + return __compare_and_swap(&_value, _value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + return atomic_cas_uint(&_value, _value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + unsigned oldval = _value; + _value = value; + return oldval; +#else + unsigned oldval = _value; + _value = value; + return oldval; +#endif +} + _OPENTHREADS_ATOMIC_INLINE Atomic::operator unsigned() const { diff --git a/include/OpenThreads/Exports b/include/OpenThreads/Exports index b388ee079..2a5414424 100644 --- a/include/OpenThreads/Exports +++ b/include/OpenThreads/Exports @@ -14,6 +14,7 @@ #ifndef _OPENTHREAD_EXPORTS_H_ #define _OPENTHREAD_EXPORTS_H_ +#include #ifndef WIN32 #define OPENTHREAD_EXPORT_DIRECTIVE diff --git a/src/OpenThreads/CMakeLists.txt b/src/OpenThreads/CMakeLists.txt index 65e3aa28e..b47e41cf1 100644 --- a/src/OpenThreads/CMakeLists.txt +++ b/src/OpenThreads/CMakeLists.txt @@ -11,6 +11,15 @@ SET(OPENTHREADS_VERSION ${OPENTHREADS_MAJOR_VERSION}.${OPENTHREADS_MINOR_VERSION INCLUDE(CheckAtomicOps) +# User Options +OPTION(DYNAMIC_OPENTHREADS "Set to ON to build OpenThreads for dynamic linking. Use OFF for static." ON) +IF (DYNAMIC_OPENTHREADS) + SET(OPENTHREADS_USER_DEFINED_DYNAMIC_OR_STATIC "SHARED") +ELSE (DYNAMIC_OPENTHREADS) + SET(OPENTHREADS_USER_DEFINED_DYNAMIC_OR_STATIC "STATIC") + SET(OT_LIBRARY_STATIC 1) +ENDIF (DYNAMIC_OPENTHREADS) + ################################################################################ # Set Config file @@ -38,14 +47,6 @@ SET(OpenThreads_PUBLIC_HEADERS ${OPENTHREADS_CONFIG_HEADER} ) -# User Options -OPTION(DYNAMIC_OPENTHREADS "Set to ON to build OpenThreads for dynamic linking. Use OFF for static." ON) -IF (DYNAMIC_OPENTHREADS) - SET(OPENTHREADS_USER_DEFINED_DYNAMIC_OR_STATIC "SHARED") -ELSE (DYNAMIC_OPENTHREADS) - SET(OPENTHREADS_USER_DEFINED_DYNAMIC_OR_STATIC "STATIC") -ENDIF(DYNAMIC_OPENTHREADS) - # Use our modified version of FindThreads.cmake which has Sproc hacks. FIND_PACKAGE(Threads) diff --git a/src/OpenThreads/common/Atomic.cpp b/src/OpenThreads/common/Atomic.cpp index f153e0852..95732afe9 100644 --- a/src/OpenThreads/common/Atomic.cpp +++ b/src/OpenThreads/common/Atomic.cpp @@ -14,7 +14,11 @@ #include #if defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) -# include +#include +#include +#pragma intrinsic(_InterlockedAnd) +#pragma intrinsic(_InterlockedOr) +#pragma intrinsic(_InterlockedXor) #endif namespace OpenThreads { @@ -40,6 +44,8 @@ Atomic::operator++() return __sync_add_and_fetch(&_value, 1); #elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) return InterlockedIncrement(&_value); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + return OSAtomicIncrement32(&_value); #else # error This implementation should happen inline in the include file #endif @@ -52,11 +58,71 @@ Atomic::operator--() return __sync_sub_and_fetch(&_value, 1); #elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) return InterlockedDecrement(&_value); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + return OSAtomicDecrement32(&_value); #else # error This implementation should happen inline in the include file #endif } +unsigned +Atomic::AND(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_fetch_and_and(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + return _InterlockedAnd(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + return OSAtomicAnd32((uint32_t)value, (uint32_t *)&_value); +#else +# error This implementation should happen inline in the include file +#endif +} + +unsigned +Atomic::OR(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_fetch_and_or(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + return _InterlockedOr(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + return OSAtomicOr32((uint32_t)value, (uint32_t *)&_value); +#else +# error This implementation should happen inline in the include file +#endif +} + +unsigned +Atomic::XOR(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_fetch_and_xor(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + return _InterlockedXor(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + return OSAtomicXor32((uint32_t)value, (uint32_t *)&_value); +#else +# error This implementation should happen inline in the include file +#endif +} + + +unsigned +Atomic::exchange(unsigned value) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_lock_test_and_set(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + return InterlockedExchange(&_value, value); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + return OSAtomicCompareAndSwap32(_value, value, &_value); +#else +# error This implementation should happen inline in the include file +#endif +} + + Atomic::operator unsigned() const { #if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) @@ -65,6 +131,9 @@ Atomic::operator unsigned() const #elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) MemoryBarrier(); return static_cast(_value); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + OSMemoryBarrier(); + return static_cast(_value); #else # error This implementation should happen inline in the include file #endif @@ -77,6 +146,8 @@ AtomicPtr::assign(void* ptrNew, const void* const ptrOld) return __sync_bool_compare_and_swap(&_ptr, ptrOld, ptrNew); #elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) return ptrOld == InterlockedCompareExchangePointer((PVOID volatile*)&_ptr, (PVOID)ptrNew, (PVOID)ptrOld); +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + return OSAtomicCompareAndSwapPtr((void *)ptrOld, (void *)ptrNew, (void* volatile *)&_ptr); #else # error This implementation should happen inline in the include file #endif @@ -91,6 +162,9 @@ AtomicPtr::get() const #elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) MemoryBarrier(); return _ptr; +#elif defined(_OPENTHREADS_ATOMIC_USE_BSD_ATOMIC) + OSMemoryBarrier(); + return _ptr; #else # error This implementation should happen inline in the include file #endif diff --git a/src/OpenThreads/common/Config.in b/src/OpenThreads/common/Config.in index 0ae5183e3..6406457d3 100644 --- a/src/OpenThreads/common/Config.in +++ b/src/OpenThreads/common/Config.in @@ -27,6 +27,8 @@ #cmakedefine _OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS #cmakedefine _OPENTHREADS_ATOMIC_USE_SUN #cmakedefine _OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED +#cmakedefine _OPENTHREADS_ATOMIC_USE_BSD_ATOMIC #cmakedefine _OPENTHREADS_ATOMIC_USE_MUTEX +#cmakedefine OT_LIBRARY_STATIC #endif diff --git a/src/OpenThreads/win32/Win32BarrierPrivateData.h b/src/OpenThreads/win32/Win32BarrierPrivateData.h index d474e9cf4..68d33a532 100644 --- a/src/OpenThreads/win32/Win32BarrierPrivateData.h +++ b/src/OpenThreads/win32/Win32BarrierPrivateData.h @@ -20,6 +20,7 @@ #ifndef _WINDOWS_ #define WIN32_LEAN_AND_MEAN +#define _WIN32_WINNT 0x0400 #include #endif diff --git a/src/OpenThreads/win32/Win32ConditionPrivateData.h b/src/OpenThreads/win32/Win32ConditionPrivateData.h index 5a14b74e0..c4a2f9e88 100644 --- a/src/OpenThreads/win32/Win32ConditionPrivateData.h +++ b/src/OpenThreads/win32/Win32ConditionPrivateData.h @@ -44,6 +44,7 @@ public: Win32ConditionPrivateData () :waiters_(0), + was_broadcast_(0), sema_(CreateSemaphore(NULL,0,0x7fffffff,NULL)), waiters_done_(CreateEvent(NULL,FALSE,FALSE,NULL)) { @@ -69,7 +70,7 @@ public: // Wake up all the waiters. ReleaseSemaphore(sema_.get(),waiters_,NULL); - cooperativeWait(waiters_done_.get(), INFINITE); + cooperativeWait(waiters_done_.get(), INFINITE); //end of broadcasting was_broadcast_ = 0; @@ -104,23 +105,23 @@ public: // wait in timeslices, giving testCancel() a change to // exit the thread if requested. - try { - DWORD dwResult = cooperativeWait(sema_.get(), timeout_ms); - if(dwResult != WAIT_OBJECT_0) - result = (int)dwResult; - } - catch(...){ - // thread is canceled in cooperative wait , do cleanup - InterlockedDecrement(&waiters_); - long w = InterlockedGet(&waiters_); - int last_waiter = was_broadcast_ && w == 0; + try { + DWORD dwResult = cooperativeWait(sema_.get(), timeout_ms); + if(dwResult != WAIT_OBJECT_0) + result = (int)dwResult; + } + catch(...){ + // thread is canceled in cooperative wait , do cleanup + InterlockedDecrement(&waiters_); + long w = InterlockedGet(&waiters_); + int last_waiter = was_broadcast_ && w == 0; - if (last_waiter) SetEvent(waiters_done_.get()); - // rethrow - throw; - } + if (last_waiter) SetEvent(waiters_done_.get()); + // rethrow + throw; + } - + // We're ready to return, so there's one less waiter. InterlockedDecrement(&waiters_); long w = InterlockedGet(&waiters_);