Fix atomic operations bug for Min and Max (kokkos#6435)

Fix atomic operations bug for Min and Max and add tests

Co-authored-by: Daniel Arndt <arndtd@ornl.gov>

core/unit_test/TestAtomicOperations_complexdouble.hpp

@@ -16,22 +16,38 @@
 
 #include <TestAtomicOperations.hpp>
 
+using namespace TestAtomicOperations;
+
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_complexdouble) {
 #if defined(KOKKOS_ENABLE_SYCL) && \
     !defined(KOKKOS_IMPL_SYCL_DEVICE_GLOBAL_SUPPORTED)
   if (std::is_same_v<TEST_EXECSPACE, Kokkos::Experimental::SYCL>)
     GTEST_SKIP() << "skipping since device_global variables are not available";
 #endif
-  const int start = 1;  // Avoid zero for division.
+  const int start = -5;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
+    using T   = Kokkos::complex<double>;
+    T old_val = static_cast<T>(i);
+    T update  = static_cast<T>(end - i - start);
+    ASSERT_TRUE(
+        (atomic_op_test<AddAtomicTest, T, TEST_EXECSPACE>(old_val, update)));
     ASSERT_TRUE(
-        (TestAtomicOperations::MulAtomicTest<Kokkos::complex<double>,
-                                             TEST_EXECSPACE>(start, end - i)));
+        (atomic_op_test<SubAtomicTest, T, TEST_EXECSPACE>(old_val, update)));
     ASSERT_TRUE(
-        (TestAtomicOperations::DivAtomicTest<Kokkos::complex<double>,
-                                             TEST_EXECSPACE>(start, end - i)));
+        (atomic_op_test<MulAtomicTest, T, TEST_EXECSPACE>(old_val, update)));
+
+    // FIXME_32BIT disable division test for 32bit where we have accuracy issues
+    // with division atomics still compile it though
+    if (sizeof(void*) == 8) {
+      ASSERT_TRUE((update != 0
+                       ? atomic_op_test<DivAtomicTest, T, TEST_EXECSPACE>(
+                             old_val, update)
+                       : true));
+    }
+    ASSERT_TRUE((atomic_op_test<LoadStoreAtomicTest, T, TEST_EXECSPACE>(
+        old_val, update)));
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_complexfloat.hpp

@@ -16,17 +16,33 @@
 
 #include <TestAtomicOperations.hpp>
 
+using namespace TestAtomicOperations;
+
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_complexfloat) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = -5;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
+    using T   = Kokkos::complex<float>;
+    T old_val = static_cast<T>(i);
+    T update  = static_cast<T>(end - i - start);
+    ASSERT_TRUE(
+        (atomic_op_test<AddAtomicTest, T, TEST_EXECSPACE>(old_val, update)));
     ASSERT_TRUE(
-        (TestAtomicOperations::MulAtomicTest<Kokkos::complex<float>,
-                                             TEST_EXECSPACE>(start, end - i)));
+        (atomic_op_test<SubAtomicTest, T, TEST_EXECSPACE>(old_val, update)));
     ASSERT_TRUE(
-        (TestAtomicOperations::DivAtomicTest<Kokkos::complex<float>,
-                                             TEST_EXECSPACE>(start, end - i)));
+        (atomic_op_test<MulAtomicTest, T, TEST_EXECSPACE>(old_val, update)));
+
+    // FIXME_32BIT disable division test for 32bit where we have accuracy issues
+    // with division atomics still compile it though
+    if (sizeof(void*) == 8) {
+      ASSERT_TRUE((update != 0
+                       ? atomic_op_test<DivAtomicTest, T, TEST_EXECSPACE>(
+                             old_val, update)
+                       : true));
+    }
+    ASSERT_TRUE((atomic_op_test<LoadStoreAtomicTest, T, TEST_EXECSPACE>(
+        old_val, update)));
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_double.hpp

@@ -18,19 +18,16 @@
 
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_double) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = -5;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 double, TEST_EXECSPACE>(start, end - i, 1)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 double, TEST_EXECSPACE>(start, end - i, 2)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 double, TEST_EXECSPACE>(start, end - i, 3)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 double, TEST_EXECSPACE>(start, end - i, 4)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 double, TEST_EXECSPACE>(start, end - i, 5)));
+    for (int t = 0; t < 8; t++)
+      // FIXME_32BIT disable division test for 32bit where we have accuracy
+      // issues with division atomics still compile it though
+      if (t != 5 || sizeof(void*) == 8) {
+        ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
+                     double, TEST_EXECSPACE>(i, end - i + start, t)));
+      }
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_float.hpp

@@ -18,19 +18,16 @@
 
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_float) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = -5;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 float, TEST_EXECSPACE>(start, end - i, 1)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 float, TEST_EXECSPACE>(start, end - i, 2)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 float, TEST_EXECSPACE>(start, end - i, 3)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 float, TEST_EXECSPACE>(start, end - i, 4)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
-                 float, TEST_EXECSPACE>(start, end - i, 5)));
+    for (int t = 0; t < 8; t++)
+      // FIXME_32BIT disable division test for 32bit where we have accuracy
+      // issues with division atomics still compile it though
+      if (t != 5 || sizeof(void*) == 8) {
+        ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestNonIntegralType<
+                     double, TEST_EXECSPACE>(i, end - i + start, t)));
+      }
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_int.hpp

@@ -18,33 +18,12 @@
 
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_int) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = -5;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 1)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 2)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 3)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 4)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 5)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 6)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 7)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 8)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 9)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 11)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 12)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 int, TEST_EXECSPACE>(start, end - i, 13)));
+    for (int t = 0; t < 16; t++)
+      ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
+                   int, TEST_EXECSPACE>(i, end - i + start, t)));
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_longint.hpp

@@ -18,33 +18,12 @@
 
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_long) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = -5;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 1)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 2)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 3)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 4)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 5)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 6)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 7)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 8)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 9)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 11)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 12)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long int, TEST_EXECSPACE>(start, end - i, 13)));
+    for (int t = 0; t < 16; t++)
+      ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
+                   long int, TEST_EXECSPACE>(i, end - i + start, t)));
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_longlongint.hpp

@@ -18,33 +18,12 @@
 
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_longlong) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = -5;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 1)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 2)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 3)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 4)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 5)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 6)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 7)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 8)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 9)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 11)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 12)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 long long int, TEST_EXECSPACE>(start, end - i, 13)));
+    for (int t = 0; t < 16; t++)
+      ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
+                   long long int, TEST_EXECSPACE>(i, end - i + start, t)));
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_unsignedint.hpp

@@ -18,39 +18,18 @@
 
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_unsigned) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = 0;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 1)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 2)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 3)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 4)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 5)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 6)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 7)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 8)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 9)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 11)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 12)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned int, TEST_EXECSPACE>(start, end - i, 13)));
+    for (int t = 0; t < 16; t++)
+      ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
+                   unsigned int, TEST_EXECSPACE>(i, end - i + start, t)));
     ASSERT_TRUE(
         (TestAtomicOperations::AtomicOperationsTestUnsignedIntegralType<
-            unsigned int, TEST_EXECSPACE>(start, end - i, 1)));  // Wrapping Inc
+            unsigned int, TEST_EXECSPACE>(i, end - i, 1)));  // Wrapping Inc
     ASSERT_TRUE(
         (TestAtomicOperations::AtomicOperationsTestUnsignedIntegralType<
-            unsigned int, TEST_EXECSPACE>(start, end - i, 2)));  // Wrapping Dec
+            unsigned int, TEST_EXECSPACE>(i, end - i, 2)));  // Wrapping Dec
   }
 }
 }  // namespace Test

core/unit_test/TestAtomicOperations_unsignedlongint.hpp

@@ -18,38 +18,17 @@
 
 namespace Test {
 TEST(TEST_CATEGORY, atomic_operations_unsignedlong) {
-  const int start = 1;  // Avoid zero for division.
+  const int start = 0;
   const int end   = 11;
   for (int i = start; i < end; ++i) {
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 1)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 2)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 3)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 4)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 5)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 6)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 7)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 8)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 9)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 11)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 12)));
-    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i, 13)));
+    for (int t = 0; t < 16; t++)
+      ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestIntegralType<
+                   unsigned long int, TEST_EXECSPACE>(i, end - i + start, t)));
     ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestUnsignedIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i,
+                 unsigned long int, TEST_EXECSPACE>(i, end - i,
                                                     1)));  // Wrapping Inc
     ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestUnsignedIntegralType<
-                 unsigned long int, TEST_EXECSPACE>(start, end - i,
+                 unsigned long int, TEST_EXECSPACE>(i, end - i,
                                                     2)));  // Wrapping Dec
   }
 }

core/unit_test/TestAtomicOperations_unsignedlonglongint.hpp

@@ -0,0 +1,36 @@
+//@HEADER
+// ************************************************************************
+//
+//                        Kokkos v. 4.0
+//       Copyright (2022) National Technology & Engineering
+//               Solutions of Sandia, LLC (NTESS).
+//
+// Under the terms of Contract DE-NA0003525 with NTESS,
+// the U.S. Government retains certain rights in this software.
+//
+// Part of Kokkos, under the Apache License v2.0 with LLVM Exceptions.
+// See https://kokkos.org/LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//@HEADER
+
+#include <TestAtomicOperations.hpp>
+
+namespace Test {
+TEST(TEST_CATEGORY, atomic_operations_unsignedlonglong) {
+  const int start = 0;
+  const int end   = 11;
+  for (int i = start; i < end; ++i) {
+    for (int t = 0; t < 16; t++)
+      ASSERT_TRUE(
+          (TestAtomicOperations::AtomicOperationsTestIntegralType<
+              unsigned long long int, TEST_EXECSPACE>(i, end - i + start, t)));
+    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestUnsignedIntegralType<
+                 unsigned long long int, TEST_EXECSPACE>(i, end - i,
+                                                         1)));  // Wrapping Inc
+    ASSERT_TRUE((TestAtomicOperations::AtomicOperationsTestUnsignedIntegralType<
+                 unsigned long long int, TEST_EXECSPACE>(i, end - i,
+                                                         2)));  // Wrapping Dec
+  }
+}
+}  // namespace Test