From a4987be36f3a801984a0aa88a8505d85709da04f Mon Sep 17 00:00:00 2001 From: Aleksei Iupinov Date: Mon, 12 Feb 2018 10:16:15 +0100 Subject: [PATCH] Add freeDeviceBuffer GPU utility function This allows to make the GPU pairlist deletion code look same for CUDA and OpenCL. More cleanup TODOs are left. Change-Id: I4f7775ae2bda65b69696f7aacdb1d3fea9c62ac5 --- src/gromacs/gpu_utils/cudautils.cuh | 15 ++++++++++ src/gromacs/gpu_utils/oclutils.h | 17 ++++++++++- .../mdlib/nbnxn_cuda/nbnxn_cuda_data_mgmt.cu | 29 +++++++++---------- .../mdlib/nbnxn_ocl/nbnxn_ocl_data_mgmt.cpp | 22 +++++++------- 4 files changed, 56 insertions(+), 27 deletions(-) diff --git a/src/gromacs/gpu_utils/cudautils.cuh b/src/gromacs/gpu_utils/cudautils.cuh index 5d985c81fc..d74ecd4d8d 100644 --- a/src/gromacs/gpu_utils/cudautils.cuh +++ b/src/gromacs/gpu_utils/cudautils.cuh @@ -297,4 +297,19 @@ static inline bool haveStreamTasksCompleted(cudaStream_t s) return true; } +/*! \brief Free a device-side buffer. + * TODO: fully replace cu_free_buffered with this. + * + * \param[in] buffer Pointer to the buffer to free. + */ +template +void freeDeviceBuffer(DeviceBuffer *buffer) +{ + GMX_ASSERT(buffer, "needs a buffer pointer"); + if (*buffer) + { + GMX_RELEASE_ASSERT(cudaFree(*buffer) == cudaSuccess, "cudaFree failed"); + } +} + #endif diff --git a/src/gromacs/gpu_utils/oclutils.h b/src/gromacs/gpu_utils/oclutils.h index 2b2c82a88a..e4bc91b320 100644 --- a/src/gromacs/gpu_utils/oclutils.h +++ b/src/gromacs/gpu_utils/oclutils.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2014,2015,2016,2017, by the GROMACS development team, led by + * Copyright (c) 2014,2015,2016,2017,2018, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -177,4 +177,19 @@ static inline bool haveStreamTasksCompleted(cl_command_queue gmx_unused s) return false; } +/*! \brief Free a device-side buffer. + * TODO: fully replace free_ocl_buffer and ocl_free_buffered with this. + * + * \param[in] buffer Pointer to the buffer to free. + */ +template +void freeDeviceBuffer(DeviceBuffer *buffer) +{ + GMX_ASSERT(buffer, "needs a buffer pointer"); + if (*buffer) + { + GMX_RELEASE_ASSERT(clReleaseMemObject(*buffer) == CL_SUCCESS, "clReleaseMemObject failed"); + } +} + #endif diff --git a/src/gromacs/mdlib/nbnxn_cuda/nbnxn_cuda_data_mgmt.cu b/src/gromacs/mdlib/nbnxn_cuda/nbnxn_cuda_data_mgmt.cu index cc8ba63b0e..ea5375667d 100644 --- a/src/gromacs/mdlib/nbnxn_cuda/nbnxn_cuda_data_mgmt.cu +++ b/src/gromacs/mdlib/nbnxn_cuda/nbnxn_cuda_data_mgmt.cu @@ -734,7 +734,6 @@ void nbnxn_gpu_free(gmx_nbnxn_cuda_t *nb) cudaError_t stat; cu_atomdata_t *atdat; cu_nbparam_t *nbparam; - cu_plist_t *plist, *plist_nl; if (nb == NULL) { @@ -743,8 +742,6 @@ void nbnxn_gpu_free(gmx_nbnxn_cuda_t *nb) atdat = nb->atdat; nbparam = nb->nbparam; - plist = nb->plist[eintLocal]; - plist_nl = nb->plist[eintNonlocal]; nbnxn_cuda_free_nbparam_table(nbparam, nb->dev_info); @@ -792,25 +789,25 @@ void nbnxn_gpu_free(gmx_nbnxn_cuda_t *nb) cu_free_buffered(atdat->atom_types, &atdat->ntypes); cu_free_buffered(atdat->lj_comb); - cu_free_buffered(plist->sci, &plist->nsci, &plist->sci_nalloc); - cu_free_buffered(plist->cj4, &plist->ncj4, &plist->cj4_nalloc); - cu_free_buffered(plist->imask, &plist->nimask, &plist->imask_nalloc); - cu_free_buffered(plist->excl, &plist->nexcl, &plist->excl_nalloc); + /* Free plist */ + auto *plist = nb->plist[eintLocal]; + freeDeviceBuffer(&plist->sci); + freeDeviceBuffer(&plist->cj4); + freeDeviceBuffer(&plist->imask); + freeDeviceBuffer(&plist->excl); + sfree(plist); if (nb->bUseTwoStreams) { - cu_free_buffered(plist_nl->sci, &plist_nl->nsci, &plist_nl->sci_nalloc); - cu_free_buffered(plist_nl->cj4, &plist_nl->ncj4, &plist_nl->cj4_nalloc); - cu_free_buffered(plist_nl->imask, &plist_nl->nimask, &plist_nl->imask_nalloc); - cu_free_buffered(plist_nl->excl, &plist_nl->nexcl, &plist->excl_nalloc); + auto *plist_nl = nb->plist[eintNonlocal]; + freeDeviceBuffer(&plist_nl->sci); + freeDeviceBuffer(&plist_nl->cj4); + freeDeviceBuffer(&plist_nl->imask); + freeDeviceBuffer(&plist_nl->excl); + sfree(plist_nl); } sfree(atdat); sfree(nbparam); - sfree(plist); - if (nb->bUseTwoStreams) - { - sfree(plist_nl); - } sfree(nb->timings); sfree(nb); diff --git a/src/gromacs/mdlib/nbnxn_ocl/nbnxn_ocl_data_mgmt.cpp b/src/gromacs/mdlib/nbnxn_ocl/nbnxn_ocl_data_mgmt.cpp index e65160374f..189c06944f 100644 --- a/src/gromacs/mdlib/nbnxn_ocl/nbnxn_ocl_data_mgmt.cpp +++ b/src/gromacs/mdlib/nbnxn_ocl/nbnxn_ocl_data_mgmt.cpp @@ -1180,18 +1180,20 @@ void nbnxn_gpu_free(gmx_nbnxn_ocl_t *nb) sfree(nb->nbparam); /* Free plist */ - free_ocl_buffer(&(nb->plist[eintLocal]->sci)); - free_ocl_buffer(&(nb->plist[eintLocal]->cj4)); - free_ocl_buffer(&(nb->plist[eintLocal]->imask)); - free_ocl_buffer(&(nb->plist[eintLocal]->excl)); - sfree(nb->plist[eintLocal]); + auto *plist = nb->plist[eintLocal]; + freeDeviceBuffer(&plist->sci); + freeDeviceBuffer(&plist->cj4); + freeDeviceBuffer(&plist->imask); + freeDeviceBuffer(&plist->excl); + sfree(plist); if (nb->bUseTwoStreams) { - free_ocl_buffer(&(nb->plist[eintNonlocal]->sci)); - free_ocl_buffer(&(nb->plist[eintNonlocal]->cj4)); - free_ocl_buffer(&(nb->plist[eintNonlocal]->imask)); - free_ocl_buffer(&(nb->plist[eintNonlocal]->excl)); - sfree(nb->plist[eintNonlocal]); + auto *plist_nl = nb->plist[eintNonlocal]; + freeDeviceBuffer(&plist_nl->sci); + freeDeviceBuffer(&plist_nl->cj4); + freeDeviceBuffer(&plist_nl->imask); + freeDeviceBuffer(&plist_nl->excl); + sfree(plist_nl); } /* Free nbst */ -- 2.22.0