From 71490dd65b42210c404919b7a4a648a7f0c8456d Mon Sep 17 00:00:00 2001 From: Roland Schulz Date: Tue, 25 Nov 2014 20:40:30 -0500 Subject: [PATCH] Use bitmask for bonded Also move bitmask to utility and f_thread_t to forcerec-threading.h. This introduces a cycle because listed-forces now depends on mdlib/forcerec-threading.h. There is not yet a module where forcerec-threading.h can go. Change-Id: Ic220be2880f7263aa0e50b13f07ed509eb86e1b3 --- docs/doxygen/cycle-suppressions.txt | 1 + src/gromacs/legacyheaders/types/forcerec.h | 14 +---- src/gromacs/listed-forces/listed-forces.cpp | 7 ++- .../listed-forces/manage-threading.cpp | 38 ++++++----- src/gromacs/mdlib/clincs.cpp | 2 +- src/gromacs/mdlib/force.cpp | 1 + src/gromacs/mdlib/forcerec-threading.h | 63 +++++++++++++++++++ src/gromacs/mdlib/forcerec.cpp | 1 + src/gromacs/mdlib/nbnxn_pairlist.h | 2 +- src/gromacs/mdlib/tests/CMakeLists.txt | 1 - src/gromacs/{mdlib => utility}/bitmask.h | 1 + src/gromacs/utility/tests/CMakeLists.txt | 1 + .../{mdlib => utility}/tests/bitmask.h | 3 +- .../{mdlib => utility}/tests/bitmask128.cpp | 0 .../{mdlib => utility}/tests/bitmask32.cpp | 0 .../{mdlib => utility}/tests/bitmask64.cpp | 0 16 files changed, 99 insertions(+), 36 deletions(-) create mode 100644 src/gromacs/mdlib/forcerec-threading.h rename src/gromacs/{mdlib => utility}/bitmask.h (99%) rename src/gromacs/{mdlib => utility}/tests/bitmask.h (98%) rename src/gromacs/{mdlib => utility}/tests/bitmask128.cpp (100%) rename src/gromacs/{mdlib => utility}/tests/bitmask32.cpp (100%) rename src/gromacs/{mdlib => utility}/tests/bitmask64.cpp (100%) diff --git a/docs/doxygen/cycle-suppressions.txt b/docs/doxygen/cycle-suppressions.txt index eae10369dc..38023f9f2d 100644 --- a/docs/doxygen/cycle-suppressions.txt +++ b/docs/doxygen/cycle-suppressions.txt @@ -19,3 +19,4 @@ topology -> fileio topology -> legacyheaders pbcutil -> fileio pbcutil -> legacyheaders +listed-forces -> mdlib diff --git a/src/gromacs/legacyheaders/types/forcerec.h b/src/gromacs/legacyheaders/types/forcerec.h index 59cd9f4279..49f32af748 100644 --- a/src/gromacs/legacyheaders/types/forcerec.h +++ b/src/gromacs/legacyheaders/types/forcerec.h @@ -185,19 +185,7 @@ typedef struct { /* Forward declaration of type for managing Ewald tables */ struct gmx_ewald_tab_t; -typedef struct { - rvec *f; - int f_nalloc; - unsigned red_mask; /* Mask for marking which parts of f are filled */ - rvec *fshift; - real ener[F_NRE]; - gmx_grppairener_t grpp; - real Vcorr_q; - real Vcorr_lj; - real dvdl[efptNR]; - tensor vir_q; - tensor vir_lj; -} f_thread_t; +typedef struct f_thread_t f_thread_t; typedef struct { interaction_const_t *ic; diff --git a/src/gromacs/listed-forces/listed-forces.cpp b/src/gromacs/listed-forces/listed-forces.cpp index fd46c3519c..e23f348065 100644 --- a/src/gromacs/listed-forces/listed-forces.cpp +++ b/src/gromacs/listed-forces/listed-forces.cpp @@ -60,6 +60,7 @@ #include "gromacs/listed-forces/bonded.h" #include "gromacs/listed-forces/position-restraints.h" #include "gromacs/math/vec.h" +#include "gromacs/mdlib/forcerec-threading.h" #include "gromacs/pbcutil/ishift.h" #include "gromacs/pbcutil/pbc.h" #include "gromacs/timing/wallcycle.h" @@ -92,11 +93,11 @@ zero_thread_forces(f_thread_t *f_t, int n, srenew(f_t->f, f_t->f_nalloc); } - if (f_t->red_mask != 0) + if (!bitmask_is_zero(f_t->red_mask)) { for (b = 0; b < nblock; b++) { - if (f_t->red_mask && (1U<red_mask, b)) { a0 = b*blocksize; a1 = std::min((b+1)*blocksize, n); @@ -161,7 +162,7 @@ reduce_thread_force_buffer(int n, rvec *f, nfb = 0; for (ft = 1; ft < nthreads; ft++) { - if (f_t[ft].red_mask & (1U< #include "gromacs/listed-forces/bonded.h" +#include "gromacs/mdlib/forcerec-threading.h" #include "gromacs/utility/fatalerror.h" #include "gromacs/utility/smalloc.h" +#include "gromacs/utility/stringutil.h" //! Divides listed interactions over threads static void divide_bondeds_over_threads(t_idef *idef, int nthreads) @@ -110,15 +112,14 @@ static void divide_bondeds_over_threads(t_idef *idef, int nthreads) } //! Construct a reduction mask for which interaction was computed on which thread -static unsigned -calc_bonded_reduction_mask(const t_idef *idef, +static void +calc_bonded_reduction_mask(gmx_bitmask_t *mask, + const t_idef *idef, int shift, int t, int nt) { - unsigned mask; - int ftype, nb, nat1, nb0, nb1, i, a; - - mask = 0; + int ftype, nb, nat1, nb0, nb1, i, a; + bitmask_clear(mask); for (ftype = 0; ftype < F_NRE; ftype++) { @@ -139,21 +140,19 @@ calc_bonded_reduction_mask(const t_idef *idef, { for (a = 1; a < nat1; a++) { - mask |= (1U << (idef->il[ftype].iatoms[i+a]>>shift)); + bitmask_set_bit(mask, idef->il[ftype].iatoms[i+a]>>shift); } } } } } - - return mask; } -/*! \brief We divide the force array in a maximum of 32 blocks. +/*! \brief We divide the force array in a maximum of BITMASK_SIZE (default 32) blocks. * Minimum force block reduction size is thus 2^6=64. */ -const int maxBlockBits = 32; +const int maxBlockBits = BITMASK_SIZE; void setup_bonded_threading(t_forcerec *fr, t_idef *idef) { @@ -189,8 +188,8 @@ void setup_bonded_threading(t_forcerec *fr, t_idef *idef) #pragma omp parallel for num_threads(fr->nthreads) schedule(static) for (t = 1; t < fr->nthreads; t++) { - fr->f_t[t].red_mask = - calc_bonded_reduction_mask(idef, fr->red_ashift, t, fr->nthreads); + calc_bonded_reduction_mask(&fr->f_t[t].red_mask, + idef, fr->red_ashift, t, fr->nthreads); } /* Determine the maximum number of blocks we need to reduce over */ @@ -201,7 +200,7 @@ void setup_bonded_threading(t_forcerec *fr, t_idef *idef) c = 0; for (b = 0; b < maxBlockBits; b++) { - if (fr->f_t[t].red_mask & (1U<f_t[t].red_mask, b)) { fr->red_nblock = std::max(fr->red_nblock, b+1); c++; @@ -209,8 +208,15 @@ void setup_bonded_threading(t_forcerec *fr, t_idef *idef) } if (debug) { - fprintf(debug, "thread %d flags %x count %d\n", - t, fr->f_t[t].red_mask, c); +#if BITMASK_SIZE <= 64 //move into bitmask when it is C++ + std::string flags = gmx::formatString("%x", fr->f_t[t].red_mask); +#else + std::string flags = gmx::formatAndJoin(fr->f_t[t].red_mask, + fr->f_t[t].red_mask+BITMASK_ALEN, + "", gmx::StringFormatter("%x")); +#endif + fprintf(debug, "thread %d flags %s count %d\n", + t, flags.c_str(), c); } ctot += c; } diff --git a/src/gromacs/mdlib/clincs.cpp b/src/gromacs/mdlib/clincs.cpp index aa4e633f77..480a96596d 100644 --- a/src/gromacs/mdlib/clincs.cpp +++ b/src/gromacs/mdlib/clincs.cpp @@ -50,10 +50,10 @@ #include "gromacs/legacyheaders/types/commrec.h" #include "gromacs/math/units.h" #include "gromacs/math/vec.h" -#include "gromacs/mdlib/bitmask.h" #include "gromacs/pbcutil/pbc.h" #include "gromacs/topology/block.h" #include "gromacs/topology/mtop_util.h" +#include "gromacs/utility/bitmask.h" #include "gromacs/utility/fatalerror.h" #include "gromacs/utility/gmxomp.h" #include "gromacs/utility/smalloc.h" diff --git a/src/gromacs/mdlib/force.cpp b/src/gromacs/mdlib/force.cpp index 1639369318..6eab0bb0b0 100644 --- a/src/gromacs/mdlib/force.cpp +++ b/src/gromacs/mdlib/force.cpp @@ -62,6 +62,7 @@ #include "gromacs/legacyheaders/types/commrec.h" #include "gromacs/listed-forces/listed-forces.h" #include "gromacs/math/vec.h" +#include "gromacs/mdlib/forcerec-threading.h" #include "gromacs/pbcutil/ishift.h" #include "gromacs/pbcutil/mshift.h" #include "gromacs/pbcutil/pbc.h" diff --git a/src/gromacs/mdlib/forcerec-threading.h b/src/gromacs/mdlib/forcerec-threading.h new file mode 100644 index 0000000000..b40a18ba32 --- /dev/null +++ b/src/gromacs/mdlib/forcerec-threading.h @@ -0,0 +1,63 @@ +/* + * This file is part of the GROMACS molecular simulation package. + * + * Copyright (c) 2014, 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. + * + * GROMACS 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 2.1 + * of the License, or (at your option) any later version. + * + * GROMACS 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 GROMACS; if not, see + * http://www.gnu.org/licenses, or write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * If you want to redistribute modifications to GROMACS, please + * consider that scientific software is very special. Version + * control is crucial - bugs must be traceable. We will be happy to + * consider code for inclusion in the official distribution, but + * derived work must not be called official GROMACS. Details are found + * in the README & COPYING files - if they are missing, get the + * official version at http://www.gromacs.org. + * + * To help us fund GROMACS development, we humbly ask that you cite + * the research papers on the package. Check out http://www.gromacs.org. + */ + +#ifndef GMX_MDLIB_FORCEREC_THREADING_H +#define GMX_MDLIB_FORCEREC_THREADING_H + +#include "gromacs/utility/bitmask.h" + +#ifdef __cplusplus +extern "C" { +#endif + +struct f_thread_t { + rvec *f; + int f_nalloc; + gmx_bitmask_t red_mask; /* Mask for marking which parts of f are filled */ + rvec *fshift; + real ener[F_NRE]; + gmx_grppairener_t grpp; + real Vcorr_q; + real Vcorr_lj; + real dvdl[efptNR]; + tensor vir_q; + tensor vir_lj; +}; + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/src/gromacs/mdlib/forcerec.cpp b/src/gromacs/mdlib/forcerec.cpp index 611642825d..d3bb4be6f6 100644 --- a/src/gromacs/mdlib/forcerec.cpp +++ b/src/gromacs/mdlib/forcerec.cpp @@ -68,6 +68,7 @@ #include "gromacs/math/units.h" #include "gromacs/math/utilities.h" #include "gromacs/math/vec.h" +#include "gromacs/mdlib/forcerec-threading.h" #include "gromacs/mdlib/nb_verlet.h" #include "gromacs/mdlib/nbnxn_atomdata.h" #include "gromacs/mdlib/nbnxn_consts.h" diff --git a/src/gromacs/mdlib/nbnxn_pairlist.h b/src/gromacs/mdlib/nbnxn_pairlist.h index 09d429a27d..09163178a3 100644 --- a/src/gromacs/mdlib/nbnxn_pairlist.h +++ b/src/gromacs/mdlib/nbnxn_pairlist.h @@ -42,8 +42,8 @@ #include "gromacs/legacyheaders/types/nblist.h" #include "gromacs/math/vectypes.h" -#include "gromacs/mdlib/bitmask.h" #include "gromacs/utility/basedefinitions.h" +#include "gromacs/utility/bitmask.h" #include "gromacs/utility/real.h" #ifdef __cplusplus diff --git a/src/gromacs/mdlib/tests/CMakeLists.txt b/src/gromacs/mdlib/tests/CMakeLists.txt index 4e00c4717a..ba2e72945a 100644 --- a/src/gromacs/mdlib/tests/CMakeLists.txt +++ b/src/gromacs/mdlib/tests/CMakeLists.txt @@ -33,5 +33,4 @@ # the research papers on the package. Check out http://www.gromacs.org. gmx_add_unit_test(MdlibUnitTest mdlib-test - bitmask32.cpp bitmask64.cpp bitmask128.cpp shake.cpp) diff --git a/src/gromacs/mdlib/bitmask.h b/src/gromacs/utility/bitmask.h similarity index 99% rename from src/gromacs/mdlib/bitmask.h rename to src/gromacs/utility/bitmask.h index 4b9c812513..e1c5989669 100644 --- a/src/gromacs/mdlib/bitmask.h +++ b/src/gromacs/utility/bitmask.h @@ -38,6 +38,7 @@ * * \author Roland Schulz * \inlibraryapi + * \ingroup module_utility */ #ifndef GMX_MDLIB_BITMASK_H diff --git a/src/gromacs/utility/tests/CMakeLists.txt b/src/gromacs/utility/tests/CMakeLists.txt index 49c752b857..9024e56ea6 100644 --- a/src/gromacs/utility/tests/CMakeLists.txt +++ b/src/gromacs/utility/tests/CMakeLists.txt @@ -33,4 +33,5 @@ # the research papers on the package. Check out http://www.gromacs.org. gmx_add_unit_test(UtilityUnitTests utility-test + bitmask32.cpp bitmask64.cpp bitmask128.cpp stringutil.cpp) diff --git a/src/gromacs/mdlib/tests/bitmask.h b/src/gromacs/utility/tests/bitmask.h similarity index 98% rename from src/gromacs/mdlib/tests/bitmask.h rename to src/gromacs/utility/tests/bitmask.h index 7041c770c2..3cc5376b1f 100644 --- a/src/gromacs/mdlib/tests/bitmask.h +++ b/src/gromacs/utility/tests/bitmask.h @@ -39,10 +39,11 @@ * These tests check the functionality of bitmask.h * \author Roland Schulz + * \ingroup module_utility */ #include -#include "gromacs/mdlib/bitmask.h" +#include "gromacs/utility/bitmask.h" //! Implemenation of BITMASK_CLASSNAME #define BITMASK_CLASSNAME_(S) BitmaskTest ## S diff --git a/src/gromacs/mdlib/tests/bitmask128.cpp b/src/gromacs/utility/tests/bitmask128.cpp similarity index 100% rename from src/gromacs/mdlib/tests/bitmask128.cpp rename to src/gromacs/utility/tests/bitmask128.cpp diff --git a/src/gromacs/mdlib/tests/bitmask32.cpp b/src/gromacs/utility/tests/bitmask32.cpp similarity index 100% rename from src/gromacs/mdlib/tests/bitmask32.cpp rename to src/gromacs/utility/tests/bitmask32.cpp diff --git a/src/gromacs/mdlib/tests/bitmask64.cpp b/src/gromacs/utility/tests/bitmask64.cpp similarity index 100% rename from src/gromacs/mdlib/tests/bitmask64.cpp rename to src/gromacs/utility/tests/bitmask64.cpp -- 2.22.0