Skip to content

Commit

Permalink
Fix global fence in Kokkos::resize(DynRankView) (kokkos#6184)
Browse files Browse the repository at this point in the history
* Fix global fence in Kokkos::resize(DynRankView)

* Add test case

* Comment on the intent of the test and guard for existence of SharedSpace

* Guard execution of test differently

* Loop in reverse order on the host to increase chances for detecting a missing fence

* Test with 1k elements, improve name of the test
  • Loading branch information
masterleinad committed Jun 8, 2023
1 parent 8661773 commit a406372
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
5 changes: 3 additions & 2 deletions containers/src/Kokkos_DynRankView.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2302,9 +2302,10 @@ inline void impl_resize(const Impl::ViewCtorProp<ViewCtorArgs...>& arg_prop,
if constexpr (alloc_prop_input::has_execution_space)
Kokkos::Impl::DynRankViewRemap<drview_type, drview_type>(
Impl::get_property<Impl::ExecutionSpaceTag>(prop_copy), v_resized, v);
else
else {
Kokkos::Impl::DynRankViewRemap<drview_type, drview_type>(v_resized, v);

Kokkos::fence("Kokkos::resize(DynRankView)");
}
v = v_resized;
}

Expand Down
52 changes: 52 additions & 0 deletions containers/unit_tests/TestDynViewAPI_rank12345.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,56 @@ namespace Test {
TEST(TEST_CATEGORY, dyn_rank_view_api_operator_rank12345) {
TestDynViewAPI<double, TEST_EXECSPACE>::run_operator_test_rank12345();
}

template <typename SharedMemorySpace>
void test_dyn_rank_view_resize() {
int n = 1000;
Kokkos::DynRankView<double, SharedMemorySpace> device_view("device view", n);
// Make sure we don't deallocate memory in Kokkos::resize
auto device_view_copy = device_view;

Kokkos::resize(device_view, 2 * n);

// Loop in reverse to increase likelihood of missing fence detection assuming
// that resize copies values in order.
for (int i = 2 * n - 1; i >= 0; --i) device_view(i) = i + 1;

Kokkos::fence();

// Check that Kokkos::resize completed before setting the values on the host
// manually (possibly because of missing fences).
for (int i = 0; i < 2 * n; ++i) ASSERT_EQ(device_view(i), i + 1);
}

template <typename SharedMemorySpace>
void test_dyn_rank_view_realloc() {
int n = 1000;
Kokkos::DynRankView<double, SharedMemorySpace> device_view("device view", n);
// Make sure we don't deallocate memory in Kokkos::realloc
auto device_view_copy = device_view;

Kokkos::realloc(device_view, 2 * n);

// Loop in reverse to increase likelihood of missing fence detection assuming
// that realloc sets values in order.
for (int i = 2 * n - 1; i >= 0; --i) device_view(i) = i + 1;

Kokkos::fence();

// Check that Kokkos::realloc completed before setting the values on the host
// manually (possibly because of missing fences).
for (int i = 0; i < 2 * n; ++i) ASSERT_EQ(device_view(i), i + 1);
}

#ifdef KOKKOS_HAS_SHARED_SPACE
TEST(TEST_CATEGORY, dyn_rank_view_check_fence_resize_realloc) {
if constexpr (std::is_same_v<TEST_EXECSPACE, Kokkos::DefaultExecutionSpace>) {
test_dyn_rank_view_resize<Kokkos::SharedSpace>();
test_dyn_rank_view_realloc<Kokkos::SharedSpace>();
} else {
GTEST_SKIP() << "skipping since not default execution space";
}
}
#endif

} // namespace Test

0 comments on commit a406372

Please sign in to comment.