Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Add implementation to forward linear iterators in strided views #2797

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Add implementation to forward linear iterators in strided views #2797

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7935a69
Forwarded linear iterator for linearly iterable expressions
Jul 13, 2024
54fe084
Fixed comments
Jul 21, 2024
9268b80
Ran pre-commit
Jul 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions include/xtensor/xstrided_view.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,36 @@ namespace xt

using inner_storage_type = typename base_type::inner_storage_type;
using storage_type = typename base_type::storage_type;
using linear_iterator = typename storage_type::iterator;
using const_linear_iterator = typename storage_type::const_iterator;
using reverse_linear_iterator = std::reverse_iterator<linear_iterator>;
using const_reverse_linear_iterator = std::reverse_iterator<const_linear_iterator>;

template <class C, class = void_t<>>
struct get_linear_iterator : std::false_type
{
using iterator = typename C::iterator;
};

template <typename C>
struct get_linear_iterator<C, void_t<decltype(std::declval<C>().linear_begin())>> : std::true_type
{
using iterator = typename C::linear_iterator;
};

template <class C, class = void_t<>>
struct get_const_linear_iterator : std::false_type
{
using iterator = typename C::const_iterator;
};

template <typename C>
struct get_const_linear_iterator<C, void_t<decltype(std::declval<C>().linear_cbegin())>> : std::true_type
{
using iterator = typename C::const_linear_iterator;
};
spectre-ns marked this conversation as resolved.
Show resolved Hide resolved

using linear_iterator = typename get_linear_iterator<storage_type>::iterator;
using const_linear_iterator = typename get_const_linear_iterator<storage_type>::iterator;
using reverse_linear_iterator = std::reverse_iterator<typename get_linear_iterator<storage_type>::iterator>;
using const_reverse_linear_iterator = std::reverse_iterator<
typename get_const_linear_iterator<storage_type>::iterator>;

using iterable_base = select_iterable_base_t<L, xexpression_type::static_layout, self_type>;
using inner_shape_type = typename base_type::inner_shape_type;
Expand Down Expand Up @@ -222,6 +248,7 @@ namespace xt
const_linear_iterator linear_begin() const;
const_linear_iterator linear_end() const;
const_linear_iterator linear_cbegin() const;

spectre-ns marked this conversation as resolved.
Show resolved Hide resolved
const_linear_iterator linear_cend() const;

reverse_linear_iterator linear_rbegin();
Expand Down Expand Up @@ -511,7 +538,16 @@ namespace xt
template <class CT, class S, layout_type L, class FST>
inline auto xstrided_view<CT, S, L, FST>::linear_cbegin() const -> const_linear_iterator
{
return this->storage().cbegin() + static_cast<std::ptrdiff_t>(data_offset());
return xtl::mpl::static_if<get_const_linear_iterator<storage_type>::value>(
[&](auto self)
{
return self(this->storage()).linear_cbegin() + static_cast<std::ptrdiff_t>(data_offset());
},
[&](auto self)
{
return self(this->storage()).cbegin() + static_cast<std::ptrdiff_t>(data_offset());
}
);
}

template <class CT, class S, layout_type L, class FST>
Expand Down
135 changes: 128 additions & 7 deletions include/xtensor/xstrided_view_base.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ namespace xt
using reverse_iterator = decltype(std::declval<std::remove_reference_t<CT>>().template rbegin<L>());
using const_reverse_iterator = decltype(std::declval<std::decay_t<CT>>().template crbegin<L>());


spectre-ns marked this conversation as resolved.
Show resolved Hide resolved
explicit flat_expression_adaptor(CT* e);

template <class FST>
Expand Down Expand Up @@ -75,6 +76,52 @@ namespace xt
size_type m_size;
};

template <class CT, layout_type L>
class linear_flat_expression_adaptor : public flat_expression_adaptor<CT, L>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why inheriting from flat_expression_adaptor here? It does not seem that you reuse anything from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohanMabille I only implemented the linear iterator methods for the new class. linear_flat_expression_adaptor relies on inheriting the implementation of flat_expression_adaptor for calls made to methods such as cbegin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohanMabille what do you think? Should I implement this differently?

{
public:

using xexpression_type = std::decay_t<CT>;
using shape_type = typename xexpression_type::shape_type;
using inner_strides_type = get_strides_t<shape_type>;
using index_type = inner_strides_type;
using size_type = typename xexpression_type::size_type;
using value_type = typename xexpression_type::value_type;
using const_reference = typename xexpression_type::const_reference;
using reference = std::conditional_t<
std::is_const<std::remove_reference_t<CT>>::value,
typename xexpression_type::const_reference,
typename xexpression_type::reference>;


using linear_iterator = decltype(std::declval<std::remove_reference_t<CT>>().linear_begin());
using const_linear_iterator = decltype(std::declval<std::decay_t<CT>>().linear_cbegin());
using reverse_linear_iterator = decltype(std::declval<std::remove_reference_t<CT>>().linear_rbegin()
);
using const_reverse_linear_iterator = decltype(std::declval<std::decay_t<CT>>().linear_crbegin());


explicit linear_flat_expression_adaptor(CT* e);

template <class FST>
linear_flat_expression_adaptor(CT* e, FST&& strides);

linear_iterator linear_begin();
linear_iterator linear_end();
const_linear_iterator linear_begin() const;
const_linear_iterator linear_end() const;
const_linear_iterator linear_cbegin() const;
const_linear_iterator linear_cend() const;

private:

static index_type& get_index();

mutable CT* m_e;
inner_strides_type m_strides;
size_type m_size;
};

template <class T>
struct is_flat_expression_adaptor : std::false_type
{
Expand All @@ -85,9 +132,21 @@ namespace xt
{
};

template <class T>
struct is_linear_flat_expression_adaptor : std::false_type
{
};

template <class CT, layout_type L>
struct is_linear_flat_expression_adaptor<linear_flat_expression_adaptor<CT, L>> : std::true_type
{
};

template <class E, class ST>
struct provides_data_interface
: xtl::conjunction<has_data_interface<std::decay_t<E>>, xtl::negation<is_flat_expression_adaptor<ST>>>
struct provides_data_interface : xtl::conjunction<
has_data_interface<std::decay_t<E>>,
xtl::negation<is_flat_expression_adaptor<ST>>,
xtl::negation<is_linear_flat_expression_adaptor<ST>>>
{
};
}
Expand Down Expand Up @@ -246,7 +305,11 @@ namespace xt
template <class CT, layout_type L>
struct flat_adaptor_getter
{
using type = flat_expression_adaptor<std::remove_reference_t<CT>, L>;
using type = std::conditional_t<
detail::has_linear_iterator<std::remove_reference_t<CT>>::value
&& (std::remove_reference_t<CT>::static_layout == L),
linear_flat_expression_adaptor<std::remove_reference_t<CT>, L>,
flat_expression_adaptor<std::remove_reference_t<CT>, L>>;
using reference = std::add_lvalue_reference_t<CT>;

template <class E>
Expand Down Expand Up @@ -318,9 +381,7 @@ namespace xt
layout_type layout
) noexcept
: m_e(std::forward<CTA>(e))
,
// m_storage(detail::get_flat_storage<undecay_expression>(m_e)),
m_storage(storage_getter::get_flat_storage(m_e))
, m_storage(storage_getter::get_flat_storage(m_e))
, m_shape(std::forward<SA>(shape))
, m_strides(std::move(strides))
, m_offset(offset)
Expand All @@ -345,6 +406,14 @@ namespace xt
new_storage.update_pointer(std::addressof(expr));
return new_storage;
}

template <class T, class E, layout_type L>
auto copy_move_storage(T& expr, const detail::linear_flat_expression_adaptor<E, L>& storage)
{
detail::linear_flat_expression_adaptor<E, L> new_storage = storage; // copy storage
new_storage.update_pointer(std::addressof(expr));
return new_storage;
}
}

template <class D>
Expand Down Expand Up @@ -652,7 +721,7 @@ namespace xt
template <class O>
inline bool xstrided_view_base<D>::has_linear_assign(const O& str) const noexcept
{
return has_data_interface<xexpression_type>::value && str.size() == strides().size()
return detail::has_linear_iterator<xexpression_type>::value && str.size() == strides().size()
&& std::equal(str.cbegin(), str.cend(), strides().begin());
}

Expand Down Expand Up @@ -783,6 +852,58 @@ namespace xt
thread_local static index_type index;
return index;
}

template <class CT, layout_type L>
inline linear_flat_expression_adaptor<CT, L>::linear_flat_expression_adaptor(CT* e)
: flat_expression_adaptor<CT, L>(e)
, m_e(e)
{
}

template <class CT, layout_type L>
template <class FST>
inline linear_flat_expression_adaptor<CT, L>::linear_flat_expression_adaptor(CT* e, FST&& strides)
: flat_expression_adaptor<CT, L>(e, strides)
, m_e(e)
, m_strides(xtl::forward_sequence<inner_strides_type, FST>(strides))
{
}

template <class CT, layout_type L>
inline auto linear_flat_expression_adaptor<CT, L>::linear_begin() -> linear_iterator
{
return m_e->linear_begin();
}

template <class CT, layout_type L>
inline auto linear_flat_expression_adaptor<CT, L>::linear_end() -> linear_iterator
{
return m_e->linear_end();
}

template <class CT, layout_type L>
inline auto linear_flat_expression_adaptor<CT, L>::linear_begin() const -> const_linear_iterator
{
return m_e->linear_cbegin();
}

template <class CT, layout_type L>
inline auto linear_flat_expression_adaptor<CT, L>::linear_end() const -> const_linear_iterator
{
return m_e->linear_cend();
}

template <class CT, layout_type L>
inline auto linear_flat_expression_adaptor<CT, L>::linear_cbegin() const -> const_linear_iterator
{
return m_e->linear_cbegin();
}

template <class CT, layout_type L>
inline auto linear_flat_expression_adaptor<CT, L>::linear_cend() const -> const_linear_iterator
{
return m_e->linear_cend();
}
}

/**********************************
Expand Down
Loading