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

@@ -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 <cstdlib>
+#include <stdlib.h>
 
 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;

include/OpenThreads/Atomic

@@ -15,9 +15,12 @@
 #define _OPENTHREADS_ATOMIC_
 
 #include <OpenThreads/Config>
+#include <OpenThreads/Exports>
 
-#if defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED)
+#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS) && defined(__i386__)
-# include <windows.h>
+#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 <atomic.h>
 #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++()
+    _OPENTHREADS_ATOMIC_INLINE unsigned operator++();
-    {
+    _OPENTHREADS_ATOMIC_INLINE unsigned operator--();
-#if defined(_OPENTHREADS_ATOMIC_USE_GCC_BUILTINS)
+    _OPENTHREADS_ATOMIC_INLINE operator unsigned() const;
-        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<Mutex> 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<Mutex> 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<unsigned const volatile &>(_value);
-#elif defined(_OPENTHREADS_ATOMIC_USE_MUTEX)
-        ScopedLock<Mutex> lock(_mutex);
-        return _value;
-#else
-        return _value;
-#endif
-    }
-    
  private:
 
     Atomic(const Atomic&);
@@ -111,59 +68,16 @@ class Atomic {
  *  @class AtomicPtr
  *  @brief  This class provides an atomic pointer assignment using cas operations.
  */
-template<typename T>
+class OPENTHREAD_EXPORT_DIRECTIVE AtomicPtr {
-class 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)
+    _OPENTHREADS_ATOMIC_INLINE bool assign(void* ptrNew, const void* const ptrOld);
-    {
+    _OPENTHREADS_ATOMIC_INLINE void* get() const;
-#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<Mutex> 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<Mutex> lock(_mutex);
-        return _ptr;
-#else
-        return _ptr;
-#endif
-    }
 
 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<Mutex> 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<Mutex> 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<Mutex> 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<Mutex> 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<Mutex> lock(_mutex);
+    return _ptr;
+#else
+    return _ptr;
+#endif
+}
+
+#endif // !defined(_OPENTHREADS_ATOMIC_USE_LIBRARY_ROUTINES)
+
 }
 
 #endif // _OPENTHREADS_ATOMIC_

include/osg/Referenced

@@ -129,7 +129,7 @@ class OSG_EXPORT Referenced
 #if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
         struct ObserverSetData;
 
-        OpenThreads::AtomicPtr<ObserverSetData> _observerSetDataPtr;
+        OpenThreads::AtomicPtr _observerSetDataPtr;
 
         mutable OpenThreads::Atomic     _refCount;
 #else

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 <OpenThreads/Atomic>
+
+#if defined(_OPENTHREADS_ATOMIC_USE_WIN32_INTERLOCKED)
+# include <windows.h>
+#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<unsigned const volatile &>(_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)
+
+}

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)

src/osg/Referenced.cpp

@@ -247,7 +247,7 @@ Referenced::~Referenced()
         delete tmpMutexPtr;
     }
 #else
-    ObserverSetData* observerSetData = _observerSetDataPtr.get();
+    ObserverSetData* observerSetData = static_cast<ObserverSetData*>(_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<ObserverSetData*>(_observerSetDataPtr.get());
     while (0 == observerSetData) {
         ObserverSetData* newObserverSetData = new ObserverSetData;
         if (!_observerSetDataPtr.assign(newObserverSetData, 0))
             delete newObserverSetData;
-        observerSetData = _observerSetDataPtr.get();
+        observerSetData = static_cast<ObserverSetData*>(_observerSetDataPtr.get());
     }
     OpenThreads::ScopedLock<OpenThreads::Mutex> 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<ObserverSetData*>(_observerSetDataPtr.get());
     if (observerSetData)
     {
        OpenThreads::ScopedLock<OpenThreads::Mutex> lock(observerSetData->_mutex);