Fix a bug when using realloc on views of non-default constructible el…

…ement types (kokkos#6993) * Add few missing constexpr for alloc_prop_input Co-authored-by: Daniel Arndt <arndtd@ornl.gov> * Update tests * Fix DualView * Address review comments * Add missing decorators * Move NoDefaultConstructor out of function --------- Co-authored-by: Daniel Arndt <arndtd@ornl.gov>
masterleinad · May 8, 2024 · d61d75a · d61d75a
1 parent e4cc686
commit d61d75a
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 6 deletions.
diff --git a/containers/src/Kokkos_DualView.hpp b/containers/src/Kokkos_DualView.hpp
@@ -944,13 +944,13 @@ class DualView : public ViewTraits<DataType, Properties...> {
 
     if (sizeMismatch) {
       ::Kokkos::realloc(arg_prop, d_view, n0, n1, n2, n3, n4, n5, n6, n7);
-      if (alloc_prop_input::initialize) {
+      if constexpr (alloc_prop_input::initialize) {
         h_view = create_mirror_view(typename t_host::memory_space(), d_view);
       } else {
         h_view = create_mirror_view(Kokkos::WithoutInitializing,
                                     typename t_host::memory_space(), d_view);
       }
-    } else if (alloc_prop_input::initialize) {
+    } else if constexpr (alloc_prop_input::initialize) {
       if constexpr (alloc_prop_input::has_execution_space) {
         const auto& exec_space =
             Impl::get_property<Impl::ExecutionSpaceTag>(arg_prop);
@@ -1038,7 +1038,7 @@ class DualView : public ViewTraits<DataType, Properties...> {
       /* Resize on Device */
       if (sizeMismatch) {
         ::Kokkos::resize(properties, d_view, n0, n1, n2, n3, n4, n5, n6, n7);
-        if (alloc_prop_input::initialize) {
+        if constexpr (alloc_prop_input::initialize) {
           h_view = create_mirror_view(typename t_host::memory_space(), d_view);
         } else {
           h_view = create_mirror_view(Kokkos::WithoutInitializing,
@@ -1054,7 +1054,7 @@ class DualView : public ViewTraits<DataType, Properties...> {
       /* Resize on Host */
       if (sizeMismatch) {
         ::Kokkos::resize(properties, h_view, n0, n1, n2, n3, n4, n5, n6, n7);
-        if (alloc_prop_input::initialize) {
+        if constexpr (alloc_prop_input::initialize) {
           d_view = create_mirror_view(typename t_dev::memory_space(), h_view);
 
         } else {

diff --git a/core/src/Kokkos_CopyViews.hpp b/core/src/Kokkos_CopyViews.hpp
@@ -3235,7 +3235,10 @@ impl_realloc(Kokkos::View<T, P...>& v, const size_t n0, const size_t n1,
     v = view_type();  // Best effort to deallocate in case no other view refers
                       // to the shared allocation
     v = view_type(arg_prop_copy, n0, n1, n2, n3, n4, n5, n6, n7);
-  } else if (alloc_prop_input::initialize) {
+    return;
+  }
+
+  if constexpr (alloc_prop_input::initialize) {
     if constexpr (alloc_prop_input::has_execution_space) {
       const auto& exec_space =
           Impl::get_property<Impl::ExecutionSpaceTag>(arg_prop);
@@ -3330,7 +3333,10 @@ impl_realloc(Kokkos::View<T, P...>& v,
   if (v.layout() != layout) {
     v = view_type();  // Deallocate first, if the only view to allocation
     v = view_type(arg_prop, layout);
-  } else if (alloc_prop_input::initialize) {
+    return;
+  }
+
+  if constexpr (alloc_prop_input::initialize) {
     if constexpr (alloc_prop_input::has_execution_space) {
       const auto& exec_space =
           Impl::get_property<Impl::ExecutionSpaceTag>(arg_prop);

diff --git a/core/unit_test/TestRealloc.hpp b/core/unit_test/TestRealloc.hpp
@@ -144,6 +144,11 @@ void impl_testRealloc() {
     EXPECT_EQ(oldPointer, newPointer);
   }
 }
+struct NoDefaultConstructor {
+  int value;
+  KOKKOS_FUNCTION
+  NoDefaultConstructor(int x) : value(x) {}
+};
 
 template <class DeviceType>
 void testRealloc() {
@@ -154,6 +159,14 @@ void testRealloc() {
     impl_testRealloc<DeviceType,
                      WithoutInitializing>();  // without data initialization
   }
+  // Check #6992 fix (no default initialization in realloc without initializing)
+  {
+    using view_type = Kokkos::View<NoDefaultConstructor*, DeviceType>;
+    view_type view_1d_no_default(
+        Kokkos::view_alloc(Kokkos::WithoutInitializing, "view_1d_no_default"),
+        5);
+    realloc_dispatch(WithoutInitializing{}, view_1d_no_default, 3);
+  }
 }
 
 }  // namespace TestViewRealloc

diff --git a/core/unit_test/TestResize.hpp b/core/unit_test/TestResize.hpp
@@ -358,6 +358,12 @@ void impl_testResize() {
   }
 }
 
+struct NoDefaultConstructor {
+  int value;
+  KOKKOS_FUNCTION
+  NoDefaultConstructor(int x) : value(x) {}
+};
+
 template <class DeviceType>
 void testResize() {
   {
@@ -367,6 +373,13 @@ void testResize() {
     impl_testResize<DeviceType,
                     WithoutInitializing>();  // without data initialization
   }
+  {
+    using view_type = Kokkos::View<NoDefaultConstructor*, DeviceType>;
+    view_type view_1d_no_default(
+        Kokkos::view_alloc(Kokkos::WithoutInitializing, "view_1d_no_default"),
+        5);
+    resize_dispatch(WithoutInitializing{}, view_1d_no_default, 3);
+  }
 }
 
 }  // namespace TestViewResize