From b9e7735881e25504000f955006c1857013d03749 Mon Sep 17 00:00:00 2001 From: Andrey Alekseenko Date: Tue, 3 Aug 2021 07:01:34 +0000 Subject: [PATCH] Remove StatePropagatorDataGpu::getVelocitiesReadyOnDeviceEvent It was unused, since we were using the copied data only in the stream in which we did the copy. So, the synchronization was performed implicitly, with no need to wait for the event. Also added a comment about a rare redundant copy. Refs #3988 --- src/gromacs/mdrun/md.cpp | 4 +++- src/gromacs/mdtypes/state_propagator_data_gpu.h | 14 ++++++-------- .../mdtypes/state_propagator_data_gpu_impl.cpp | 8 -------- .../mdtypes/state_propagator_data_gpu_impl.h | 10 ---------- .../state_propagator_data_gpu_impl_gpu.cpp | 16 ++++------------ 5 files changed, 13 insertions(+), 39 deletions(-) diff --git a/src/gromacs/mdrun/md.cpp b/src/gromacs/mdrun/md.cpp index c26f8f9684..30f17fdf14 100644 --- a/src/gromacs/mdrun/md.cpp +++ b/src/gromacs/mdrun/md.cpp @@ -1507,8 +1507,10 @@ void gmx::LegacySimulator::do_md() *md); // Copy data to the GPU after buffers might have being reinitialized - // coordinates have been copied already if PME or buffer ops has not needed it this step. + /* The velocity copy is redundant if we had Center-of-Mass motion removed on + * the previous step. We don't check that now. */ stateGpu->copyVelocitiesToGpu(state->v, AtomLocality::Local); + // coordinates have been copied already if PME or buffer ops has not needed it this step. const bool useGpuPmeOnThisRank = runScheduleWork->simulationWork.useGpuPme && thisRankHasDuty(cr, DUTY_PME) && runScheduleWork->stepWork.computeSlowForces; diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu.h b/src/gromacs/mdtypes/state_propagator_data_gpu.h index cd119960f3..16e87d31e9 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu.h +++ b/src/gromacs/mdtypes/state_propagator_data_gpu.h @@ -168,6 +168,10 @@ public: DeviceBuffer getCoordinates(); /*! \brief Copy positions to the GPU memory. + * + * Use \ref getCoordinatesReadyOnDeviceEvent to get the associated event synchronizer or + * \ref waitCoordinatesCopiedToDevice to wait for the copy completion. + * Note: the event is not marked in OpenCL, because it is not used. * * \param[in] h_x Positions in the host memory. * \param[in] atomLocality Locality of the particles to copy. @@ -229,20 +233,14 @@ public: DeviceBuffer getVelocities(); /*! \brief Copy velocities to the GPU memory. + * + * Does not mark any event, because we don't use it anywhere at the moment. * * \param[in] h_v Velocities in the host memory. * \param[in] atomLocality Locality of the particles to copy. */ void copyVelocitiesToGpu(gmx::ArrayRef h_v, AtomLocality atomLocality); - /*! \brief Get the event synchronizer for the H2D velocities copy. - * - * \param[in] atomLocality Locality of the particles to wait for. - * - * \returns The event to synchronize the stream that consumes velocities on device. - */ - GpuEventSynchronizer* getVelocitiesReadyOnDeviceEvent(AtomLocality atomLocality); - /*! \brief Copy velocities from the GPU memory. * * \param[in] h_v Velocities buffer in the host memory. diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp index 0ce99a4dd4..4f69d96370 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp +++ b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp @@ -166,14 +166,6 @@ void StatePropagatorDataGpu::copyVelocitiesToGpu(const gmx::ArrayRef /* h_v */, AtomLocality /* atomLocality */) { diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.h b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.h index 0a041db04f..cfceab3bff 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.h +++ b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.h @@ -230,14 +230,6 @@ public: */ void copyVelocitiesToGpu(gmx::ArrayRef h_v, AtomLocality atomLocality); - /*! \brief Get the event synchronizer on the H2D velocities copy. - * - * \param[in] atomLocality Locality of the particles to wait for. - * - * \returns The event to synchronize the stream that consumes velocities on device. - */ - GpuEventSynchronizer* getVelocitiesReadyOnDeviceEvent(AtomLocality atomLocality); - /*! \brief Copy velocities from the GPU memory. * * \param[in] h_v Velocities buffer in the host memory. @@ -353,8 +345,6 @@ private: //! An array of events that indicate D2H copy of coordinates is complete (one event for each atom locality) EnumerationArray xReadyOnHost_; - //! An array of events that indicate H2D copy of velocities is complete (one event for each atom locality) - EnumerationArray vReadyOnDevice_; //! An array of events that indicate D2H copy of velocities is complete (one event for each atom locality) EnumerationArray vReadyOnHost_; diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp b/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp index d4868d20dc..93a0839b9e 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp +++ b/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp @@ -417,18 +417,15 @@ void StatePropagatorDataGpu::Impl::copyVelocitiesToGpu(const gmx::ArrayRef h_v, AtomLocality atomLocality) { GMX_ASSERT(atomLocality < AtomLocality::Count, "Wrong atom locality."); @@ -637,11 +634,6 @@ void StatePropagatorDataGpu::copyVelocitiesToGpu(const gmx::ArrayRefcopyVelocitiesToGpu(h_v, atomLocality); } -GpuEventSynchronizer* StatePropagatorDataGpu::getVelocitiesReadyOnDeviceEvent(AtomLocality atomLocality) -{ - return impl_->getVelocitiesReadyOnDeviceEvent(atomLocality); -} - void StatePropagatorDataGpu::copyVelocitiesFromGpu(gmx::ArrayRef h_v, AtomLocality atomLocality) { return impl_->copyVelocitiesFromGpu(h_v, atomLocality); -- 2.22.0