[PATCH v4 4/8] libstdc++: Implement layout_right from mdspan.
Luc Grosheintz
luc.grosheintz@gmail.com
Thu May 29 10:18:53 GMT 2025
On 5/28/25 16:22, Tomasz Kaminski wrote:
> On Mon, May 26, 2025 at 4:15 PM Luc Grosheintz <luc.grosheintz@gmail.com>
> wrote:
>
>> Implement the parts of layout_left that depend on layout_right; and the
>> parts of layout_right that don't depend on layout_stride.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/std/mdspan (layout_right): New class.
>> * src/c++23/std.cc.in: Add layout_right.
>>
>> Signed-off-by: Luc Grosheintz <luc.grosheintz@gmail.com>
>>
> LGTM. Only some very subjective comments regarding parenthesis.
> Also added some comments for possible future improvements for extents
> converting constructor.
>
>> ---
>> libstdc++-v3/include/std/mdspan | 153 ++++++++++++++++++++++++++++++-
>> libstdc++-v3/src/c++23/std.cc.in | 1 +
>> 2 files changed, 153 insertions(+), 1 deletion(-)
>>
>> diff --git a/libstdc++-v3/include/std/mdspan
>> b/libstdc++-v3/include/std/mdspan
>> index d81072596b4..7daa0713716 100644
>> --- a/libstdc++-v3/include/std/mdspan
>> +++ b/libstdc++-v3/include/std/mdspan
>> @@ -397,6 +397,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> class mapping;
>> };
>>
>> + struct layout_right
>> + {
>> + template<typename _Extents>
>> + class mapping;
>> + };
>> +
>> namespace __mdspan
>> {
>> template<typename _Tp>
>> @@ -489,7 +495,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> _Mapping>;
>>
>> template<typename _Mapping>
>> - concept __standardized_mapping = __mapping_of<layout_left,
>> _Mapping>;
>> + concept __standardized_mapping = __mapping_of<layout_left, _Mapping>
>> + || __mapping_of<layout_right,
>> _Mapping>;
>>
>> template<typename M>
>> concept __mapping_like = requires
>> @@ -539,6 +546,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> { }
>>
>> + template<typename _OExtents>
>> + requires (_Extents::rank() <= 1
>> + && is_constructible_v<_Extents, _OExtents>)
>>
> I got confused for a moment by parametrization here. My preference would be
> to use (_Extents::rank() <= 1) && is_constructible_v<_Extents, _OExtents>?
>
>> + constexpr explicit(!is_convertible_v<_OExtents, _Extents>)
>> + mapping(const layout_right::mapping<_OExtents>& __other) noexcept
>> + : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> + { }
>> +
>> constexpr mapping&
>> operator=(const mapping&) noexcept = default;
>>
>> @@ -606,6 +621,142 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> [[no_unique_address]] _Extents _M_extents{};
>> };
>>
>> + namespace __mdspan
>> + {
>> + template<typename _Extents, typename... _Indices>
>> + constexpr typename _Extents::index_type
>> + __linear_index_right(const _Extents& __exts, _Indices... __indices)
>> + {
>> + using _IndexType = typename _Extents::index_type;
>> + array<_IndexType, sizeof...(__indices)> __ind_arr{__indices...};
>> + _IndexType __res = 0;
>> + if constexpr (sizeof...(__indices) > 0)
>> + {
>> + _IndexType __mult = 1;
>> + auto __update = [&, __pos = __exts.rank()](_IndexType) mutable
>> + {
>> + --__pos;
>> + __res += __ind_arr[__pos] * __mult;
>> + __mult *= __exts.extent(__pos);
>> + };
>> + (__update(__indices), ...);
>> + }
>> + return __res;
>> + }
>> + }
>> +
>> + template<typename _Extents>
>> + class layout_right::mapping
>> + {
>> + public:
>> + using extents_type = _Extents;
>> + using index_type = typename extents_type::index_type;
>> + using size_type = typename extents_type::size_type;
>> + using rank_type = typename extents_type::rank_type;
>> + using layout_type = layout_right;
>> +
>> + static_assert(__mdspan::__representable_size<_Extents, index_type>,
>> + "The size of extents_type must be representable as index_type");
>> +
>> + constexpr
>> + mapping() noexcept = default;
>> +
>> + constexpr
>> + mapping(const mapping&) noexcept = default;
>> +
>> + constexpr
>> + mapping(const _Extents& __extents) noexcept
>> + : _M_extents(__extents)
>> + {
>> __glibcxx_assert(__mdspan::__is_representable_extents(_M_extents)); }
>> +
>> + template<typename _OExtents>
>> + requires (is_constructible_v<extents_type, _OExtents>)
>>
> I do not think this parense are necessary? Are they?
No, it seems not. I think I got confused by the need of putting
around function calls, e.g.
requires (is_foo())
requires the extra set of parens. I think that's why I started
putting them unconditionally.
I stripped all the unneeded parens I could find.
>
>> + constexpr explicit(!is_convertible_v<_OExtents, extents_type>)
>> + mapping(const mapping<_OExtents>& __other) noexcept
>> + : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> + { }
>> +
>> + template<class _OExtents>
>> + requires (extents_type::rank() <= 1
>> + && is_constructible_v<extents_type, _OExtents>)
>>
> Same comment regarding parametrization.
>
>> + constexpr explicit(!is_convertible_v<_OExtents, extents_type>)
>> + mapping(const layout_left::mapping<_OExtents>& __other) noexcept
>> + : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> + { }
>> +
>> + constexpr mapping&
>> + operator=(const mapping&) noexcept = default;
>> +
>> + constexpr const _Extents&
>> + extents() const noexcept { return _M_extents; }
>> +
>> + constexpr index_type
>> + required_span_size() const noexcept
>> + { return __mdspan::__fwd_prod(_M_extents, extents_type::rank()); }
>> +
>> + template<__mdspan::__valid_index_type<index_type>... _Indices>
>> + requires (sizeof...(_Indices) == extents_type::rank())
>> + constexpr index_type
>> + operator()(_Indices... __indices) const noexcept
>> + {
>> + return __mdspan::__linear_index_right(
>> + _M_extents, static_cast<index_type>(__indices)...);
>> + }
>> +
>> + static constexpr bool
>> + is_always_unique() noexcept
>> + { return true; }
>> +
>> + static constexpr bool
>> + is_always_exhaustive() noexcept
>> + { return true; }
>> +
>> + static constexpr bool
>> + is_always_strided() noexcept
>> + { return true; }
>> +
>> + static constexpr bool
>> + is_unique() noexcept
>> + { return true; }
>> +
>> + static constexpr bool
>> + is_exhaustive() noexcept
>> + { return true; }
>> +
>> + static constexpr bool
>> + is_strided() noexcept
>> + { return true; }
>> +
>> + constexpr index_type
>> + stride(rank_type __i) const noexcept
>> + requires (extents_type::rank() > 0)
>> + {
>> + __glibcxx_assert(__i < extents_type::rank());
>> + return __mdspan::__rev_prod(_M_extents, __i);
>> + }
>> +
>> + template<typename _OExtents>
>> + requires (extents_type::rank() == _OExtents::rank())
>> + friend constexpr bool
>> + operator==(const mapping& __self, const mapping<_OExtents>&
>> __other)
>> + noexcept
>> + { return __self.extents() == __other.extents(); }
>> +
>> + private:
>> + template<typename _OExtents>
>> + constexpr explicit
>> + mapping(const _OExtents& __oexts, __mdspan::__internal_ctor)
>> noexcept
>> + : _M_extents(__oexts)
>>
> Comment for future, this is material for commits outside this series.
> The check that static_extents are representable as index type is part of
> the precondition
> on the _S_int_cast. For static_extent in source extent we could check that
> statically in
> constructor:
> template<typename _OIndexType, size_t... _OExtents>
> requires (_S_is_compatible_extents<_OExtents...>())
> constexpr explicit(_S_ctor_explicit<_OIndexType, _OExtents...>())
> extents(const extents<_OIndexType, _OExtents...>& __other) noexcept
> : _M_exts(__other._M_exts)
> {
> static_assert(
> (__mdspan::__valid_static_extent<_OExtents, _IndexType> && ...),
> "Extents must either be dynamic or representable as IndexType");
>
> }
>
>
> + {
>> + static_assert(__mdspan::__representable_size<_OExtents,
>> index_type>,
>>
> + "The size of OtherExtents must be representable as
>> index_type");
>>
> +
>> __glibcxx_assert(__mdspan::__is_representable_extents(_M_extents));
>> + }
>> +
>> + public:
>> + [[no_unique_address]] _Extents _M_extents{};
>> + };
>> +
>> _GLIBCXX_END_NAMESPACE_VERSION
>> }
>> #endif
>> diff --git a/libstdc++-v3/src/c++23/std.cc.in b/libstdc++-v3/src/c++23/
>> std.cc.in
>> index b4b0bb24a8c..e9b5709203c 100644
>> --- a/libstdc++-v3/src/c++23/std.cc.in
>> +++ b/libstdc++-v3/src/c++23/std.cc.in
>> @@ -1842,6 +1842,7 @@ export namespace std
>> {
>> using std::extents;
>> using std::layout_left;
>> + using std::layout_right;
>> // FIXME layout_*, default_accessor and mdspan
>> }
>> #endif
>> --
>> 2.49.0
>>
>>
>
More information about the Libstdc++
mailing list