[Libreoffice-commits] core.git: include/opencl opencl/source

[Stephan Bergmann (via logerrit)]

 include/opencl/OpenCLZone.hxx |   18 +++++++++++-------
 opencl/source/OpenCLZone.cxx  |    3 +--
 2 files changed, 12 insertions(+), 9 deletions(-)

New commits:
commit 33d922ea07b6b20276471b19fd9c148ed90d7442
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Wed Sep 18 19:13:34 2019 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Wed Sep 18 20:28:29 2019 +0200

    -Werror=volatile in OpenCLZone
    
    Recent GCC 10 trunk in C++20 mode reports issues like
    
    > include/opencl/OpenCLZone.hxx:27:9: error: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Werror=volatile]
    >    27 |         gnEnterCount++;
    >       |         ^~~~~~~~~~~~
    
    But unlike in the case of ec17c8ec5256386b0197a8ffe5d7cad3e7d70f8f
    "-Werror=volatile in OpenGLZone",
    
    * it looks like there are no multi-threading issues here, and the counters are
      just accessed (via OpenCLZone::isInZone) from the VCLExceptionSignal_impl
      signal handler in addition to being modified (via OpenCLZone RAII objects)
      from mainline code; and
    
    * from the usage pattern of gnEnterCount and gnLeaveCount it appears that they
      can be combined into a single counter.
      (f41eb66302208f384a475fb20c98b6d1b0676cb6 "opencl: OpenCLZone, detect CL
      device change and disable CL on crash" presumably modelled OpenCLZone naively
      after OpenGLZone, without simplifying it where possible.)  One minor advantage
      of having two monotonically increasing counters is that when they overflow,
      the implementation of isInZone (comparing them for equality) still gives
      ~useful results (assuming that a false "match" of non-overflown gnEnterCount
      against overflown gnLeaveCount is highly unlikely).  But instances of
      OpenCLZone RAII objects are presumably never nested very deeply (if at all),
      so that the newly added "TODO: overflow" comment (which would even cause UB if
      std::sig_atomic_t is signed) is probably of no practical concern.
    
    Change-Id: I92e1f2c46ca996a0a86bacabcda2accba5eb6298
    Reviewed-on: https://gerrit.libreoffice.org/79106
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/include/opencl/OpenCLZone.hxx b/include/opencl/OpenCLZone.hxx
index 0d2059dddc87..d1c59dcd25d4 100644
--- a/include/opencl/OpenCLZone.hxx
+++ b/include/opencl/OpenCLZone.hxx
@@ -10,33 +10,37 @@
 #ifndef INCLUDED_OPENCL_INC_OPENCL_ZONE_HXX
 #define INCLUDED_OPENCL_INC_OPENCL_ZONE_HXX
 
+#include <sal/config.h>
+
+#include <cassert>
+#include <csignal>
+
 #include <opencl/opencldllapi.h>
 
 // FIXME: post back-port, templatize me and share with OpenGLZone.
 class OPENCL_DLLPUBLIC OpenCLZone
 {
-    /// how many times have we entered a CL zone
-    static volatile sal_uInt64 gnEnterCount;
-    /// how many times have we left a new CL zone
-    static volatile sal_uInt64 gnLeaveCount;
+    /// how many times have we entered a CL zone and not yet left it
+    static volatile std::sig_atomic_t gnEnterCount;
     static volatile bool gbInInitialTest;
 
 public:
     OpenCLZone()
     {
-        gnEnterCount++;
+        gnEnterCount = gnEnterCount + 1; //TODO: overflow
     }
 
     ~OpenCLZone()
     {
-        gnLeaveCount++;
+        assert(gnEnterCount > 0);
+        gnEnterCount = gnEnterCount - 1;
         if (!isInZone())
             gbInInitialTest = false;
     }
 
     static bool isInZone()
     {
-        return gnEnterCount != gnLeaveCount;
+        return gnEnterCount > 0;
     }
 
     static bool isInInitialTest()
diff --git a/opencl/source/OpenCLZone.cxx b/opencl/source/OpenCLZone.cxx
index e4e3ff08fff4..f7c8e961364d 100644
--- a/opencl/source/OpenCLZone.cxx
+++ b/opencl/source/OpenCLZone.cxx
@@ -19,8 +19,7 @@
 
 // FIXME: templatize me vs. OpenGLZone.
 
-sal_uInt64 volatile OpenCLZone::gnEnterCount = 0;
-sal_uInt64 volatile OpenCLZone::gnLeaveCount = 0;
+std::sig_atomic_t volatile OpenCLZone::gnEnterCount = 0;
 bool volatile OpenCLZone::gbInInitialTest = false;
 
 /**