Skip to content

Commit 064731d

Browse files
committed
PathMSDBase: drop four omp simd pragmas that cannot vectorize
Building with clang and OpenMP emits, four times: PathMSDBase.cpp:160:19: warning: loop not vectorized: the optimizer was unable to perform the requested transformation ... [-Wpass-failed=transform-warning] All four remarks are attributed to calculate()'s opening line, which hides which loops are at fault. Isolating them by keeping one pragma at a time shows exactly four contributors, and two pragmas that are fine: line 176 imgVec[i].property = ... ; imgVec[i].index = i; 1 warning line 234 tmp_derivs2[i*nat+j] = tmp_derivs[j]; 1 warning line 244 (the same loop again) 1 warning line 266 (the same loop again) 1 warning line 335 derivs_s[i] += tmp*it.distder[i]; 0 - vectorizes line 340 derivs_z[i] += ...; 0 - vectorizes Neither failing case can be vectorized, and asking for it is misleading: - Line 176 is not merely hard to vectorize, it is meaningless. ImagePath::property is a std::vector<double>, so `imgVec[i].property = indexvec[i]` is a vector copy-assignment that ALLOCATES. There is no SIMD work in that loop. - The other three are the same contiguous copy of Vector elements written out by hand. std::copy_n says what is meant, and lets the compiler emit a memmove instead of an element loop the vectorizer then declines to transform. Fixing the source rather than silencing the diagnostic means the warning is gone for every compiler, and a build using -Werror no longer fails here. Behaviour is unchanged: std::copy_n over [0,nat) is exactly the loop it replaces, and the remaining two pragmas, which do vectorize, are untouched. Verified with clang 20.1.1: clang++ -std=c++17 -O3 -fopenmp -march=skylake-avx512 -Wall \ -c colvar/PathMSDBase.cpp -I. 4 warnings before, 0 after. GCC is unaffected either way (its 17 warnings here are pre-existing -Wunknown-pragmas from `#pragma acc` in headers, which PLUMED's own build silences).
1 parent bc65c35 commit 064731d

1 file changed

Lines changed: 7 additions & 13 deletions

File tree

src/colvar/PathMSDBase.cpp

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include "tools/RMSD.h"
2929
#include "tools/Tools.h"
3030

31+
#include <algorithm>
32+
3133
namespace PLMD {
3234
namespace colvar {
3335

@@ -173,7 +175,8 @@ void PathMSDBase::calculate() {
173175
// resize the list to full
174176
if(imgVec.empty()) { // this is the signal that means: recalculate all
175177
imgVec.resize(nframes);
176-
#pragma omp simd
178+
// No `omp simd` here: ImagePath::property is a std::vector<double>, so this body performs a
179+
// vector copy-assignment, i.e. it allocates. There is nothing for a SIMD unit to do.
177180
for(unsigned i=0; i<nframes; i++) {
178181
imgVec[i].property=indexvec[i];
179182
imgVec[i].index=i;
@@ -231,20 +234,14 @@ void PathMSDBase::calculate() {
231234
for(unsigned i=rank; i<imgVec.size(); i+=stride) {
232235
tmp_distances[i] = msdv[imgVec[i].index].calc_Rot(getPositions(), tmp_derivs, tmp_rotationRefClose[imgVec[i].index], true);
233236
plumed_assert(tmp_derivs.size()==nat);
234-
#pragma omp simd
235-
for(unsigned j=0; j<nat; j++) {
236-
tmp_derivs2[i*nat+j]=tmp_derivs[j];
237-
}
237+
std::copy_n(tmp_derivs.begin(), nat, tmp_derivs2.begin()+i*nat);
238238
}
239239
} else {
240240
//approximate distance with saved rotation matrices
241241
for(unsigned i=rank; i<imgVec.size(); i+=stride) {
242242
tmp_distances[i] = msdv[imgVec[i].index].calculateWithCloseStructure(getPositions(), tmp_derivs, rotationPosClose, rotationRefClose[imgVec[i].index], drotationPosCloseDrr01, true);
243243
plumed_assert(tmp_derivs.size()==nat);
244-
#pragma omp simd
245-
for(unsigned j=0; j<nat; j++) {
246-
tmp_derivs2[i*nat+j]=tmp_derivs[j];
247-
}
244+
std::copy_n(tmp_derivs.begin(), nat, tmp_derivs2.begin()+i*nat);
248245
if (debugClose) {
249246
double withclose = tmp_distances[i];
250247
RMSD opt;
@@ -263,10 +260,7 @@ void PathMSDBase::calculate() {
263260
for(unsigned i=rank; i<imgVec.size(); i+=stride) {
264261
tmp_distances[i]=msdv[imgVec[i].index].calculate(getPositions(),tmp_derivs,true);
265262
plumed_assert(tmp_derivs.size()==nat);
266-
#pragma omp simd
267-
for(unsigned j=0; j<nat; j++) {
268-
tmp_derivs2[i*nat+j]=tmp_derivs[j];
269-
}
263+
std::copy_n(tmp_derivs.begin(), nat, tmp_derivs2.begin()+i*nat);
270264
}
271265
}
272266

0 commit comments

Comments
 (0)