0022980: Fixed Standard_Atomic.hxx

Standard_Atomic, prefer gcc built-ins rather than WinAPI calls (revert previous change of order)
Raise compiler error if no implementation found for atomic operations.
Add new draw command for testing this bug
Fix for compilation with TBB disabled
Adding CSF_TBB_INCLUDES macro in edl files
Correction of *.edl files
Update of EXTERNLIB files with CSF_TBB
Adding EXTERNLIB file

src/Draw/Draw_CMPLRS.edl

@@ -20,5 +20,5 @@
 @uses "CSF.edl";
 
 ---@string %CMPLRS_CXX_Options += " -I"%CSF_TCL_INCLUDE" -DUSE_TK";
-@string %CMPLRS_CXX_Options += " -I"%CSF_TCL_INCLUDE;
+@string %CMPLRS_CXX_Options += " -I"%CSF_TCL_INCLUDE" -I"%CSF_TBB_INCLUDES ;
 

src/Draw/EXTERNLIB

@@ -3,4 +3,4 @@ CSF_TclTkLibs
 CSF_gdi32
 CSF_advapi32
 CSF_user32
-
+CSF_TBB

src/QABugs/EXTERNLIB

@@ -0,0 +1 @@
+CSF_TBB

src/QABugs/FILES

@@ -18,3 +18,4 @@ QABugs_17.cxx
 QABugs_18.cxx
 QABugs_19.cxx
 QABugs_CMPLRS.edl
+EXTERNLIB

src/QABugs/QABugs_10.cxx

@@ -1169,7 +1169,7 @@ static Standard_Integer OCC828 (Draw_Interpretor& di,Standard_Integer argc, cons
 }
 
 void QABugs::Commands_10(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("OCC426", "OCC426 shape1 shape2 shape3 shape4 shape5 shape6 [BRepAlgoAPI/BRepAlgo = 1/0]", __FILE__, OCC426, group);
 

src/QABugs/QABugs_12.cxx

@@ -126,7 +126,7 @@ static Standard_Integer OCC895 (Draw_Interpretor& di, Standard_Integer argc, con
 }
 
 void QABugs::Commands_12(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("OCC895", "OCC895 result [angle [reverse [order]]]", __FILE__, OCC895, group);
 

src/QABugs/QABugs_13.cxx

@@ -1061,7 +1061,7 @@ static Standard_Integer OCC817 (Draw_Interpretor& di, Standard_Integer argc, con
 }
 
 void QABugs::Commands_13(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("OCC332", "OCC332 [wall_thickness [dia1 [dia2 [length [major_radius]]]]]", __FILE__, OCC332bug, group);
   //////theCommands.Add("OCC544", "OCC544 [[[[[wT [[[[d1 [[[d2 [[R [length]]]]]", __FILE__, OCC544, group);

src/QABugs/QABugs_15.cxx

@@ -69,6 +69,6 @@ static Standard_Integer BUC60720 (Draw_Interpretor& di,Standard_Integer argc,con
 
 void QABugs::Commands_15(Draw_Interpretor& theCommands)
 {
-  char *group = "QABugs";
+  const char *group = "QABugs";
   theCommands.Add("BUC60720","BUC60720 0/1",__FILE__,BUC60720,group);
 }

src/QABugs/QABugs_16.cxx

@@ -853,7 +853,7 @@ static Standard_Integer OCC904 (Draw_Interpretor& di, Standard_Integer argc, con
 }
 
 void QABugs::Commands_16(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("BUC60848", "BUC60848 shape", __FILE__, BUC60848, group);
   theCommands.Add ("BUC60828", "BUC60828", __FILE__, BUC60828, group);

src/QABugs/QABugs_17.cxx

@@ -2252,7 +2252,7 @@ Standard_Integer OCC17480(Draw_Interpretor& di,   Standard_Integer n,   const ch
 }
 
 void QABugs::Commands_17(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("BUC60842", "BUC60842", __FILE__, BUC60842, group);
   theCommands.Add ("BUC60843", "BUC60843 result_shape name_of_circle name_of_curve [ par1 [ tol ] ]", __FILE__, BUC60843, group);

src/QABugs/QABugs_18.cxx

@@ -429,7 +429,7 @@ static Standard_Integer OCC404bug (Draw_Interpretor& di, Standard_Integer argc,
 }
 
 void QABugs::Commands_18(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add("BUC60851", "BUC60851", __FILE__, BUC60851, group);
   theCommands.Add("OCC216", "OCC216", __FILE__, OCC216, group);

src/QABugs/QABugs_19.cxx

@@ -47,7 +47,6 @@
 #include <BRepPrimAPI_MakeSphere.hxx>
 #include <BRepAlgo_Cut.hxx>
 
-//static Standard_Integer OCC230 (Draw_Interpretor& /*di*/, Standard_Integer /*argc*/, const char ** /*argv*/)
 static Standard_Integer OCC230 (Draw_Interpretor& di, Standard_Integer argc, const char ** argv)
 {
   if ( argc != 4) {
@@ -148,13 +147,82 @@ static Standard_Integer OCC23237 (Draw_Interpretor& di, Standard_Integer /*argc*
   return 0;
 }
 
+#ifdef HAVE_TBB
+
+#include <Standard_Atomic.hxx>
+#include <tbb/blocked_range.h>
+#include <tbb/parallel_for.h>
+
+class IncrementerDecrementer
+{
+public:
+    IncrementerDecrementer (Standard_Integer* theVal, Standard_Boolean thePositive) : myVal (theVal), myPositive (thePositive)
+    {}
+    void operator() (const tbb::blocked_range<size_t>& r) const
+    {
+        if (myPositive)
+            for (size_t i = r.begin(); i != r.end(); ++i)
+                Standard_Atomic_Increment (myVal);
+        else
+            for (size_t i = r.begin(); i != r.end(); ++i)
+                Standard_Atomic_Decrement (myVal);
+    }
+private:
+    Standard_Integer*   myVal;
+    Standard_Boolean   myPositive;
+};
+#endif
+
+#define QCOMPARE(val1, val2) \
+  di << "Checking " #val1 " == " #val2 << \
+        ((val1) == (val2) ? ": OK\n" : ": Error\n")
+
+#ifdef HAVE_TBB
+static Standard_Integer OCC22980 (Draw_Interpretor& di, Standard_Integer /*argc*/, const char ** /*argv*/)
+{
+  int aSum = 0;
+
+  //check returned value
+  QCOMPARE (Standard_Atomic_Decrement (&aSum), -1);
+  QCOMPARE (Standard_Atomic_Increment (&aSum), 0);
+  QCOMPARE (Standard_Atomic_Increment (&aSum), 1);
+  QCOMPARE (Standard_Atomic_Increment (&aSum), 2);
+//  QCOMPARE (Standard_Atomic_DecrementTest (&aSum), 0);
+//  QCOMPARE (Standard_Atomic_DecrementTest (&aSum), 1);
+
+  //check atomicity 
+  aSum = 0;
+  const int N = 1 << 24; //big enough to ensure concurrency
+
+  //increment
+  tbb::parallel_for (tbb::blocked_range<size_t> (0, N), IncrementerDecrementer (&aSum, true));
+  QCOMPARE (aSum, N);
+
+  //decrement
+  tbb::parallel_for (tbb::blocked_range<size_t> (0, N), IncrementerDecrementer (&aSum, false));
+  QCOMPARE (aSum, 0);
+
+  return 0;
+}
+
+#else /* HAVE_TBB */
+
+static Standard_Integer OCC22980 (Draw_Interpretor& di, Standard_Integer /*argc*/, const char **argv)
+{
+  di << "Test skipped: command " << argv[0] << " requires TBB library\n";
+  return 0;
+}
+
+#endif /* HAVE_TBB */
+
 void QABugs::Commands_19(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("OCC230", "OCC230 TrimmedCurve Pnt2d Pnt2d", __FILE__, OCC230, group);
   theCommands.Add ("OCC142", "OCC142", __FILE__, OCC142, group);
   theCommands.Add ("OCC23361", "OCC23361", __FILE__, OCC23361, group);
-  theCommands.Add("OCC23237", "OCC23237", __FILE__, OCC23237, group); 
+  theCommands.Add ("OCC23237", "OCC23237", __FILE__, OCC23237, group); 
+  theCommands.Add ("OCC22980", "OCC22980", __FILE__, OCC22980, group);
 
   return;
 }

src/QABugs/QABugs_2.cxx

@@ -200,7 +200,7 @@ static Standard_Integer OCC1048 (Draw_Interpretor& di, Standard_Integer argc, co
 }
 
 void QABugs::Commands_2(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   //theCommands.Add("OCC527", "OCC527 shape", __FILE__, OCC527, group);  
   theCommands.Add("OCC527", "OCC527 shape [BRepAlgoAPI/BRepAlgo = 1/0]", __FILE__, OCC527, group);  

src/QABugs/QABugs_4.cxx

@@ -145,7 +145,7 @@ static int BUC60627(Draw_Interpretor& di, Standard_Integer argc, const char ** a
 }
 
 void QABugs::Commands_4(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add("BUC60738","BUC60738",__FILE__,BUC60738,group);
   theCommands.Add("BUC60606","BUC60606 name",__FILE__,BUC60606,group);

src/QABugs/QABugs_5.cxx

@@ -118,7 +118,7 @@ static Standard_Integer OCC5696 (Draw_Interpretor& di, Standard_Integer argc, co
 }
 
 void QABugs::Commands_5(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("OCC6001", "OCC6001 name curve/wire surface\n\t\tintersect curve by surface", 
                    __FILE__, OCC6001, group);

src/QABugs/QABugs_6.cxx

@@ -122,7 +122,7 @@ static Standard_Integer OCC281bug (Draw_Interpretor& di, Standard_Integer argc,
 }
 
 void QABugs::Commands_6(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("OCC281", "OCC281 x y TypeOfMarker(0-12)", __FILE__, OCC281bug, group);
 

src/QABugs/QABugs_7.cxx

@@ -323,7 +323,7 @@ static Standard_Integer OCC862 (Draw_Interpretor& di, Standard_Integer argc, con
 }
 
 void QABugs::Commands_7(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add("OCC862", "OCC862 curve1 curve2", __FILE__, OCC862, group);  
   return;

src/QABugs/QABugs_8.cxx

@@ -259,7 +259,7 @@ static Standard_Integer OCC1651 (Draw_Interpretor& di, Standard_Integer argc, co
 }
 
 void QABugs::Commands_8(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add("BUC60753", "BUC60753 mode ratio", __FILE__, BUC60753, group);
   theCommands.Add("OCC162", "OCC162 name", __FILE__, OCC162, group);

src/QABugs/QABugs_9.cxx

@@ -150,7 +150,7 @@ static Standard_Integer OCC137_z (Draw_Interpretor& di, Standard_Integer argc, c
 }
 
 void QABugs::Commands_9(Draw_Interpretor& theCommands) {
-  char *group = "QABugs";
+  const char *group = "QABugs";
 
   theCommands.Add ("BUC60857", "BUC60857", __FILE__, BUC60857, group);
   theCommands.Add("OCC137","OCC137 mode [shape]",__FILE__,OCC137,group);

src/QABugs/QABugs_CMPLRS.edl

@@ -23,7 +23,7 @@
 
 @uses "CSF.edl";
 @ifdefined( %CSF_TCL_INCLUDE ) then
-@string %CMPLRS_CXX_Options = " -I"%CSF_TCL_INCLUDE %CMPLRS_CXX_Options ;
+@string %CMPLRS_CXX_Options = " -I"%CSF_TCL_INCLUDE" -I"%CSF_TBB_INCLUDES %CMPLRS_CXX_Options ;
 @endif;
 
 @endif;

src/QANCollection/QANCollection1.cxx

@@ -141,7 +141,7 @@ static Standard_Integer QANColCheckSequence(Draw_Interpretor& di, Standard_Integ
 }
 
 void QANCollection::Commands1(Draw_Interpretor& theCommands) {
-  char *group = "QANCollection";
+  const char *group = "QANCollection";
 
   // from agvCollTest/src/AgvColEXE/TestEXE.cxx
   theCommands.Add("QANColCheckArray1",   "QANColCheckArray1 [-n]",               __FILE__, QANColCheckArray1,   group);  

src/QANCollection/QANCollection2.cxx

@@ -288,7 +288,7 @@ static Standard_Integer QANColTestSequence(Draw_Interpretor& di, Standard_Intege
 }
 
 void QANCollection::Commands2(Draw_Interpretor& theCommands) {
-  char *group = "QANCollection";
+  const char *group = "QANCollection";
 
   // from agvCollTest/src/CollectionEXE/FuncTestEXE.cxx
   theCommands.Add("QANColTestArray1",         "QANColTestArray1",         __FILE__, QANColTestArray1,         group);  

src/QANCollection/QANCollection3.cxx

@@ -256,7 +256,7 @@ static Standard_Integer QANColCheckSparseArray(Draw_Interpretor& di, Standard_In
 }
 
 void QANCollection::Commands3(Draw_Interpretor& theCommands) {
-  char *group = "QANCollection";
+  const char *group = "QANCollection";
 
   // from agvCollTest/src/CollectionEXE/PerfTestEXE.cxx
   theCommands.Add("QANColPerfArray1",         "QANColPerfArray1 Repeat Size",         __FILE__, QANColPerfArray1,         group);  

src/Standard/Standard_Atomic.hxx

@@ -17,7 +17,6 @@
 // purpose or non-infringement. Please see the License for the specific terms
 // and conditions governing the rights and limitations under the License.
 
-
 //! @file
 //! Implementation of some atomic operations (elementary operations
 //! with data that cannot be interrupted by parallel threads in the
@@ -32,14 +31,39 @@
 #ifndef _Standard_Atomic_HeaderFile
 #define _Standard_Atomic_HeaderFile
 
-#include <Standard_Macro.hxx>
+//! Increments atomically integer variable pointed by theValue
+//! and returns resulting incremented value.
+inline int Standard_Atomic_Increment (volatile int* theValue);
 
-#if (defined(_WIN32) || defined(__WIN32__))
+//! Decrements atomically integer variable pointed by theValue
-extern "C" {
+//! and returns resulting decremented value.
-  long _InterlockedIncrement(long volatile* lpAddend);
+inline int Standard_Atomic_Decrement (volatile int* theValue);
-  long _InterlockedDecrement(long volatile* lpAddend);
+
+// Platform-dependent implementation
+#if defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4)
+// gcc explicitly defines the macros __GCC_HAVE_SYNC_COMPARE_AND_SWAP_*
+// starting with version 4.4+, although built-in functions
+// are available since 4.1.x. However unless __GCC_HAVE_SYNC_COMPARE_AND_SWAP_*
+// are defined, linking may fail without specifying -march option when
+// building for 32bit architecture on 64bit (using -m32 option). To avoid
+// making -march mandatory, check for __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* is
+// enforced.
+
+int Standard_Atomic_Increment (volatile int* theValue)
+{
+  return __sync_add_and_fetch (theValue, 1);
+}
+
+int Standard_Atomic_Decrement (volatile int* theValue)
+{
+  return __sync_sub_and_fetch (theValue, 1);
+}
+
+#elif defined(_WIN32) || defined(__WIN32__)
+extern "C" {
+  long _InterlockedIncrement (volatile long* lpAddend);
+  long _InterlockedDecrement (volatile long* lpAddend);
 }
-#endif
 
 #if defined(_MSC_VER)
   // force intrinsic instead of WinAPI calls
@@ -47,74 +71,79 @@ extern "C" {
   #pragma intrinsic (_InterlockedDecrement)
 #endif
 
-//! Increments atomically integer variable pointed by theValue
+// WinAPI function or MSVC intrinsic
-//! and returns resulting incremented value.
+// Note that we safely cast int* to long*, as they have same size and endian-ness
-static int Standard_Atomic_Increment (volatile int* theValue)
+
+int Standard_Atomic_Increment (volatile int* theValue)
 {
-#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+  return _InterlockedIncrement (reinterpret_cast<volatile long*>(theValue));
-  // mordern g++ compiler (gcc4.4+)
-  // built-in functions available for appropriate CPUs (at least -march=i486 should be specified on x86 platform)
-  return __sync_add_and_fetch (theValue, 1);
-#elif (defined(_WIN32) || defined(__WIN32__))
-  // WinAPI function or MSVC intrinsic
-  return _InterlockedIncrement(reinterpret_cast<long volatile*>(theValue));
-#elif defined(LIN)
-  // use x86 / x86_64 inline assembly (compatibility with alien compilers / old GCC)
-  int anIncResult;
-  __asm__ __volatile__ (
-  #if defined(_OCC64)
-    "lock xaddl %%ebx, (%%rax) \n\t"
-    "incl %%ebx                \n\t"
-    : "=b" (anIncResult)
-    : "a" (theValue), "b" (1)
-    : "cc", "memory");
-  #else
-    "lock xaddl %%eax, (%%ecx) \n\t"
-    "incl %%eax                \n\t"
-    : "=a" (anIncResult)
-    : "c" (theValue), "a" (1)
-    : "memory");
-  #endif
-  return anIncResult;
-#else
-  //#error "Atomic operation doesn't implemented for current platform!"
-  return ++(*theValue);
-#endif
 }
 
-//! Decrements atomically integer variable pointed by theValue
+int Standard_Atomic_Decrement (volatile int* theValue)
-//! and returns resulting decremented value.
-static int Standard_Atomic_Decrement (volatile int* theValue)
 {
-#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4
+  return _InterlockedDecrement (reinterpret_cast<volatile long*>(theValue));
-  // mordern g++ compiler (gcc4.4+)
-  // built-in functions available for appropriate CPUs (at least -march=i486 should be specified on x86 platform)
-  return __sync_sub_and_fetch (theValue, 1);
-#elif (defined(_WIN32) || defined(__WIN32__))
-  // WinAPI function or MSVC intrinsic
-  return _InterlockedDecrement(reinterpret_cast<long volatile*>(theValue));
-#elif defined(LIN)
-  // use x86 / x86_64 inline assembly (compatibility with alien compilers / old GCC)
-  int aDecResult;
-  __asm__ __volatile__ (
-  #if defined(_OCC64)
-    "lock xaddl %%ebx, (%%rax) \n\t"
-    "decl %%ebx                \n\t"
-    : "=b" (aDecResult)
-    : "a" (theValue), "b" (-1)
-    : "cc", "memory");
-  #else
-    "lock xaddl %%eax, (%%ecx) \n\t"
-    "decl %%eax                \n\t"
-    : "=a" (aDecResult)
-    : "c" (theValue), "a" (-1)
-    : "memory");
-  #endif
-  return aDecResult;
-#else
-  //#error "Atomic operation doesn't implemented for current platform!"
-  return --(*theValue);
-#endif
 }
 
+#elif defined(__APPLE__)
+// use atomic operations provided by MacOS
+
+#include <libkern/OSAtomic.h>
+
+int Standard_Atomic_Increment (volatile int* theValue)
+{
+  return OSAtomicIncrement32Barrier (theValue);
+}
+
+int Standard_Atomic_Decrement (volatile int* theValue)
+{
+  return OSAtomicDecrement32Barrier (theValue);
+}
+
+#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64))
+// use x86 / x86_64 inline assembly (compatibility with alien compilers / old GCC)
+
+inline int Standard_Atomic_Add (volatile int* theValue, int theVal)
+{
+  // C equivalent:
+  // *theValue += theVal;
+  // return *theValue;
+
+  int previous;
+  __asm__ __volatile__
+  (
+    "lock xadd %0,%1"
+  : "=q"(previous), "=m"(*theValue) //output
+  : "0"(theVal), "m"(*theValue) //input
+  : "memory" //clobbers
+  );
+  return previous + theVal;
+}
+
+int Standard_Atomic_Increment (volatile int* theValue)
+{
+  return Standard_Atomic_Add (theValue, 1);
+}
+
+int Standard_Atomic_Decrement (volatile int* theValue)
+{
+  return Standard_Atomic_Add (theValue, -1);
+}
+
+#else
+
+#ifndef IGNORE_NO_ATOMICS
+  #error "Atomic operation isn't implemented for current platform!"
+#endif
+int Standard_Atomic_Increment (volatile int* theValue)
+{
+  return ++(*theValue);
+}
+
+int Standard_Atomic_Decrement (volatile int* theValue)
+{
+  return --(*theValue);
+}
+
+#endif
+
 #endif //_Standard_Atomic_HeaderFile

src/Standard/Standard_CMPLRS.edl

@@ -21,7 +21,7 @@
 @set %Standard_CMPLRS_EDL="";
 
 @if ( %Station != "sun" ) then
-  @string %CMPLRS_CXX_INCLUDE = %CMPLRS_CXX_INCLUDE " " %CSF_TBB_INCLUDES ;
+  @string %CMPLRS_CXX_INCLUDE = " -I"%CMPLRS_CXX_INCLUDE" -I"%CSF_TBB_INCLUDES ;
 @endif;
 
 @endif;

src/TKDraw/EXTERNLIB

@@ -14,3 +14,4 @@ CSF_TclTkLibs
 CSF_gdi32
 CSF_advapi32
 CSF_user32
+CSF_TBB

src/TKQADraw/EXTERNLIB

@@ -39,3 +39,4 @@ CSF_TclTkLibs
 CSF_gdi32
 CSF_advapi32
 CSF_user32
+CSF_TBB

tests/bugs/fclasses/bug22980

@@ -0,0 +1,11 @@
+puts "============"
+puts "OCC22980"
+puts "============"
+puts ""
+#######################################################################
+# Fixed Standard_Atomic.hxx
+#######################################################################
+
+pload QAcommands
+
+OCC22980