Small updates regarding mutex synchronization.

jblueh · jblueh · commit d8059756e7a6 · 2025-10-28T13:57:06.000+01:00
Unify waitDeleteFunc, decrementDeleteFunc.
Add assertion.
Test for mutex synchronization across parallel regions.

Merge branch 'revisit/mutexSynchronization' into develop
diff --git a/include/opdi/logic/omp/mutexOmpLogic.cpp b/include/opdi/logic/omp/mutexOmpLogic.cpp
@@ -73,11 +73,6 @@ void opdi::MutexOmpLogic::waitReverseFunc(void* dataPtr) {
   #endif
 }
 
-void opdi::MutexOmpLogic::waitDeleteFunc(void* dataPtr) {
-  Data* data = static_cast<Data*>(dataPtr);
-  delete data;
-}
-
 void opdi::MutexOmpLogic::decrementReverseFunc(void* dataPtr) {
 
   Data* data = static_cast<Data*>(dataPtr);
@@ -87,8 +82,18 @@ void opdi::MutexOmpLogic::decrementReverseFunc(void* dataPtr) {
   #endif
 
   // decrement counter
-  #pragma omp atomic update
-  MutexOmpLogic::evaluationCounters[data->mutexKind][data->waitId] -= 1;
+  #ifdef NDEBUG
+    #pragma omp atomic update
+    MutexOmpLogic::evaluationCounters[data->mutexKind][data->waitId] -= 1;
+  #else
+    Counter newValue;
+    #pragma omp atomic capture
+    {
+      MutexOmpLogic::evaluationCounters[data->mutexKind][data->waitId] -= 1;
+      newValue = MutexOmpLogic::evaluationCounters[data->mutexKind][data->waitId];
+    }
+    assert(newValue == data->counter);
+  #endif
 
   #if OPDI_OMP_LOGIC_INSTRUMENT
     for (auto& instrument : ompLogicInstruments) {
@@ -97,7 +102,7 @@ void opdi::MutexOmpLogic::decrementReverseFunc(void* dataPtr) {
   #endif
 }
 
-void opdi::MutexOmpLogic::decrementDeleteFunc(void* dataPtr) {
+void opdi::MutexOmpLogic::deleteFunc(void* dataPtr) {
   Data* data = static_cast<Data*>(dataPtr);
   delete data;
 }
@@ -151,7 +156,7 @@ void opdi::MutexOmpLogic::onMutexAcquired(MutexKind mutexKind, WaitId waitId) {
       data->waitId = waitId;
 
       omp_set_lock(&recordings[mutexKind].lock);
-      data->counter = recordings[mutexKind].counters[waitId]++;
+      data->counter = recordings[mutexKind].counters[waitId]++;  // store value prior to increment
       localCounters[mutexKind][waitId] = recordings[mutexKind].counters[waitId];  // remember incremented counter value for the release event
       omp_unset_lock(&recordings[mutexKind].lock);
 
@@ -165,7 +170,7 @@ void opdi::MutexOmpLogic::onMutexAcquired(MutexKind mutexKind, WaitId waitId) {
       Handle* handle = new Handle;
       handle->data = static_cast<void*>(data);
       handle->reverseFunc = MutexOmpLogic::decrementReverseFunc;
-      handle->deleteFunc = MutexOmpLogic::decrementDeleteFunc;
+      handle->deleteFunc = MutexOmpLogic::deleteFunc;
 
       tool->pushExternalFunction(tool->getThreadLocalTape(), handle);
     }
@@ -206,7 +211,7 @@ void opdi::MutexOmpLogic::onMutexReleased(MutexKind mutexKind, WaitId waitId) {
       Handle* handle = new Handle;
       handle->data = static_cast<void*>(data);
       handle->reverseFunc = MutexOmpLogic::waitReverseFunc;
-      handle->deleteFunc = MutexOmpLogic::waitDeleteFunc;
+      handle->deleteFunc = MutexOmpLogic::deleteFunc;
 
       tool->pushExternalFunction(tool->getThreadLocalTape(), handle);
     }
diff --git a/include/opdi/logic/omp/mutexOmpLogic.hpp b/include/opdi/logic/omp/mutexOmpLogic.hpp
@@ -89,10 +89,8 @@ namespace opdi {
       void checkKind(MutexKind mutexKind);
 
       static void waitReverseFunc(void* dataPtr);
-      static void waitDeleteFunc(void* dataPtr);
-
       static void decrementReverseFunc(void* dataPtr);
-      static void decrementDeleteFunc(void* dataPtr);
+      static void deleteFunc(void* dataPtr);
 
     protected:
 
diff --git a/tests/results/FirstOrderForwardLock2.ref b/tests/results/FirstOrderForwardLock2.ref
@@ -0,0 +1,18 @@
+Point 0 :
+-79.9235
+-225.656
+423.421
+-257.64
+88.8672
+Point 1 :
+-29.7063
+-877.013
+-1758.64
+-1586.99
+-263.202
+Point 2 :
+28.3306
+-302.06
+100.675
+299.426
+683.226
diff --git a/tests/results/FirstOrderReverseLock2.ref b/tests/results/FirstOrderReverseLock2.ref
@@ -0,0 +1,18 @@
+Point 0 :
+-79.9235
+-225.656
+423.421
+-257.64
+88.8672
+Point 1 :
+-29.7063
+-877.013
+-1758.64
+-1586.99
+-263.202
+Point 2 :
+28.3306
+-302.06
+100.675
+299.426
+683.226
diff --git a/tests/results/FirstOrderReverseNestedParallelLock2.ref b/tests/results/FirstOrderReverseNestedParallelLock2.ref
@@ -0,0 +1,18 @@
+Point 0 :
+-79.9235
+-225.656
+423.421
+-257.64
+88.8672
+Point 1 :
+-29.7063
+-877.013
+-1758.64
+-1586.99
+-263.202
+Point 2 :
+28.3306
+-302.06
+100.675
+299.426
+683.226
diff --git a/tests/results/FirstOrderReverseNoOpenMPLock2.ref b/tests/results/FirstOrderReverseNoOpenMPLock2.ref
@@ -0,0 +1,18 @@
+Point 0 :
+-79.9235
+-225.656
+423.421
+-257.64
+88.8672
+Point 1 :
+-29.7063
+-877.013
+-1758.64
+-1586.99
+-263.202
+Point 2 :
+28.3306
+-302.06
+100.675
+299.426
+683.226
diff --git a/tests/results/FirstOrderReverseNoParallelLock2.ref b/tests/results/FirstOrderReverseNoParallelLock2.ref
@@ -0,0 +1,18 @@
+Point 0 :
+-79.9235
+-225.656
+423.421
+-257.64
+88.8672
+Point 1 :
+-29.7063
+-877.013
+-1758.64
+-1586.99
+-263.202
+Point 2 :
+28.3306
+-302.06
+100.675
+299.426
+683.226
diff --git a/tests/results/FirstOrderReversePassiveLock2.ref b/tests/results/FirstOrderReversePassiveLock2.ref
@@ -0,0 +1,18 @@
+Point 0 :
+-79.9235
+0
+0
+0
+0
+Point 1 :
+-29.7063
+0
+0
+0
+0
+Point 2 :
+28.3306
+0
+0
+0
+0
diff --git a/tests/results/FirstOrderReverseSingleThreadLock2.ref b/tests/results/FirstOrderReverseSingleThreadLock2.ref
@@ -0,0 +1,18 @@
+Point 0 :
+-79.9235
+-225.656
+423.421
+-257.64
+88.8672
+Point 1 :
+-29.7063
+-877.013
+-1758.64
+-1586.99
+-263.202
+Point 2 :
+28.3306
+-302.06
+100.675
+299.426
+683.226
diff --git a/tests/results/PrimalLock2.ref b/tests/results/PrimalLock2.ref
@@ -0,0 +1,6 @@
+Point 0 :
+-79.9235
+Point 1 :
+-29.7063
+Point 2 :
+28.3306
diff --git a/tests/results/SecondOrderReverseForwardLock2.ref b/tests/results/SecondOrderReverseForwardLock2.ref
@@ -0,0 +1,66 @@
+Point 0 :
+-79.9235
+-225.656
+423.421
+-257.64
+88.8672
+-1.00642e+06
+-56792.5
+6832.03
+-137972
+-56792.5
+135177
+-175415
+-1273.13
+6832.03
+-175415
+1.13859e+06
+221700
+-137972
+-1273.13
+221700
+38738.6
+Point 1 :
+-29.7063
+-877.013
+-1758.64
+-1586.99
+-263.202
+591567
+398157
+1045.24
+33867.3
+398157
+333866
+77951.1
+2956.49
+1045.24
+77951.1
+60099.2
+-130885
+33867.3
+2956.49
+-130885
+-364572
+Point 2 :
+28.3306
+-302.06
+100.675
+299.426
+683.226
+-971640
+-777217
+104.041
+-15939.8
+-777217
+-1.80055e+06
+-870470
+-2509.74
+104.041
+-870470
+-642711
+10825.4
+-15939.8
+-2509.74
+10825.4
+27787.9
diff --git a/tests/tests/TestLock2.hpp b/tests/tests/TestLock2.hpp
@@ -0,0 +1,97 @@
+﻿/*
+ * OpDiLib, an Open Multiprocessing Differentiation Library
+ *
+ * Copyright (C) 2020-2022 Chair for Scientific Computing (SciComp), TU Kaiserslautern
+ * Copyright (C) 2023-2025 Chair for Scientific Computing (SciComp), University of Kaiserslautern-Landau
+ * Homepage: https://scicomp.rptu.de
+ * Contact:  Prof. Nicolas R. Gauger (opdi@scicomp.uni-kl.de)
+ *
+ * Lead developer: Johannes Blühdorn (SciComp, University of Kaiserslautern-Landau)
+ *
+ * This file is part of OpDiLib (https://scicomp.rptu.de/software/opdi).
+ *
+ * OpDiLib is free software: you can redistribute it and/or modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later
+ * version.
+ *
+ * OpDiLib 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 GNU Lesser General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License along with OpDiLib. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#pragma once
+
+#include "testBase.hpp"
+
+template<typename _Case>
+struct TestLock2 : public TestBase<4, 1, 3, TestLock2<_Case>> {
+  public:
+    using Case = _Case;
+    using Base = TestBase<4, 1, 3, TestLock2<Case>>;
+
+    template<typename T>
+    static void test(std::array<T, Base::nIn> const& in, std::array<T, Base::nOut>& out) {
+
+      int const N = 100;
+      T* jobResults = new T[N];
+      omp_lock_t lock1, lock2;
+      INIT_LOCK(&lock1);
+      INIT_LOCK(&lock2);
+
+      SET_LOCK(&lock1);
+
+      OPDI_PARALLEL()
+      {
+        int nThreads = omp_get_num_threads();
+        int start = ((N - 1) / nThreads + 1) * omp_get_thread_num();
+        int end = std::min(N, ((N - 1) / nThreads + 1) * (omp_get_thread_num() + 1));
+
+        if (omp_get_thread_num() != 0) {
+          SET_LOCK(&lock1);
+        }
+
+        for (int i = start; i < end; ++i) {
+          Base::job1(i, in, jobResults[i]);
+          out[0] += jobResults[i];
+        }
+        UNSET_LOCK(&lock1);
+
+        OPDI_BARRIER()
+
+        if (omp_get_thread_num() == 0) {
+          SET_LOCK(&lock1);
+          SET_LOCK(&lock2);
+        }
+      }
+      OPDI_END_PARALLEL
+
+      UNSET_LOCK(&lock2);
+
+      OPDI_PARALLEL()
+      {
+        int nThreads = omp_get_num_threads();
+        int start = ((N - 1) / nThreads + 1) * omp_get_thread_num();
+        int end = std::min(N, ((N - 1) / nThreads + 1) * (omp_get_thread_num() + 1));
+
+        if (omp_get_thread_num() != 0) {
+          SET_LOCK(&lock1);
+        }
+
+        for (int i = start; i < end; ++i) {
+          Base::job2(i, in, jobResults[i]);
+          out[0] += jobResults[i];
+        }
+        UNSET_LOCK(&lock1);
+      }
+      OPDI_END_PARALLEL
+
+      DESTROY_LOCK(&lock1);
+      DESTROY_LOCK(&lock2);
+
+      delete [] jobResults;
+    }
+};
