Merge pull request kokkos#7385 from crtrott/view-refactor-basicview

View-Refactor: BasicView

core/src/Kokkos_View.hpp

@@ -22,6 +22,10 @@ static_assert(false,
 #ifndef KOKKOS_VIEW_HPP
 #define KOKKOS_VIEW_HPP
 
+#ifdef KOKKOS_ENABLE_IMPL_MDSPAN
+#include <View/Kokkos_BasicView.hpp>
+#endif
+
 #include <View/Kokkos_ViewLegacy.hpp>
 
 #endif /* KOKKOS_VIEW_HPP */

core/src/View/Kokkos_ViewTraits.hpp

@@ -27,6 +27,10 @@ static_assert(false,
 #include <Kokkos_MemoryTraits.hpp>
 #include <Kokkos_ExecPolicy.hpp>
 #include <View/Hooks/Kokkos_ViewHooks.hpp>
+#ifdef KOKKOS_ENABLE_IMPL_MDSPAN
+#include <View/MDSpan/Kokkos_MDSpan_Layout.hpp>
+#include <View/MDSpan/Kokkos_MDSpan_Accessor.hpp>
+#endif
 
 //----------------------------------------------------------------------------
 //----------------------------------------------------------------------------

core/src/View/MDSpan/Kokkos_MDSpan_Accessor.hpp

@@ -388,6 +388,23 @@ class ReferenceCountedAccessor {
   NestedAccessor m_nested_acc;
 };
 
+template <class ElementType, class MemorySpace>
+using CheckedReferenceCountedAccessor =
+    SpaceAwareAccessor<MemorySpace,
+                       ReferenceCountedAccessor<ElementType, MemorySpace,
+                                                default_accessor<ElementType>>>;
+
+template <class ElementType, class MemorySpace,
+          class MemoryScope = desul::MemoryScopeDevice>
+using CheckedRelaxedAtomicAccessor =
+    SpaceAwareAccessor<MemorySpace, AtomicAccessorRelaxed<ElementType>>;
+
+template <class ElementType, class MemorySpace,
+          class MemoryScope = desul::MemoryScopeDevice>
+using CheckedReferenceCountedRelaxedAtomicAccessor = SpaceAwareAccessor<
+    MemorySpace, ReferenceCountedAccessor<ElementType, MemorySpace,
+                                          AtomicAccessorRelaxed<ElementType>>>;
+
 }  // namespace Impl
 }  // namespace Kokkos
 

core/src/impl/Kokkos_SharedAlloc.hpp

@@ -658,12 +658,25 @@ struct SharedAllocationDisableTrackingGuard {
   // clang-format on
 };
 
+// Intel classic compiler screwed up the reference counting here
+// in the BasicView test. Apparently the code simplification in
+// BasicView over View, let it think it can optimize stuff away
+// and it ends up not setting the "do-not-deref" flag
+// because it skips some copy ctors.
+// I tried a lot of stuff to no avail (don't move, make an explicit
+// volatile copy first etc.), only reducing opt-level fixed it.
+#ifdef KOKKOS_COMPILER_INTEL
+#pragma optimize("", off)
+#endif
 template <class FunctorType, class... Args>
 inline FunctorType construct_with_shared_allocation_tracking_disabled(
     Args&&... args) {
   [[maybe_unused]] auto guard = SharedAllocationDisableTrackingGuard{};
   return {std::forward<Args>(args)...};
 }
+#ifdef KOKKOS_COMPILER_INTEL
+#pragma optimize("", on)
+#endif
 } /* namespace Impl */
 } /* namespace Kokkos */
 #endif

core/unit_test/CMakeLists.txt

@@ -92,9 +92,14 @@ set(COMPILE_ONLY_SOURCES
     TestTeamPolicyCTAD.cpp
     TestTeamMDRangePolicyCTAD.cpp
     TestNestedReducerCTAD.cpp
+    view/TestBasicViewMDSpanConversion.cpp
     view/TestExtentsDatatypeConversion.cpp
 )
 
+if(NOT Kokkos_ENABLE_IMPL_MDSPAN)
+  list(REMOVE_ITEM COMPILE_ONLY_SOURCES view/TestBasicViewMDSpanConversion.cpp)
+endif()
+
 #testing if windows.h and Kokkos_Core.hpp can be included
 if(WIN32)
   list(APPEND COMPILE_ONLY_SOURCES TestWindowsInclude.cpp)
@@ -344,7 +349,7 @@ foreach(Tag Threads;Serial;OpenMP;Cuda;HPX;OpenMPTarget;OpenACC;HIP;SYCL)
     # detail tests for the mdspan based View
     set(${Tag}_VIEWSUPPORT)
     if(Kokkos_ENABLE_IMPL_MDSPAN)
-      foreach(Name ReferenceCountedAccessor ReferenceCountedDataHandle)
+      foreach(Name BasicView ReferenceCountedAccessor ReferenceCountedDataHandle)
         set(file ${dir}/Test${Tag}_View_${Name}.cpp)
         # Write to a temporary intermediate file and call configure_file to avoid
         # updating timestamps triggering unnecessary rebuilds on subsequent cmake runs.

core/unit_test/view/TestBasicView.hpp

@@ -0,0 +1,268 @@
+//@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 <gtest/gtest.h>
+
+#include <Kokkos_Core.hpp>
+#include <type_traits>
+
+using ExecutionSpace = TEST_EXECSPACE;
+
+namespace {
+template <class ExecutionSpace, class Extents>
+auto make_spanning_mdrange_policy_from_extents_impl(const Extents &extents,
+                                                    std::index_sequence<0>) {
+  return Kokkos::RangePolicy<ExecutionSpace>{0, extents.extent(0)};
+}
+
+template <class ExecutionSpace, class Extents, std::size_t... Indices>
+auto make_spanning_mdrange_policy_from_extents_impl(
+    const Extents &extents, std::index_sequence<Indices...>) {
+  using index_type    = typename Extents::index_type;
+  constexpr auto rank = Extents::rank();
+  return Kokkos::MDRangePolicy<ExecutionSpace, Kokkos::Rank<rank>>{
+      {(static_cast<index_type>(Indices * 0))...},
+      {extents.extent(Indices)...}};
+}
+
+template <class ExecutionSpace, class Extents>
+auto make_spanning_mdrange_policy_from_extents(const Extents &extents) {
+  return make_spanning_mdrange_policy_from_extents_impl<ExecutionSpace>(
+      extents, std::make_index_sequence<Extents::rank()>{});
+}
+
+template <class T, class ExtentsType>
+void test_default_constructor() {
+  using extents_type  = ExtentsType;
+  using layout_type   = Kokkos::Experimental::layout_right_padded<>;
+  using accessor_type = Kokkos::Impl::CheckedReferenceCountedAccessor<
+      T, typename ExecutionSpace::memory_space>;
+  using view_type =
+      Kokkos::BasicView<T, extents_type, layout_type, accessor_type>;
+  view_type view;
+
+  EXPECT_FALSE(view.data_handle().has_record());
+  EXPECT_EQ(view.data_handle().get(), nullptr);
+  EXPECT_EQ(view.extents(), extents_type{});
+  EXPECT_EQ(view.data_handle().use_count(), 0);
+  EXPECT_TRUE(view.is_exhaustive());
+  EXPECT_EQ(view.data_handle().get_label(), "");
+  EXPECT_TRUE(view.empty());
+  EXPECT_EQ(view.size(), 0u);
+}
+
+TEST(TEST_CATEGORY, basic_view_default_ctor) {
+  test_default_constructor<double, Kokkos::extents<std::size_t, 1>>();
+}
+
+template <class T, class ExtentsType>
+void test_extents_constructor(const ExtentsType &extents) {
+  using extents_type  = ExtentsType;
+  using layout_type   = Kokkos::Experimental::layout_right_padded<>;
+  using accessor_type = Kokkos::Impl::CheckedReferenceCountedAccessor<
+      T, typename ExecutionSpace::memory_space>;
+  using view_type =
+      Kokkos::BasicView<T, extents_type, layout_type, accessor_type>;
+
+  view_type view("test_view", extents);
+
+  EXPECT_TRUE(view.data_handle().has_record());
+  EXPECT_NE(view.data_handle().get(), nullptr);
+  EXPECT_EQ(view.extents(), extents);
+  EXPECT_EQ(view.data_handle().use_count(), 1);
+  EXPECT_TRUE(view.is_exhaustive());
+  EXPECT_EQ(view.data_handle().get_label(), "test_view");
+  size_t expected_size = 1;
+  // Avoid pointless comparison of unsigned warning for rank==0
+  for (int r = 0; r < static_cast<int>(view_type::rank()); r++)
+    expected_size *= extents.extent(r);
+  EXPECT_EQ(view.size(), expected_size);
+  EXPECT_EQ(view.empty(), expected_size == 0u);
+}
+
+TEST(TEST_CATEGORY, basic_view_extents_ctor) {
+  test_extents_constructor<double>(
+      Kokkos::extents<std::size_t, 2, Kokkos::dynamic_extent, 4>(8));
+  test_extents_constructor<double>(
+      Kokkos::extents<std::size_t, 2, Kokkos::dynamic_extent, 4>(0));
+  test_extents_constructor<std::size_t>(Kokkos::extents<std::size_t, 2, 4>());
+  test_extents_constructor<int>(Kokkos::extents<std::size_t>());
+}
+
+template <class T, template <std::size_t> class LayoutType, class ExtentsType>
+void test_mapping_constructor(const ExtentsType &extents, std::size_t padding) {
+  using extents_type  = ExtentsType;
+  using layout_type   = LayoutType<Kokkos::dynamic_extent>;
+  using mapping_type  = typename layout_type::template mapping<ExtentsType>;
+  using accessor_type = Kokkos::Impl::CheckedReferenceCountedAccessor<
+      T, typename ExecutionSpace::memory_space>;
+  using view_type =
+      Kokkos::BasicView<T, extents_type, layout_type, accessor_type>;
+  static_assert(std::is_same_v<typename view_type::mapping_type, mapping_type>);
+
+  auto mapping = mapping_type(extents, padding);
+
+  view_type view("test_view", mapping);
+
+  EXPECT_TRUE(view.data_handle().has_record());
+  EXPECT_NE(view.data_handle().get(), nullptr);
+  EXPECT_EQ(view.data_handle().use_count(), 1);
+  EXPECT_EQ(view.data_handle().get_label(), "test_view");
+  EXPECT_EQ(view.extents(), mapping.extents());
+  EXPECT_EQ(view.is_exhaustive(), mapping.is_exhaustive());
+  size_t expected_size = 1;
+  // Avoid pointless comparison of unsigned warning for rank==0
+  for (int r = 0; r < static_cast<int>(view_type::rank()); r++)
+    expected_size *= view.extent(r);
+  EXPECT_EQ(view.size(), expected_size);
+  EXPECT_EQ(view.empty(), expected_size == 0u);
+}
+
+TEST(TEST_CATEGORY, basic_view_mapping_ctor_right) {
+  test_mapping_constructor<double, Kokkos::Experimental::layout_left_padded>(
+      Kokkos::extents<std::size_t, 2, Kokkos::dynamic_extent>(2, 5), 8);
+  test_mapping_constructor<std::size_t,
+                           Kokkos::Experimental::layout_left_padded>(
+      Kokkos::extents<std::size_t>(), 4);
+  test_mapping_constructor<double, Kokkos::Experimental::layout_left_padded>(
+      Kokkos::extents<std::size_t, 2, 3>(), 9);
+  test_mapping_constructor<int, Kokkos::Experimental::layout_right_padded>(
+      Kokkos::extents<std::size_t, 2, Kokkos::dynamic_extent>(2, 5), 8);
+  test_mapping_constructor<double, Kokkos::Experimental::layout_right_padded>(
+      Kokkos::extents<std::size_t>(), 4);
+  test_mapping_constructor<unsigned, Kokkos::Experimental::layout_right_padded>(
+      Kokkos::extents<std::size_t, 2, 3>(), 9);
+}
+
+// There seems to be some hard to track down issue.
+// See: https://github.com/kokkos/kokkos/pull/7385#issuecomment-2392528089
+#ifndef KOKKOS_ENABLE_OPENACC
+template <class ViewType>
+struct MDRangeTestFunctor {
+  ViewType view;
+  template <class... Idxs>
+  KOKKOS_FUNCTION void operator()(Idxs... idxs) const {
+    view(idxs...) = (idxs + ...);
+  }
+};
+
+template <class T, class LayoutType, class ExtentsType>
+void test_access_with_extents(const ExtentsType &extents) {
+  using extents_type  = ExtentsType;
+  using layout_type   = Kokkos::Experimental::layout_right_padded<>;
+  using accessor_type = Kokkos::Impl::CheckedReferenceCountedAccessor<
+      T, typename ExecutionSpace::memory_space>;
+  using view_type =
+      Kokkos::BasicView<T, extents_type, layout_type, accessor_type>;
+
+  auto view = view_type("test_view", extents);
+
+  EXPECT_TRUE(view.data_handle().has_record());
+  EXPECT_NE(view.data_handle().get(), nullptr);
+
+  auto mdrange_policy =
+      make_spanning_mdrange_policy_from_extents<ExecutionSpace>(extents);
+
+  Kokkos::parallel_for(mdrange_policy, MDRangeTestFunctor<view_type>{view});
+}
+
+template <class T, class LayoutType>
+void test_access() {
+  test_access_with_extents<T, LayoutType>(Kokkos::extents<std::size_t, 5>());
+  test_access_with_extents<T, LayoutType>(
+      Kokkos::extents<std::size_t, 5, 10>());
+  test_access_with_extents<T, LayoutType>(
+      Kokkos::extents<std::size_t, 5, 2, 2, 2, 2, 2>());
+}
+
+TEST(TEST_CATEGORY, basic_view_access) {
+  test_access<double, Kokkos::Experimental::layout_left_padded<
+                          Kokkos::dynamic_extent>>();
+  test_access<std::size_t, Kokkos::Experimental::layout_right_padded<
+                               Kokkos::dynamic_extent>>();
+}
+
+#if 0  // TODO: this test should be active after View is put on top of BasicView
+  template <class T, template <std::size_t> class LayoutType, class SrcViewType,
+            class ExtentsType>
+  void test_construct_from_view(const ExtentsType &extents,
+                                       std::size_t padding) {
+    using extents_type  = ExtentsType;
+    using layout_type   = LayoutType<Kokkos::dynamic_extent>;
+    using mapping_type  = typename layout_type::template mapping<ExtentsType>;
+    using accessor_type = Kokkos::Impl::CheckedReferenceCountedAccessor<
+        T, typename ExecutionSpace::memory_space>;
+    using basic_view_type =
+        Kokkos::BasicView<T, extents_type, layout_type, accessor_type>;
+    using view_type = SrcViewType;
+    static_assert(std::is_constructible_v<basic_view_type, SrcViewType>);
+  }
+#endif
+
+#if 0  // TODO: this test should be active after View is put on top of BasicView
+TEST(TEST_CATEGORY, basic_view_view_ctor) {
+    test_construct_from_view<double,
+        Kokkos::Experimental::layout_left_padded,
+        Kokkos::View<double[3], Kokkos::LayoutLeft, ExecutionSpace>>(
+        Kokkos::extents<std::size_t, 3>(), 0);
+
+    test_construct_from_view<size_t,
+        Kokkos::Experimental::layout_left_padded,
+        Kokkos::View<double[3], Kokkos::LayoutLeft, ExecutionSpace>>(
+        Kokkos::extents<std::size_t, Kokkos::dynamic_extent>(3), 0);
+}
+#endif
+
+template <class T>
+void test_atomic_accessor() {
+  using extents_type = Kokkos::extents<int, 10, 12, 30>;
+  using layout_type  = Kokkos::Experimental::layout_right_padded<>;
+  using accessor_type =
+      Kokkos::Impl::CheckedReferenceCountedRelaxedAtomicAccessor<
+          T, typename ExecutionSpace::memory_space>;
+  using view_type =
+      Kokkos::BasicView<T, extents_type, layout_type, accessor_type>;
+  using um_accessor_type = Kokkos::Impl::CheckedRelaxedAtomicAccessor<
+      T, typename ExecutionSpace::memory_space>;
+  using um_view_type =
+      Kokkos::BasicView<T, extents_type, layout_type, um_accessor_type>;
+
+  extents_type extents{};
+  auto view = view_type("test_view", extents);
+  um_view_type um_view(view);
+
+  EXPECT_TRUE(view.data_handle().has_record());
+  EXPECT_NE(view.data_handle().get(), nullptr);
+
+  auto mdrange_policy =
+      make_spanning_mdrange_policy_from_extents<ExecutionSpace>(extents);
+
+  Kokkos::parallel_for(mdrange_policy, MDRangeTestFunctor<view_type>{view});
+  Kokkos::parallel_for(mdrange_policy,
+                       MDRangeTestFunctor<um_view_type>{um_view});
+}
+
+TEST(TEST_CATEGORY, basic_view_atomic_accessor) {
+  test_atomic_accessor<int>();
+  test_atomic_accessor<double>();
+// FIXME OPENACC atomics
+#ifndef KOKKOS_ENABLE_OPENACC
+  test_atomic_accessor<Kokkos::complex<double>>();
+#endif
+}
+#endif
+
+}  // namespace