Second review, and changes I have done locally: * replaced auto with span<const size_t> when saving __static_exts to avoid making copy of array * in __contains_zero for array keeping static size is not important, so just rating dynamically sized span. I will post update patch on this thread soon.
diff --git a/libstdc++-v3/include/std/mdspan b/libstdc++-v3/include/std/mdspan index 7b73df8550e..08e6b8ff518 100644 --- a/libstdc++-v3/include/std/mdspan +++ b/libstdc++-v3/include/std/mdspan @@ -355,7 +355,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<typename _Tp, size_t _Nm> consteval bool __contains_zero(const array<_Tp, _Nm>& __exts) noexcept - { return __contains_zero(span<const _Tp, _Nm>{__exts}); } + { return __contains_zero(span<const _Tp>(__exts)); } template<typename _Extents> constexpr bool @@ -388,9 +388,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION size_t __ret = 1; if constexpr (_Extents::rank_dynamic() != _Extents::rank()) { - auto __sta_exts = __static_extents<_Extents>(); - __ret = __static_extents_prod(span{__sta_exts.data() + __begin, - __sta_exts.data() + __end}); + std::span<const size_t> __sta_exts = __static_extents<_Extents>(); + __ret = __static_extents_prod(__sta_exts.subspan(__begin, + __end - __begin)); if (__ret == 0) return 0; } @@ -483,7 +483,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION consteval _IndexType __static_quotient(_IndexType __nom = numeric_limits<_IndexType>::max()) { - auto __sta_exts = __static_extents<_Extents>(); + std::span<const size_t> __sta_exts = __static_extents<_Extents>(); for (auto __factor : __sta_exts) { On Sun, Aug 3, 2025 at 11:02 PM Luc Grosheintz <luc.groshei...@gmail.com> wrote: > In mdspan related code involving static extents, often the IndexType is > part of the template parameters, even though it's not needed. > > This commit extracts the parts of _ExtentsStorage not related to > IndexType into a separate class _StaticExtents. > > It also prefers passing the array of static extents, instead of the > whole extents object where possible. > > The size of an object file compiled with -O2 that instantiates > Layout::mapping<extents<IndexType, Indices...>::stride > Layout::mapping<extents<IndexType, Indices...>::required_span_size > for the product of > - eight IndexTypes > - three Layouts, > - nine choices of Indices... > decreases by 19% from 69.2kB to 55.8kB. > > libstdc++-v3/ChangeLog: > > * include/std/mdspan (_StaticExtents): Extract non IndexType > related code from _ExtentsStorage. > (_ExtentsStorage): Use _StaticExtents. > (__static_extents): Return reference to NTTP of _StaticExtents. > (__contains_zero): New overload. > > Signed-off-by: Luc Grosheintz <luc.groshei...@gmail.com> > --- > libstdc++-v3/include/std/mdspan | 54 ++++++++++++++++++++++----------- > 1 file changed, 36 insertions(+), 18 deletions(-) > > diff --git a/libstdc++-v3/include/std/mdspan > b/libstdc++-v3/include/std/mdspan > index 5e79d4bfb59..7b73df8550e 100644 > --- a/libstdc++-v3/include/std/mdspan > +++ b/libstdc++-v3/include/std/mdspan > @@ -49,19 +49,14 @@ namespace std _GLIBCXX_VISIBILITY(default) > _GLIBCXX_BEGIN_NAMESPACE_VERSION > namespace __mdspan > { > - template<typename _IndexType, array _Extents> > - class _ExtentsStorage > + template<array _Extents> > + class _StaticExtents > { > public: > static consteval bool > _S_is_dyn(size_t __ext) noexcept > { return __ext == dynamic_extent; } > > - template<typename _OIndexType> > - static constexpr _IndexType > - _S_int_cast(const _OIndexType& __other) noexcept > - { return _IndexType(__other); } > - > static constexpr size_t _S_rank = _Extents.size(); > > // For __r in [0, _S_rank], _S_dynamic_index[__r] is the number > @@ -99,6 +94,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > static constexpr size_t > _S_static_extent(size_t __r) noexcept > { return _Extents[__r]; } > + }; > + > + template<typename _IndexType, array _Extents> > + class _ExtentsStorage : public _StaticExtents<_Extents> > + { > + private: > + using _S_base = _StaticExtents<_Extents>; > + > + public: > + using _S_base::_S_rank; > + using _S_base::_S_rank_dynamic; > + using _S_base::_S_dynamic_index; > + using _S_base::_S_dynamic_index_inv; > + > + template<typename _OIndexType> > + static constexpr _IndexType > + _S_int_cast(const _OIndexType& __other) noexcept > + { return _IndexType(__other); } > > constexpr _IndexType > _M_extent(size_t __r) const noexcept > @@ -157,9 +170,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > { return __exts[__i]; }); > } > > - static constexpr span<const size_t> > - _S_static_extents(size_t __begin, size_t __end) noexcept > - { return {_Extents.data() + __begin, _Extents.data() + __end}; } > + static constexpr const array<size_t, _S_rank>& > + _S_static_extents() noexcept > + { return _Extents; } > > constexpr span<const _IndexType> > _M_dynamic_extents(size_t __begin, size_t __end) const noexcept > @@ -185,10 +198,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > || _Extent <= numeric_limits<_IndexType>::max(); > > template<typename _Extents> > - constexpr span<const size_t> > - __static_extents(size_t __begin = 0, size_t __end = > _Extents::rank()) > - noexcept > - { return _Extents::_S_storage::_S_static_extents(__begin, __end); } > + constexpr const array<size_t, _Extents::rank()>& > + __static_extents() noexcept > + { return _Extents::_S_storage::_S_static_extents(); } > > template<typename _Extents> > constexpr span<const typename _Extents::index_type> > @@ -314,8 +326,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > } > > private: > - friend span<const size_t> > - __mdspan::__static_extents<extents>(size_t, size_t); > + friend const array<size_t, rank()>& > + __mdspan::__static_extents<extents>(); > > friend span<const index_type> > __mdspan::__dynamic_extents<extents>(const extents&, size_t, > size_t); > @@ -340,6 +352,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > return false; > } > > + template<typename _Tp, size_t _Nm> > + consteval bool > + __contains_zero(const array<_Tp, _Nm>& __exts) noexcept > + { return __contains_zero(span<const _Tp, _Nm>{__exts}); } > + > template<typename _Extents> > constexpr bool > __empty(const _Extents& __exts) noexcept > @@ -371,8 +388,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > size_t __ret = 1; > if constexpr (_Extents::rank_dynamic() != _Extents::rank()) > { > - auto __sta_exts = __static_extents<_Extents>(__begin, __end); > - __ret = __static_extents_prod(__sta_exts); > + auto __sta_exts = __static_extents<_Extents>(); > + __ret = __static_extents_prod(span{__sta_exts.data() + __begin, > + __sta_exts.data() + __end}); > if (__ret == 0) > return 0; > } > -- > 2.50.0 > >