From 5a4ce5a3878a21ef181e6ab4f3c4472fab909c74 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Thu, 26 Jun 2008 10:27:16 +0000 Subject: [PATCH] From Mathias Froechlich, "Attached is a change to that atomic stuff to move the win32, msvc implementation of the atomic increment and decrement into a implementation file. This way inlining and compiler optimization can no longer happen for these implementations, but it fixes compilation on win32 msvc targets. I expect that this is still faster than with with mutexes. Also the i386 gcc target gets atomic operations with this patch. By using an implementation file we can guarantee that we have the right compiler flags available." --- CMakeModules/CheckAtomicOps.cmake | 15 +- include/OpenThreads/Atomic | 227 +++++++++++++----------- include/osg/Referenced | 2 +- src/OpenThreads/common/Atomic.cpp | 101 +++++++++++ src/OpenThreads/pthreads/CMakeLists.txt | 1 + src/osg/Referenced.cpp | 8 +- 6 files changed, 231 insertions(+), 123 deletions(-) create mode 100644 src/OpenThreads/common/Atomic.cpp diff --git a/CMakeModules/CheckAtomicOps.cmake b/CMakeModules/CheckAtomicOps.cmake index 9cfd899c0..71c7eb788 100644 --- a/CMakeModules/CheckAtomicOps.cmake +++ b/CMakeModules/CheckAtomicOps.cmake @@ -10,14 +10,6 @@ CHECK_CXX_SOURCE_RUNS(" int main() { -#ifdef __i386__ - // Bad, gcc behaves dependent on the compilers -march=... flags. - // Since the osg::Referenced stuff is code distributed in headers, it is - // unclear if we will have this feature available at compile time of the - // headers. So just do not use this feature for 32 bit code. - // May be we can implement around that limitation at some time.. - return EXIT_FAILURE; -#else unsigned value = 0; void* ptr = &value; __sync_add_and_fetch(&value, 1); @@ -30,12 +22,11 @@ int main() return EXIT_FAILURE; return EXIT_SUCCESS; -#endif } " _OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) CHECK_CXX_SOURCE_RUNS(" -#include +#include int main(int, const char**) { @@ -44,10 +35,10 @@ int main(int, const char**) __add_and_fetch(&value, 1); __synchronize(value); __sub_and_fetch(&value, 1); - if (!__sync_compare_and_swap(&value, 0, 1)) + if (!__compare_and_swap(&value, 0, 1)) return EXIT_FAILURE; - if (!__sync_compare_and_swap(&ptr, ptr, ptr)) + if (!__compare_and_swap((unsigned long*)&ptr, (unsigned long)ptr, (unsigned long)ptr)) return EXIT_FAILURE; return EXIT_SUCCESS; diff --git a/include/OpenThreads/Atomic b/include/OpenThreads/Atomic index b29e475d8..f30aa9106 100644 --- a/include/OpenThreads/Atomic +++ b/include/OpenThreads/Atomic @@ -15,9 +15,12 @@ #define _OPENTHREADS_ATOMIC_ #include +#include -#if defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) -# include +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) && defined(__i386__) +#define _OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) +#define _OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES #elif defined(_OPENTHREADS_ATOMIC_USE_SUN) # include #elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) @@ -25,71 +28,25 @@ # include "ScopedLock" #endif +#if defined(_OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES) +#define _OPENTHREADS_ATOMIC_INLINE +#else +#define _OPENTHREADS_ATOMIC_INLINE inline +#endif + namespace OpenThreads { /** * @class Atomic * @brief This class provides an atomic increment and decrement operation. */ -class Atomic { +class OPENTHREAD_EXPORT_DIRECTIVE Atomic { public: Atomic(unsigned value = 0) : _value(value) { } - unsigned operator++() - { -#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) - return __sync_add_and_fetch(&_value, 1); -#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) - return __add_and_fetch(&_value, 1); -#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) - return atomic_inc_uint_nv(&_value); -#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) - return InterlockedIncrement(&_value); -#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) - ScopedLock lock(_mutex); - return ++_value; -#else - return ++_value; -#endif - } - unsigned operator--() - { -#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) - return __sync_sub_and_fetch(&_value, 1); -#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) - return __sub_and_fetch(&_value, 1); -#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) - return atomic_dec_uint_nv(&_value); -#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) - return InterlockedDecrement(&_value); -#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) - ScopedLock lock(_mutex); - return --_value; -#else - return --_value; -#endif - } - operator unsigned() const - { -#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) - __sync_synchronize(); - return _value; -#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) - __synchronize(_value); - return _value; -#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) - membar_consumer(); // Hmm, do we need??? - return _value; -#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) - return static_cast(_value); -#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) - ScopedLock lock(_mutex); - return _value; -#else - return _value; -#endif - } - + _OPENTHREADS_ATOMIC_INLINE unsigned operator++(); + _OPENTHREADS_ATOMIC_INLINE unsigned operator--(); + _OPENTHREADS_ATOMIC_INLINE operator unsigned() const; private: Atomic(const Atomic&); @@ -111,59 +68,16 @@ class Atomic { * @class AtomicPtr * @brief This class provides an atomic pointer assignment using cas operations. */ -template -class AtomicPtr { +class OPENTHREAD_EXPORT_DIRECTIVE AtomicPtr { public: - AtomicPtr(T* ptr = 0) : _ptr(ptr) + AtomicPtr(void* ptr = 0) : _ptr(ptr) { } ~AtomicPtr() { _ptr = 0; } // assigns a new pointer - bool assign(T* ptrNew, const T* const ptrOld) - { -#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) - return __sync_bool_compare_and_swap(&_ptr, ptrOld, ptrNew); -#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) - return __compare_and_swap(&_ptr, ptrOld, ptrNew); -#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) - return ptrOld == atomic_cas_ptr(&_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_MUTEX) - ScopedLock lock(_mutex); - if (_ptr != ptrOld) - return false; - _ptr = ptrNew; - return true; -#else - if (_ptr != ptrOld) - return false; - _ptr = ptrNew; - return true; -#endif - } - - T* get() const - { -#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) - __sync_synchronize(); - return _ptr; -#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) - __synchronize(_ptr); - return _ptr; -#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) - membar_consumer(); // Hmm, do we need??? - return _ptr; -#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) - return _ptr; -#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) - ScopedLock lock(_mutex); - return _ptr; -#else - return _ptr; -#endif - } + _OPENTHREADS_ATOMIC_INLINE bool assign(void* ptrNew, const void* const ptrOld); + _OPENTHREADS_ATOMIC_INLINE void* get() const; private: AtomicPtr(const AtomicPtr&); @@ -172,9 +86,110 @@ private: #if defined(_OPENTHREADS_ATOMIC_USE_MUTEX) mutable Mutex _mutex; #endif - T* volatile _ptr; + void* volatile _ptr; }; +#if !defined(_OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES) + +_OPENTHREADS_ATOMIC_INLINE unsigned +Atomic::operator++() +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_add_and_fetch(&_value, 1); +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + return __add_and_fetch(&_value, 1); +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + return atomic_inc_uint_nv(&_value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + return ++_value; +#else + return ++_value; +#endif +} + +_OPENTHREADS_ATOMIC_INLINE unsigned +Atomic::operator--() +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_sub_and_fetch(&_value, 1); +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + return __sub_and_fetch(&_value, 1); +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + return atomic_dec_uint_nv(&_value); +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + return --_value; +#else + return --_value; +#endif +} + +_OPENTHREADS_ATOMIC_INLINE +Atomic::operator unsigned() const +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + __sync_synchronize(); + return _value; +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + __synchronize(); + return _value; +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + membar_consumer(); // Hmm, do we need??? + return _value; +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + return _value; +#else + return _value; +#endif +} + +_OPENTHREADS_ATOMIC_INLINE bool +AtomicPtr::assign(void* ptrNew, const void* const ptrOld) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_bool_compare_and_swap(&_ptr, ptrOld, ptrNew); +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + return __compare_and_swap((unsigned long*)&_ptr, (unsigned long)ptrOld, (unsigned long)ptrNew); +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + return ptrOld == atomic_cas_ptr(&_ptr, ptrOld, ptrNew); +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + if (_ptr != ptrOld) + return false; + _ptr = ptrNew; + return true; +#else + if (_ptr != ptrOld) + return false; + _ptr = ptrNew; + return true; +#endif +} + +_OPENTHREADS_ATOMIC_INLINE void* +AtomicPtr::get() const +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + __sync_synchronize(); + return _ptr; +#elif defined(_OPENTHREADS_ATOMIC_USE_MIPOSPRO_BUILTINS) + __synchronize(); + return _ptr; +#elif defined(_OPENTHREADS_ATOMIC_USE_SUN) + membar_consumer(); // Hmm, do we need??? + return _ptr; +#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX) + ScopedLock lock(_mutex); + return _ptr; +#else + return _ptr; +#endif +} + +#endif // !defined(_OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES) + } #endif // _OPENTHREADS_ATOMIC_ diff --git a/include/osg/Referenced b/include/osg/Referenced index 357c13689..7835ac1da 100644 --- a/include/osg/Referenced +++ b/include/osg/Referenced @@ -129,7 +129,7 @@ class OSG_EXPORT Referenced #if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS) struct ObserverSetData; - OpenThreads::AtomicPtr _observerSetDataPtr; + OpenThreads::AtomicPtr _observerSetDataPtr; mutable OpenThreads::Atomic _refCount; #else diff --git a/src/OpenThreads/common/Atomic.cpp b/src/OpenThreads/common/Atomic.cpp new file mode 100644 index 000000000..f153e0852 --- /dev/null +++ b/src/OpenThreads/common/Atomic.cpp @@ -0,0 +1,101 @@ +/* -*-c++-*- OpenThreads library, Copyright (C) 2008 The Open Thread Group + * + * This library is open source and may be redistributed and/or modified under + * the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or + * (at your option) any later version. The full license is in LICENSE file + * included with this distribution, and on the openscenegraph.org website. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * OpenSceneGraph Public License for more details. +*/ + +#include + +#if defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) +# include +#endif + +namespace OpenThreads { + +#if defined(_OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES) + +// Non inline implementations for two special cases: +// * win32 +// * i386 gcc +// +// On win32 we do not want to pull windows.h in the Atomic header. +// windows.h just pulls too much stuff that disturbs sources. +// +// On i386 gcc, we have that nice builtins available but only with some compiler +// architectural compiler flags enabled. This way we can make sure that the +// compiler flags are like we had them in the configure test. If the functions +// are inline, we do not need what compiler flags the Atomic header will see. + +unsigned +Atomic::operator++() +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_add_and_fetch(&_value, 1); +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + return InterlockedIncrement(&_value); +#else +# error This implementation should happen inline in the include file +#endif +} + +unsigned +Atomic::operator--() +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + return __sync_sub_and_fetch(&_value, 1); +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + return InterlockedDecrement(&_value); +#else +# error This implementation should happen inline in the include file +#endif +} + +Atomic::operator unsigned() const +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + __sync_synchronize(); + return _value; +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + MemoryBarrier(); + return static_cast(_value); +#else +# error This implementation should happen inline in the include file +#endif +} + +bool +AtomicPtr::assign(void* ptrNew, const void* const ptrOld) +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + 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); +#else +# error This implementation should happen inline in the include file +#endif +} + +void* +AtomicPtr::get() const +{ +#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) + __sync_synchronize(); + return _ptr; +#elif defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED) + MemoryBarrier(); + return _ptr; +#else +# error This implementation should happen inline in the include file +#endif +} + +#endif // defined(_OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES) + +} diff --git a/src/OpenThreads/pthreads/CMakeLists.txt b/src/OpenThreads/pthreads/CMakeLists.txt index 92bff8310..f10bd1fd9 100644 --- a/src/OpenThreads/pthreads/CMakeLists.txt +++ b/src/OpenThreads/pthreads/CMakeLists.txt @@ -20,6 +20,7 @@ ADD_LIBRARY(${LIB_NAME} PThreadMutexPrivateData.h PThreadPrivateData.h ../common/Version.cpp + ../common/Atomic.cpp ) IF(OPENTHREADS_SONAMES) diff --git a/src/osg/Referenced.cpp b/src/osg/Referenced.cpp index c8bd6dac8..bac3b2c56 100644 --- a/src/osg/Referenced.cpp +++ b/src/osg/Referenced.cpp @@ -247,7 +247,7 @@ Referenced::~Referenced() delete tmpMutexPtr; } #else - ObserverSetData* observerSetData = _observerSetDataPtr.get(); + ObserverSetData* observerSetData = static_cast(_observerSetDataPtr.get()); if (observerSetData) { for(ObserverSet::iterator itr = observerSetData->_observers.begin(); @@ -307,12 +307,12 @@ void Referenced::unref_nodelete() const void Referenced::addObserver(Observer* observer) { #if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS) - ObserverSetData* observerSetData = _observerSetDataPtr.get(); + ObserverSetData* observerSetData = static_cast(_observerSetDataPtr.get()); while (0 == observerSetData) { ObserverSetData* newObserverSetData = new ObserverSetData; if (!_observerSetDataPtr.assign(newObserverSetData, 0)) delete newObserverSetData; - observerSetData = _observerSetDataPtr.get(); + observerSetData = static_cast(_observerSetDataPtr.get()); } OpenThreads::ScopedLock lock(observerSetData->_mutex); observerSetData->_observers.insert(observer); @@ -335,7 +335,7 @@ void Referenced::addObserver(Observer* observer) void Referenced::removeObserver(Observer* observer) { #if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS) - ObserverSetData* observerSetData = _observerSetDataPtr.get(); + ObserverSetData* observerSetData = static_cast(_observerSetDataPtr.get()); if (observerSetData) { OpenThreads::ScopedLock lock(observerSetData->_mutex);