Skip to content

Commit

Permalink
Adding converting constructor in Kokkos::RandomAccessIterator (kokkos…
Browse files Browse the repository at this point in the history
…#6929)

* Adding converting constructor in Kokkos::RandomAccessIterator

* fix constructible tests for Kokkos::RandomAccessIterator

* fix converting constructor in Kokkos::RandomAccessIterator

* Add comments to explain friend class of RandomAccessIterator is needed for converting constructor

* Introduce KOKKOS_IMPL_CONDITIONAL_EXPLICIT macro from kokkos#6830

* Adding a conditional explicit in converting constructor of RandomAccessIterator

* Rename ViewType to OtherViewType in converting constructor for readability

* Replace tests with static_assert if they rely on compile time behaviour only

* fix a condition for conditional explicit

* Revert "Introduce KOKKOS_IMPL_CONDITIONAL_EXPLICIT macro from kokkos#6830"

This reverts commit ee42c6d.

* On second thought `KOKKOS_IMPL_CONDITIONAL_EXPLICIT` is not such a good idea

because it let user write code that would compile with C++17 but not
with later standards.

---------

Co-authored-by: Yuuichi Asahi <yuuichi.asahi@cea.fr>
  • Loading branch information
yasahi-hpc and Yuuichi Asahi committed Apr 16, 2024
1 parent 8c7cc95 commit a8115e5
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 0 deletions.
28 changes: 28 additions & 0 deletions algorithms/src/std_algorithms/impl/Kokkos_RandomAccessIterator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,30 @@ class RandomAccessIterator< ::Kokkos::View<DataType, Args...> > {
ptrdiff_t current_index)
: m_view(view), m_current_index(current_index) {}

#ifndef KOKKOS_ENABLE_CXX17 // C++20 and beyond
template <class OtherViewType>
requires(std::is_constructible_v<view_type, OtherViewType>) KOKKOS_FUNCTION
explicit(!std::is_convertible_v<OtherViewType, view_type>)
RandomAccessIterator(const RandomAccessIterator<OtherViewType>& other)
: m_view(other.m_view), m_current_index(other.m_current_index) {}
#else
template <
class OtherViewType,
std::enable_if_t<std::is_constructible_v<view_type, OtherViewType> &&
!std::is_convertible_v<OtherViewType, view_type>,
int> = 0>
KOKKOS_FUNCTION explicit RandomAccessIterator(
const RandomAccessIterator<OtherViewType>& other)
: m_view(other.m_view), m_current_index(other.m_current_index) {}

template <class OtherViewType,
std::enable_if_t<std::is_convertible_v<OtherViewType, view_type>,
int> = 0>
KOKKOS_FUNCTION RandomAccessIterator(
const RandomAccessIterator<OtherViewType>& other)
: m_view(other.m_view), m_current_index(other.m_current_index) {}
#endif

KOKKOS_FUNCTION
iterator_type& operator++() {
++m_current_index;
Expand Down Expand Up @@ -155,6 +179,10 @@ class RandomAccessIterator< ::Kokkos::View<DataType, Args...> > {
private:
view_type m_view;
ptrdiff_t m_current_index = 0;

// Needed for the converting constructor accepting another iterator
template <class>
friend class RandomAccessIterator;
};

} // namespace Impl
Expand Down
38 changes: 38 additions & 0 deletions algorithms/unit_tests/TestRandomAccessIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,44 @@ TEST_F(random_access_iterator_test, constructor) {
EXPECT_TRUE(true);
}

TEST_F(random_access_iterator_test, constructiblity) {
auto first_d = KE::begin(m_dynamic_view);
auto cfirst_d = KE::cbegin(m_dynamic_view);

static_assert(std::is_constructible_v<decltype(cfirst_d), decltype(first_d)>);
static_assert(
!std::is_constructible_v<decltype(first_d), decltype(cfirst_d)>);
[[maybe_unused]] decltype(cfirst_d) tmp_cfirst_d(first_d);

auto first_s = KE::begin(m_static_view);
auto cfirst_s = KE::cbegin(m_static_view);

static_assert(std::is_constructible_v<decltype(cfirst_s), decltype(first_s)>);
static_assert(
!std::is_constructible_v<decltype(first_s), decltype(cfirst_s)>);
[[maybe_unused]] decltype(cfirst_s) tmp_cfirst_s(first_s);

auto first_st = KE::begin(m_strided_view);
auto cfirst_st = KE::cbegin(m_strided_view);

static_assert(
std::is_constructible_v<decltype(cfirst_st), decltype(first_st)>);
static_assert(
!std::is_constructible_v<decltype(first_st), decltype(cfirst_st)>);
[[maybe_unused]] decltype(cfirst_st) tmp_cfirst_st(first_st);

// [FIXME] Better to have tests for the explicit specifier with an expression.
// As soon as View converting constructors are re-implemented with a
// conditional explicit, we may add those tests.
static_assert(std::is_constructible_v<decltype(first_s), decltype(first_d)>);
static_assert(std::is_constructible_v<decltype(first_st), decltype(first_d)>);
static_assert(std::is_constructible_v<decltype(first_d), decltype(first_s)>);
static_assert(std::is_constructible_v<decltype(first_st), decltype(first_s)>);
static_assert(std::is_constructible_v<decltype(first_d), decltype(first_st)>);
static_assert(std::is_constructible_v<decltype(first_s), decltype(first_st)>);
EXPECT_TRUE(true);
}

template <class IteratorType, class ValueType>
void test_random_access_it_verify(IteratorType it, ValueType gold_value) {
using view_t = Kokkos::View<typename IteratorType::value_type>;
Expand Down

0 comments on commit a8115e5

Please sign in to comment.