Simplify GPU force reduction conditionals

[alexxy/gromacs.git] / src / gromacs / mdlib / gpuforcereduction_impl.cpp

diff --git a/src/gromacs/mdlib/gpuforcereduction_impl.cpp b/src/gromacs/mdlib/gpuforcereduction_impl.cpp
index fb9ab131bd91f18645c6e506e0fb5edcb338c68e..2b4a4a1784400a98a55e5e816860dc13b228bea6 100644 (file)
--- a/src/gromacs/mdlib/gpuforcereduction_impl.cpp
+++ b/src/gromacs/mdlib/gpuforcereduction_impl.cpp
@@ -110,6 +110,7 @@ void GpuForceReduction::Impl::registerRvecForce(DeviceBuffer<RVec> forcePtr)
 
 void GpuForceReduction::Impl::addDependency(GpuEventSynchronizer* dependency)
 {
+    GMX_ASSERT(dependency != nullptr, "Force reduction dependency synchronizer should not be NULL");
     dependencyList_.push_back(dependency);
 }
 
@@ -118,32 +119,43 @@ void GpuForceReduction::Impl::execute()
     wallcycle_start_nocount(wcycle_, WallCycleCounter::LaunchGpu);
     wallcycle_sub_start(wcycle_, WallCycleSubCounter::LaunchGpuNBFBufOps);
 
-    if (numAtoms_ == 0)
+    if (numAtoms_ != 0)
     {
-        return;
+        GMX_ASSERT(nbnxmForceToAdd_, "Nbnxm force for reduction has no data");
+
+        // Enqueue wait on all dependencies passed
+        for (auto* synchronizer : dependencyList_)
+        {
+            synchronizer->enqueueWaitEvent(deviceStream_);
+        }
+
+        const bool addRvecForce = static_cast<bool>(rvecForceToAdd_); // True iff initialized
+
+        launchForceReductionKernel(numAtoms_,
+                                   atomStart_,
+                                   addRvecForce,
+                                   accumulate_,
+                                   nbnxmForceToAdd_,
+                                   rvecForceToAdd_,
+                                   baseForce_,
+                                   cellInfo_.d_cell,
+                                   deviceStream_);
     }
-
-    GMX_ASSERT(nbnxmForceToAdd_, "Nbnxm force for reduction has no data");
-
-    // Enqueue wait on all dependencies passed
-    for (auto* synchronizer : dependencyList_)
+    else
     {
-        synchronizer->enqueueWaitEvent(deviceStream_);
+        /* In case we have nothing to do, but still have dependencies, we need
+         * to consume them and mark our own event.
+         * Happens sometimes in MdrunVsitesTest.
+         * Issue #3988, #4227. */
+        for (auto* synchronizer : dependencyList_)
+        {
+            synchronizer->consume();
+        }
     }
 
-    const bool addRvecForce = static_cast<bool>(rvecForceToAdd_); // True iff initialized
-
-    launchForceReductionKernel(numAtoms_,
-                               atomStart_,
-                               addRvecForce,
-                               accumulate_,
-                               nbnxmForceToAdd_,
-                               rvecForceToAdd_,
-                               baseForce_,
-                               cellInfo_.d_cell,
-                               deviceStream_);
-
-    // Mark that kernel has been launched
+    /* Mark that kernel has been launched.
+     * Even if we have no work to do and have not launched the kernel, we still mark the event
+     * in order to ensure proper marking/consumption balance, see Issue #3988, #4227. */
     if (completionMarker_ != nullptr)
     {
         completionMarker_->markEvent(deviceStream_);