On Wed, 30 Apr 2025 at 09:55, Luc Grosheintz <luc.groshei...@gmail.com> wrote:
> On 4/30/25 4:37 AM, Tomasz Kaminski wrote:
> > On Tue, Apr 29, 2025 at 11:52 PM Jonathan Wakely <jwak...@redhat.com> wrote:
> >> On Tue, 29 Apr 2025 at 14:55, Tomasz Kaminski <tkami...@redhat.com> wrote:
> >>> On Tue, Apr 29, 2025 at 2:55 PM Luc Grosheintz <luc.groshei...@gmail.com>
> >> wrote:
> >>>> + template<typename _OIndexType, size_t... _OExtents>
> >>>> + friend constexpr bool
> >>>> + operator==(const extents& __self,
> >>>> + const extents<_OIndexType, _OExtents...>& __other)
> >> noexcept
> >>>> + {
> >>>> + if constexpr (!_S_is_compatible_extents<_OExtents...>())
> >>>> + return false;
> >>>
> >>> We can add here:
> >>> // N.B. if extents are compatible and static, it implies
> >> that they are equal
> >>> else if constexpr ((_Extents != dynamic_extent) && ...)
> >>> return true;
> >>
> >> Can't we have a case where _OExtents==dynamic_extent and
> >> _Extents!=dynamic_extents, which would mean they're compatible, but
> >> not equal?
> >>
> > Ah, indeed the conditions needs to be:
> > (_Extents != dynamic_extent && _Extents == _OExtents) && ...
> >
>
> The reason I didn't follow through with this is because when looking
> at the generated code I saw no advantage of the additional constexpr
> branch. Therefore, I preferred the simplicity of Tomasz first proposal,
> i.e. the current implementation.
Thanks for checking it. So the runtime 'for' loop is able to optimize
to a constant, because the .extent(i) calls are constants in the cases
that the constexpr-if branch would handle.
> Would you still like me to add the additional `else if constexpr`
> branch?
No, we can always add it later if it turns out to help (e.g. when
sizeof...(_Extents) is large, if the optimizer gives up on the 'for'
loop).
> Here's the output of objdump when compiling with `-O2`:
>
> bool same1(const std::extents<int, 1, 2, 3>& e1,
> const std::extents<int, 1, 2, 3>& e2)
> { return e1 == e2; }
> 0: b8 01 00 00 00 mov $0x1,%eax
> 5: c3 ret
>
> bool same2(const std::extents<int, 2, 2, 3>& e1,
> const std::extents<int, 1, 2, 3>& e2)
> { return e1 == e2; }
> 0: 31 c0 xor %eax,%eax
> 2: c3 ret
>
> bool same3(const std::extents<int, 2, dyn, 3>& e1,
> const std::extents<int, 1, 2, 3>& e2)
> { return e1 == e2; }
> 0: 31 c0 xor %eax,%eax
> 2: c3 ret
>
> bool same4(const std::extents<int, 2, dyn, 3>& e1,
> const std::extents<int, dyn, 2, 4>& e2)
> { return e1 == e2; }
> 0: 31 c0 xor %eax,%eax
> 2: c3 ret
>
> bool same5(const std::extents<int, dyn>& e1,
> const std::extents<int, dyn, dyn>& e2)
> { return e1 == e2; }
> 0: 31 c0 xor %eax,%eax
> 2: c3 ret
>
>
> >>
> >>
> >>>>
> >>>> + else
> >>>> + {
> >>>> + for (size_t __i = 0; __i < __self.rank(); ++__i)
> >>>> + if (!cmp_equal(__self.extent(__i), __other.extent(__i)))
> >>>> + return false;
> >>>> + return true;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + private:
> >>>> + using _S_storage = __mdspan::_ExtentsStorage<
> >>>> + _IndexType, array<size_t, sizeof...(_Extents)>{_Extents...}>;
> >>>> + [[no_unique_address]] _S_storage _M_dynamic_extents;
> >>>> +
> >>>> + template<typename _OIndexType, size_t... _OExtents>
> >>>> + friend class extents;
> >>>> + };
> >>>> +
> >>>> + namespace __mdspan
> >>>> + {
> >>>> + template<typename _IndexType, size_t... _Counts>
> >>>> + auto __build_dextents_type(integer_sequence<size_t, _Counts...>)
> >>>> + -> extents<_IndexType, ((void) _Counts, dynamic_extent)...>;
> >>>> +
> >>>> + template<typename _Tp>
> >>>> + consteval size_t
> >>>> + __dynamic_extent() { return dynamic_extent; }
> >>>> + }
> >>>> +
> >>>> + template<typename _IndexType, size_t _Rank>
> >>>> + using dextents =
> >> decltype(__mdspan::__build_dextents_type<_IndexType>(
> >>>> + make_index_sequence<_Rank>()));
> >>>> +
> >>>> + template<typename... _Integrals>
> >>>> + requires (is_convertible_v<_Integrals, size_t> && ...)
> >>>> + explicit extents(_Integrals...) ->
> >>>> + extents<size_t, __mdspan::__dynamic_extent<_Integrals>()...>;
> >>>>
> >>>> _GLIBCXX_END_NAMESPACE_VERSION
> >>>> }
> >>>> diff --git a/libstdc++-v3/src/c++23/std.cc.in b/libstdc++-v3/src/c++23/
> >> std.cc.in
> >>>> index 930a489ff44..0df27cd7e7d 100644
> >>>> --- a/libstdc++-v3/src/c++23/std.cc.in
> >>>> +++ b/libstdc++-v3/src/c++23/std.cc.in
> >>>> @@ -1833,7 +1833,11 @@ export namespace std
> >>>> }
> >>>> }
> >>>>
> >>>> -// FIXME <mdspan>
> >>>> +// <mdspan>
> >>>> +{
> >>>> + using std::extents;
> >>>> + // FIXME layout_*, default_accessor and mdspan
> >>>> +}
> >>>>
> >>>> // 20.2 <memory>
> >>>> export namespace std
> >>>> --
> >>>> 2.49.0
> >>>>
> >>
> >>
> >
>
