[PATCH v3 8/9] libstdc++: Implement layout_stride from mdspan.
Luc Grosheintz
luc.grosheintz@gmail.com
Thu May 22 20:20:09 GMT 2025
On 5/22/25 14:37, Tomasz Kaminski wrote:
> On Wed, May 21, 2025 at 12:04 PM Luc Grosheintz <luc.grosheintz@gmail.com>
> wrote:
>
>> Implements the remaining parts of layout_left and layout_right; and all
>> of layout_stride.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/std/mdspan(layout_stride): New class.
>>
>> Signed-off-by: Luc Grosheintz <luc.grosheintz@gmail.com>
>> ---
>> libstdc++-v3/include/std/mdspan | 216 +++++++++++++++++++++++++++++++-
>> 1 file changed, 213 insertions(+), 3 deletions(-)
>>
>> diff --git a/libstdc++-v3/include/std/mdspan
>> b/libstdc++-v3/include/std/mdspan
>> index 43676c3463c..732fc4eb1c2 100644
>> --- a/libstdc++-v3/include/std/mdspan
>> +++ b/libstdc++-v3/include/std/mdspan
>> @@ -399,6 +399,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> class mapping;
>> };
>>
>> + struct layout_stride
>> + {
>> + template<typename _Extents>
>> + class mapping;
>> + };
>> +
>> namespace __mdspan
>> {
>> template<typename _Tp>
>> @@ -499,7 +505,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>
>> template<typename _Mapping>
>> concept __standardized_mapping = __mapping_of<layout_left, _Mapping>
>> - || __mapping_of<layout_right,
>> _Mapping>;
>> + || __mapping_of<layout_right,
>> _Mapping>
>> + || __mapping_of<layout_stride,
>> _Mapping>;
>>
>> template<typename M>
>> concept __mapping_like = requires
>> @@ -557,6 +564,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> { }
>>
>> + template<typename _OExtents>
>> + requires (is_constructible_v<extents_type, _OExtents>)
>> + constexpr explicit(extents_type::rank() > 0)
>> + mapping(const layout_stride::mapping<_OExtents>& __other)
>>
> I think I would make it noexcept, as implementations can add noexcept to
> what is specified in standard.
> And just add appropriate comment.
Alright, I prepared this as a separate commit. I don't remember coming
to a conclusion, when it was discussed in the the email chain about
potential bugs in the standard. IIUC Jonathan thought the missing noexcept
was both preferable and an intentional part of the standard.
We should probably make sure he's aware we made this choice (the separate
commit should help).
>
>> + : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> + { __glibcxx_assert(*this == __other); }
>> +
>> constexpr mapping&
>> operator=(const mapping&) noexcept = default;
>>
>> @@ -572,8 +586,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> constexpr index_type
>> operator()(_Indices... __indices) const noexcept
>> {
>> - return __mdspan::__linear_index_left(
>> - this->extents(), static_cast<index_type>(__indices)...);
>> + return __mdspan::__linear_index_left(_M_extents,
>> + static_cast<index_type>(__indices)...);
>>
> Could you move this change to layout_left commit.
>
>> }
>>
>> static constexpr bool
>> @@ -687,6 +701,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> { }
>>
>> + template<class _OExtents>
>> + requires (is_constructible_v<extents_type, _OExtents>)
>> + constexpr explicit(extents_type::rank() > 0)
>> + mapping(const layout_stride::mapping<_OExtents>& __other) noexcept
>> + : mapping(__other.extents(), __mdspan::__internal_ctor{})
>> + { __glibcxx_assert(*this == __other); }
>> +
>> constexpr mapping&
>> operator=(const mapping&) noexcept = default;
>>
>> @@ -760,6 +781,195 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> [[no_unique_address]] _Extents _M_extents;
>> };
>>
>> + namespace __mdspan
>> + {
>> + template<typename _Mapping>
>> + constexpr typename _Mapping::index_type
>> + __offset(const _Mapping& __m) noexcept
>> + {
>> + using _IndexType = typename _Mapping::index_type;
>> +
>> + auto __impl = [&__m]<size_t... _Counts>(index_sequence<_Counts...>)
>> + { return __m(((void) _Counts, _IndexType(0))...); };
>> + return
>> __impl(make_index_sequence<_Mapping::extents_type::rank()>());
>> + }
>> +
>> + template<typename _Mapping, typename... _Indices>
>> + constexpr typename _Mapping::index_type
>> + __linear_index_strides(const _Mapping& __m,
>> + _Indices... __indices)
>> + {
>> + using _IndexType = typename _Mapping::index_type;
>> + _IndexType __res = 0;
>> + if constexpr (sizeof...(__indices) > 0)
>> + {
>> + auto __update = [&, __pos = 0u](_IndexType __idx) mutable
>> + {
>> + __res += __idx * __m.stride(__pos++);
>> + };
>> + (__update(__indices), ...);
>> + }
>> + return __res;
>> + }
>> + }
>> +
>> + template<typename _Extents>
>> + class layout_stride::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_stride;
>> +
>> + static_assert(__mdspan::__representable_size<_Extents, index_type>,
>> + "The size of extents_type must be representable as index_type");
>> +
>> + constexpr
>> + mapping() noexcept
>>
> + {
>> + auto __stride = index_type(1);
>> + for (size_t __i = extents_type::rank(); __i > 0; --__i)
>> + {
>> + _M_strides[__i - 1] = __stride;
>> + __stride *= _M_extents.extent(__i - 1);
>> + }
>> + }
>> +
>> + constexpr
>> + mapping(const mapping&) noexcept = default;
>> +
>> + template<__mdspan::__valid_index_type<index_type> _OIndexType>
>> + constexpr
>> + mapping(const extents_type& __exts,
>> + span<_OIndexType, extents_type::rank()> __strides) noexcept
>> + : _M_extents(__exts)
>> + {
>> + for (size_t __i = 0; __i < extents_type::rank(); ++__i)
>> + _M_strides[__i] = index_type(as_const(__strides[__i]));
>> + }
>> +
>> + template<__mdspan::__valid_index_type<index_type> _OIndexType>
>> + constexpr
>> + mapping(const extents_type& __exts,
>> + const array<_OIndexType, extents_type::rank()>& __strides)
>> + noexcept
>> + : mapping(__exts,
>> + span<const _OIndexType, extents_type::rank()>(__strides))
>> + { }
>> +
>> + template<__mdspan::__mapping_like _StridedMapping>
>> + requires (is_constructible_v<extents_type,
>> + typename
>> _StridedMapping::extents_type>
>> + && _StridedMapping::is_always_unique()
>> + && _StridedMapping::is_always_strided())
>> + constexpr explicit(!(
>> + is_convertible_v<typename _StridedMapping::extents_type,
>> extents_type>
>> + && __mdspan::__standardized_mapping<_StridedMapping>))
>> + mapping(const _StridedMapping& __other) noexcept
>> + : _M_extents(extents_type(__other.extents()))
>>
> I think this would also work: _M_extents(__other.extents())
>
>> + {
>>
> We could guard runtime check, with compile time for size)
> if constexpr(std::numeric_limits<typename
> _StridedMapping::index_type>::max()
> >
>
>
>> + if constexpr (extents_type::rank() > 0)
>> + for (size_t __i = 0; __i < extents_type::rank(); ++__i)
>> + _M_strides[__i] = index_type(__other.stride(__i));
>>
> I would add assert if other.required_span_size() <= numeric_limits::max.
>
> + }
>> +
>> + constexpr mapping&
>> + operator=(const mapping&) noexcept = default;
>> +
>> + constexpr const _Extents&
>> + extents() const noexcept { return _M_extents; }
>> +
>> + constexpr array<index_type, extents_type::rank()>
>> + strides() const noexcept {
>> + array<index_type, extents_type::rank()> __ret;
>> + for (size_t __i = 0; __i < extents_type::rank(); ++__i)
>> + __ret[__i] = _M_strides[__i];
>> + return __ret;
>> + }
>> +
>> + constexpr index_type
>> + required_span_size() const noexcept
>> + {
>> + index_type __ret = 1;
>> + for (size_t __i = 0; __i < extents_type::rank(); ++__i)
>> + {
>> + index_type __ext = _M_extents.extent(__i);
>> + if (__ext == 0)
>> + return 0;
>> + __ret += (__ext - 1) * _M_strides[__i];
>>
> If the index type is sized, this can overflow and cause UB, before we find
> zero.
> We need to check first if any of the extents is zero, before attempting
> multiplication.
>
> + }
>> + return __ret;
>> + }
>> +
>> + 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_strides(*this,
>> + static_cast<index_type>(__indices)...);
>> + }
>> +
>> + static constexpr bool
>> + is_always_unique() noexcept { return true; }
>> +
>> + static constexpr bool
>> + is_always_exhaustive() noexcept { return false; }
>> +
>> + static constexpr bool
>> + is_always_strided() noexcept { return true; }
>> +
>> + static constexpr bool
>> + is_unique() noexcept { return true; }
>> +
>> + constexpr bool
>> + is_exhaustive() const noexcept
>> + {
>> + if constexpr (extents_type::rank() == 0)
>> + return true;
>> + else
>> + {
>>
> The brace in else should be indented.
>
>> + // Under the assumption that the mapping is unique and has
>> positive
>> + // size, the mapping is exhaustive, if and only if the largest
>> value
>> + // returned by m(i...) is within the required range.
>> + // However, the standard requires implementing a condition
>> that's not
>> + // always true when the size of the mapping is zero.
>>
> Could re rephrase this:
> // We deviate from the standard, that does not thread all zero
> extents as exhaustive.
>
>> + auto __size = __mdspan::__fwd_prod(_M_extents,
>> extents_type::rank());
>> + if (__size == 0)
>> + return true;
>> + return __size == required_span_size();
>> + }
>> + }
>> +
>> + static constexpr bool
>> + is_strided() noexcept { return true; }
>> +
>> + constexpr index_type
>> + stride(rank_type __r) const noexcept { return _M_strides[__r]; }
>> +
>> + template<__mdspan::__mapping_like _OMapping>
>> + requires ((extents_type::rank() == _OMapping::extents_type::rank())
>> + && _OMapping::is_always_strided())
>> + friend constexpr bool
>> + operator==(const mapping& __self, const _OMapping& __other)
>> noexcept
>> + {
>> + if (__self.extents() != __other.extents())
>> + return false;
>> + if constexpr (extents_type::rank() > 0)
>> + for (size_t __i = 0; __i < extents_type::rank(); ++__i)
>> + if (__self.stride(__i) != __other.stride(__i))
>> + return false;
>> + return __mdspan::__offset(__other) == 0;
>> + }
>> +
>> + private:
>> + using _S_strides_t = typename __array_traits<index_type,
>> +
>> extents_type::rank()>::_Type;
>> + [[no_unique_address]] _Extents _M_extents;
>> + [[no_unique_address]] _S_strides_t _M_strides;
>> + };
>> +
>> _GLIBCXX_END_NAMESPACE_VERSION
>> }
>> #endif
>> --
>> 2.49.0
>>
>>
>
More information about the Libstdc++
mailing list