Fix SIMD anyTrue bug, and add better unit test

The double precision versions could occasionally
miss elements.

Fixes #2588.

Change-Id: Ie033a3af0e5de25205533868062716a83f33df3c

diff --git a/src/gromacs/simd/impl_x86_avx_256/impl_x86_avx_256_simd_double.h b/src/gromacs/simd/impl_x86_avx_256/impl_x86_avx_256_simd_double.h
index 66cbb7b5291939fdc237db1023f2c6fc619b4342..b3d238722e7288fcffe7697940adc8302c3f774b 100644 (file)
--- a/src/gromacs/simd/impl_x86_avx_256/impl_x86_avx_256_simd_double.h
+++ b/src/gromacs/simd/impl_x86_avx_256/impl_x86_avx_256_simd_double.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.
@@ -650,7 +650,7 @@ operator||(SimdDIBool a, SimdDIBool b)
 }
 
 static inline bool gmx_simdcall
-anyTrue(SimdDIBool a) { return _mm_movemask_epi8(_mm_shuffle_epi32(a.simdInternal_, _MM_SHUFFLE(1, 0, 1, 0))) != 0; }
+anyTrue(SimdDIBool a) { return _mm_movemask_epi8(a.simdInternal_) != 0; }
 
 static inline SimdDInt32 gmx_simdcall
 selectByMask(SimdDInt32 a, SimdDIBool mask)

diff --git a/src/gromacs/simd/tests/simd_floatingpoint.cpp b/src/gromacs/simd/tests/simd_floatingpoint.cpp
index 2c57354c9bb70dcf969f173d910769422ceaa15a..04e7be3f2a3beeb17ab997cb90fc34a79d465a58 100644 (file)
--- a/src/gromacs/simd/tests/simd_floatingpoint.cpp
+++ b/src/gromacs/simd/tests/simd_floatingpoint.cpp
@@ -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.
@@ -36,6 +36,8 @@
 
 #include <cmath>
 
+#include <array>
+
 #include "gromacs/math/utilities.h"
 #include "gromacs/simd/simd.h"
 #include "gromacs/utility/basedefinitions.h"
@@ -414,17 +416,18 @@ TEST_F(SimdFloatingpointTest, orB)
 
 TEST_F(SimdFloatingpointTest, anyTrueB)
 {
-    SimdBool eq;
+    alignas(GMX_SIMD_ALIGNMENT) std::array<real, GMX_SIMD_REAL_WIDTH> mem {};
 
-    /* this test is a bit tricky since we don't know the simd width.
-     * We cannot check for truth values for "any" element beyond the first,
-     * since that part of the data will not be used if simd width is 1.
-     */
-    eq = rSimd_c4c6c8 == setSimdRealFrom3R(c4, 0, 0);
-    EXPECT_TRUE(anyTrue(eq));
+    // Test the false case
+    EXPECT_FALSE(anyTrue(setZero() < load<SimdReal>(mem.data())));
 
-    eq = rSimd_c0c1c2 == rSimd_c3c4c5;
-    EXPECT_FALSE(anyTrue(eq));
+    // Test each bit (these should all be true)
+    for (int i = 0; i < GMX_SIMD_REAL_WIDTH; i++)
+    {
+        mem.fill(0.0);
+        mem[i] = 1.0;
+        EXPECT_TRUE(anyTrue(setZero() < load<SimdReal>(mem.data()))) << "Not detecting true in element " << i;
+    }
 }
 
 TEST_F(SimdFloatingpointTest, blend)

diff --git a/src/gromacs/simd/tests/simd_integer.cpp b/src/gromacs/simd/tests/simd_integer.cpp
index 594827d84d4b943d2c44143a62f8534ac5f5e503..3b093cbf0e8552a5b62d5664617a4f0b9b57ba34 100644 (file)
--- a/src/gromacs/simd/tests/simd_integer.cpp
+++ b/src/gromacs/simd/tests/simd_integer.cpp
@@ -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.
@@ -34,6 +34,8 @@
  */
 #include "gmxpre.h"
 
+#include <array>
+
 #include "gromacs/simd/simd.h"
 #include "gromacs/utility/basedefinitions.h"
 
@@ -274,16 +276,18 @@ TEST_F(SimdIntegerTest, orB)
 
 TEST_F(SimdIntegerTest, anyTrue)
 {
-    SimdIBool eq;
+    alignas(GMX_SIMD_ALIGNMENT) std::array<std::int32_t, GMX_SIMD_REAL_WIDTH> mem {};
 
-    /* See comment in floatingpoint.cpp. We should only check the first element here,
-     * since the SIMD width could be 1 as a special case.
-     */
-    eq = (iSimd_5_7_9 == setSimdIntFrom3I(5, 0, 0));
-    EXPECT_TRUE(anyTrue(eq));
+    // Test the false case
+    EXPECT_FALSE(anyTrue(setZero() < load<SimdInt32>(mem.data())));
 
-    eq = (iSimd_1_2_3 == iSimd_4_5_6);
-    EXPECT_FALSE(anyTrue(eq));
+    // Test each bit (these should all be true)
+    for (int i = 0; i < GMX_SIMD_REAL_WIDTH; i++)
+    {
+        mem.fill(0);
+        mem[i] = 1;
+        EXPECT_TRUE(anyTrue(setZero() < load<SimdInt32>(mem.data()))) << "Not detecting true in element " << i;
+    }
 }
 
 TEST_F(SimdIntegerTest, blend)