On Thu, Jul 17, 2025 at 11:57 AM Luc Grosheintz <luc.groshei...@gmail.com>
wrote:
>
>
> On 7/8/25 17:37, Tomasz Kaminski wrote:
> > On Thu, Jul 3, 2025 at 12:38 PM Luc Grosheintz <luc.groshei...@gmail.com
> >
> > wrote:
> >
> >> This commit completes the implementation of P2897R7 by implementing and
> >> testing the template class aligned_accessor.
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >> * include/bits/version.def (aligned_accessor): Add.
> >> * include/bits/version.h: Regenerate.
> >> * include/std/mdspan (aligned_accessor): New class.
> >> * src/c++23/std.cc.in (aligned_accessor): Add.
> >> * testsuite/23_containers/mdspan/accessors/generic.cc: Add
> tests
> >> for aligned_accessor.
> >> * testsuite/23_containers/mdspan/accessors/aligned.cc: New
> test.
> >> * testsuite/23_containers/mdspan/accessors/aligned_ftm.cc: New
> >> test.
> >> * testsuite/23_containers/mdspan/accessors/aligned_neg.cc: New
> >> test.
> >> ---
> >> libstdc++-v3/include/bits/version.def | 10 +++
> >> libstdc++-v3/include/bits/version.h | 10 +++
> >> libstdc++-v3/include/std/mdspan | 72 +++++++++++++++++++
> >> libstdc++-v3/src/c++23/std.cc.in | 3 +-
> >> .../23_containers/mdspan/accessors/aligned.cc | 43 +++++++++++
> >> .../mdspan/accessors/aligned_ftm.cc | 6 ++
> >> .../mdspan/accessors/aligned_neg.cc | 33 +++++++++
> >> .../accessors/debug/aligned_access_neg.cc | 23 ++++++
> >> .../accessors/debug/aligned_offset_neg.cc | 23 ++++++
> >> .../23_containers/mdspan/accessors/generic.cc | 27 +++++++
> >> 10 files changed, 249 insertions(+), 1 deletion(-)
> >> create mode 100644
> >> libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned.cc
> >> create mode 100644
> >> libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_ftm.cc
> >> create mode 100644
> >> libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_neg.cc
> >> create mode 100644
> >>
> libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_access_neg.cc
> >> create mode 100644
> >>
> libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_offset_neg.cc
> >>
> >> diff --git a/libstdc++-v3/include/bits/version.def
> >> b/libstdc++-v3/include/bits/version.def
> >> index a2695e67716..42445165b91 100644
> >> --- a/libstdc++-v3/include/bits/version.def
> >> +++ b/libstdc++-v3/include/bits/version.def
> >> @@ -1022,6 +1022,16 @@ ftms = {
> >> };
> >> };
> >>
> >> +ftms = {
> >> + name = aligned_accessor;
> >> + values = {
> >> + v = 202411;
> >> + cxxmin = 26;
> >> + extra_cond = "__glibcxx_assume_aligned "
> >> + "&& __glibcxx_is_sufficiently_aligned";
> >> + };
> >> +};
> >> +
> >> ftms = {
> >> name = ssize;
> >> values = {
> >> diff --git a/libstdc++-v3/include/bits/version.h
> >> b/libstdc++-v3/include/bits/version.h
> >> index 1b17a965239..3efa7b1baae 100644
> >> --- a/libstdc++-v3/include/bits/version.h
> >> +++ b/libstdc++-v3/include/bits/version.h
> >> @@ -1143,6 +1143,16 @@
> >> #endif /* !defined(__cpp_lib_mdspan) &&
> defined(__glibcxx_want_mdspan) */
> >> #undef __glibcxx_want_mdspan
> >>
> >> +#if !defined(__cpp_lib_aligned_accessor)
> >> +# if (__cplusplus > 202302L) && (__glibcxx_assume_aligned &&
> >> __glibcxx_is_sufficiently_aligned)
> >> +# define __glibcxx_aligned_accessor 202411L
> >> +# if defined(__glibcxx_want_all) ||
> >> defined(__glibcxx_want_aligned_accessor)
> >> +# define __cpp_lib_aligned_accessor 202411L
> >> +# endif
> >> +# endif
> >> +#endif /* !defined(__cpp_lib_aligned_accessor) &&
> >> defined(__glibcxx_want_aligned_accessor) */
> >> +#undef __glibcxx_want_aligned_accessor
> >> +
> >> #if !defined(__cpp_lib_ssize)
> >> # if (__cplusplus >= 202002L)
> >> # define __glibcxx_ssize 201902L
> >> diff --git a/libstdc++-v3/include/std/mdspan
> >> b/libstdc++-v3/include/std/mdspan
> >> index c72a64094b7..6eb804bf9a0 100644
> >> --- a/libstdc++-v3/include/std/mdspan
> >> +++ b/libstdc++-v3/include/std/mdspan
> >> @@ -39,7 +39,12 @@
> >> #include <limits>
> >> #include <utility>
> >>
> >> +#if __cplusplus > 202302L
> >> +#include <bits/align.h>
> >> +#endif
> >> +
> >> #define __glibcxx_want_mdspan
> >> +#define __glibcxx_want_aligned_accessor
> >> #include <bits/version.h>
> >>
> >> #ifdef __glibcxx_mdspan
> >> @@ -1035,6 +1040,73 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >> { return __p + __i; }
> >> };
> >>
> >> +#ifdef __glibcxx_aligned_accessor
> >> + template<typename _ElementType, size_t _ByteAlignment>
> >> + struct aligned_accessor
> >> + {
> >> + static_assert(has_single_bit(_ByteAlignment),
> >> + "ByteAlignment must be a power of two");
> >> + static_assert(_ByteAlignment >= alignof(_ElementType),
> >> + "ByteAlignment is too small for ElementType");
> >> + static_assert(!is_array_v<_ElementType>,
> >> + "ElementType must not be an array type");
> >> + static_assert(!is_abstract_v<_ElementType>,
> >> + "ElementType must not be an abstract class type");
> >> +
> >> + using offset_policy = default_accessor<_ElementType>;
> >> + using element_type = _ElementType;
> >> + using reference = element_type&;
> >> + using data_handle_type = element_type*;
> >> +
> >> + static constexpr size_t byte_alignment = _ByteAlignment;
> >> +
> >> + constexpr
> >> + aligned_accessor() noexcept = default;
> >> +
> >> + template<typename _OElementType, size_t _OByteAlignment>
> >> + requires (is_convertible_v<_OElementType(*)[],
> element_type(*)[]>
> >> + && _OByteAlignment >= byte_alignment)
> >> + constexpr
> >> + aligned_accessor(aligned_accessor<_OElementType,
> _OByteAlignment>)
> >> + noexcept
> >> + { }
> >> +
> >> + template<typename _OElementType>
> >> + requires is_convertible_v<_OElementType(*)[], element_type(*)[]>
> >> + constexpr explicit
> >> + aligned_accessor(default_accessor<_OElementType>) noexcept
> >> + { }
> >> +
> >> + template<typename _OElementType>
> >> + requires is_convertible_v<_OElementType(*)[], element_type(*)[]>
> >> + constexpr
> >> + operator default_accessor<_OElementType>() const noexcept
> >> + { return {}; }
> >> +
> >> + constexpr reference
> >> + access(data_handle_type __p, size_t __i) const noexcept
> >> + {
> >> + if !consteval
> >> + {
> >> + _GLIBCXX_DEBUG_ASSERT(
> >> + std::is_sufficiently_aligned<_ByteAlignment>(__p));
> >> + }
> >>
> > I would not add this check, as the purpose of this accessor is to assume
> > aligned,
> > and that should be checked during mdspan construction, however we do not
> > currently have a
> > way to do so.
> > Such a check is currently performed anyway by assume_aligned, and in my
> > opinion should stay there.
>
> Correct, I'll update the patch accordingly.
>
> >
> >
> >> + return std::assume_aligned<byte_alignment>(__p)[__i];
> >> + }
> >> +
> >> + constexpr typename offset_policy::data_handle_type
> >> + offset(data_handle_type __p, size_t __i) const noexcept
> >> + {
> >> + if !consteval
> >> + {
> >> + _GLIBCXX_DEBUG_ASSERT(
> >> + std::is_sufficiently_aligned<_ByteAlignment>(__p));
> >> + }
> >> + return std::assume_aligned<byte_alignment>(__p) + __i;
> >> + }
> >> + };
> >> +#endif
> >> +
> >> _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 6f4214ed3a7..02e1713bdc3 100644
> >> --- a/libstdc++-v3/src/c++23/std.cc.in
> >> +++ b/libstdc++-v3/src/c++23/std.cc.in
> >> @@ -1851,7 +1851,8 @@ export namespace std
> >> using std::layout_right;
> >> using std::layout_stride;
> >> using std::default_accessor;
> >> - // FIXME layout_left_padded, layout_right_padded, aligned_accessor
> and
> >> mdspan
> >> + using std::aligned_accessor;
> >> + // FIXME layout_left_padded, layout_right_padded and mdspan
> >> }
> >> #endif
> >>
> >> diff --git
> >> a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned.cc
> >> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned.cc
> >> new file mode 100644
> >> index 00000000000..75581d3eb70
> >> --- /dev/null
> >> +++ b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned.cc
> >> @@ -0,0 +1,43 @@
> >> +// { dg-do compile { target c++26 } }
> >> +#include <mdspan>
> >> +
> >> +#include <testsuite_hooks.h>
> >> +
> >> +constexpr bool
> >> +test_from_other()
> >> +{
> >> + std::aligned_accessor<double, 4*sizeof(double)> a4;
> >> + [[maybe_unused]] std::aligned_accessor<double, 2*sizeof(double)>
> a2(a4);
> >> +
> static_assert(std::is_nothrow_convertible_v<std::aligned_accessor<char,
> >> 4>,
> >> +
> std::aligned_accessor<char,
> >> 2>>);
> >> + static_assert(!std::is_convertible_v<std::aligned_accessor<char, 2>,
> >> + std::aligned_accessor<char, 4>>);
> >>
> > I would check !is_cosntructible, to indicate that there is no explicit
> > constructor.
> >
> >> + return true;
> >> +}
> >> +static_assert(test_from_other());
> >> +
> >> +constexpr bool
> >> +test_from_default()
> >> +{
> >> + std::default_accessor<double> ad;
> >> + [[maybe_unused]] std::aligned_accessor<double, 4*sizeof(double)>
> a4(ad);
> >> + static_assert(!std::is_convertible_v<std::default_accessor<char>,
> >> + std::aligned_accessor<char, 1>>);
> >> + static_assert(!std::is_convertible_v<std::default_accessor<char>,
> >> + std::aligned_accessor<char, 2>>);
> >> + static_assert(std::is_nothrow_constructible_v<
> >> + std::aligned_accessor<char, 4>, std::default_accessor<char>>);
> >> + return true;
> >> +}
> >> +static_assert(test_from_default());
> >> +
> >> +constexpr bool
> >> +test_to_default()
> >> +{
> >> + std::aligned_accessor<double, 4*sizeof(double)> a4;
> >> + [[maybe_unused]] std::default_accessor<double> ad = a4;
> >> +
> static_assert(std::is_nothrow_convertible_v<std::aligned_accessor<char,
> >> 2>,
> >> +
> >> std::default_accessor<char>>);
> >> + return true;
> >> +}
> >> +static_assert(test_to_default());
> >> diff --git
> >> a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_ftm.cc
> >> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_ftm.cc
> >> new file mode 100644
> >> index 00000000000..361bf750c33
> >> --- /dev/null
> >> +++
> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_ftm.cc
> >> @@ -0,0 +1,6 @@
> >> +// { dg-do compile { target c++26 } }
> >> +#include <mdspan>
> >> +
> >> +#ifndef __cpp_lib_aligned_accessor
> >> +#error "Missing FTM"
> >>
> > Updated how this test is performed. And I would rename this file to
> version.
> >
> >> +#endif
> >> diff --git
> >> a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_neg.cc
> >> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_neg.cc
> >> new file mode 100644
> >> index 00000000000..46d80bf4083
> >> --- /dev/null
> >> +++
> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/aligned_neg.cc
> >> @@ -0,0 +1,33 @@
> >> +// { dg-do compile { target c++26 } }
> >> +#include<mdspan>
> >> +
> >> +#include <cstdint>
> >> +
> >> +std::aligned_accessor<uint32_t, 0> a; // { dg-error "required
> >> from here" }
> >> +std::aligned_accessor<uint32_t, 7> b; // { dg-error "required
> >> from here" }
> >> +std::aligned_accessor<uint32_t, size_t(-1)> c; // { dg-error "required
> >> from here" }
> >> +
> >> +std::aligned_accessor<uint32_t, 2> d; // { dg-error "required
> >> from here" }
> >> +
> >> +std::aligned_accessor<int[2], 32> e; // { dg-error "required
> >> from here" }
> >> +
> >> +class Abstract
> >> +{
> >> + virtual void
> >> + foo() const = 0;
> >> +};
> >> +
> >> +class Derived : public Abstract
> >> +{
> >> + void
> >> + foo() const override
> >> + { }
> >> +};
> >> +
> >> +std::aligned_accessor<Derived, alignof(int)> f_ok;
> >> +std::aligned_accessor<Abstract, alignof(int)> f_err; // { dg-error
> >> "required from here" }
> >> +
> >> +// { dg-prune-output "ByteAlignment must be a power of two" }
> >> +// { dg-prune-output "ByteAlignment is too small for ElementType" }
> >> +// { dg-prune-output "ElementType must not be an array type" }
> >> +// { dg-prune-output "ElementType must not be an abstract" }
> >> diff --git
> >>
> a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_access_neg.cc
> >>
> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_access_neg.cc
> >> new file mode 100644
> >> index 00000000000..3511cef1c3a
> >> --- /dev/null
> >> +++
> >>
> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_access_neg.cc
> >>
> > Again, I feel like these negative checks belong to assume_alinged.
>
> I fully agree that offset and access can rely on assume_aligned
> to do the check.
>
> However, testing that the two methods fail for non-aligned input seems
> prudent. We're testing a property of offset and access. They happen to
> rely on assume_aligned to perform the check, but that's a detail. We're
> not testing how it does the check, only that the check happens.
>
> Since it might make the tests too brittle, feel free to insist.
>
The standard just says that this is UB, so we are testing something that is
not
required to pass by the standard. This is why I am a bit hesitant on death
tests like that.
Jonathan, do you have an opinion on this?
>
> >
> >> @@ -0,0 +1,23 @@
> >> +// { dg-do run { target c++26 xfail *-*-* } }
> >> +// { dg-require-debug-mode "" }
> >> +
> >> +#include <mdspan>
> >> +#include <array>
> >> +
> >> +void
> >> +test_unaligned_access()
> >> +{
> >> + constexpr size_t N = 4;
> >> + alignas(N) std::array<char, 128> buffer{};
> >> + auto* unaligned = buffer.data() + 1;
> >> + auto a = std::aligned_accessor<char, N>{};
> >> +
> >> + [[maybe_unused]] char x = a.access(unaligned, 0);
> >> +}
> >> +
> >> +int
> >> +main()
> >> +{
> >> + test_unaligned_access();
> >> + return 0;
> >> +};
> >> diff --git
> >>
> a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_offset_neg.cc
> >>
> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_offset_neg.cc
> >> new file mode 100644
> >> index 00000000000..319da5ffef3
> >> --- /dev/null
> >> +++
> >>
> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/debug/aligned_offset_neg.cc
> >> @@ -0,0 +1,23 @@
> >> +// { dg-do run { target c++26 xfail *-*-* } }
> >> +// { dg-require-debug-mode "" }
> >> +
> >> +#include <mdspan>
> >> +#include <array>
> >> +
> >> +void
> >> +test_unaligned_offset()
> >> +{
> >> + constexpr size_t N = 4;
> >> + alignas(N) std::array<char, 128> buffer{};
> >> + auto* unaligned = buffer.data() + 1;
> >> + auto a = std::aligned_accessor<char, N>{};
> >> +
> >> + [[maybe_unused]] char* x = a.offset(unaligned, 0);
> >> +}
> >> +
> >> +int
> >> +main()
> >> +{
> >> + test_unaligned_offset();
> >> + return 0;
> >> +};
> >> diff --git
> >> a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/generic.cc
> >> b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/generic.cc
> >> index 600d152b690..f97946bd276 100644
> >> --- a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/generic.cc
> >> +++ b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/generic.cc
> >> @@ -98,10 +98,32 @@
> >> static_assert(test_class_properties<std::default_accessor<double>>());
> >> static_assert(test_accessor_policy<std::default_accessor<double>>());
> >> static_assert(test_ctor<DefaultAccessorTrait>());
> >>
> >> +#ifdef __glibcxx_aligned_accessor
> >> +struct AlignedAccessorTrait
> >> +{
> >> + template<typename T>
> >> + using type = std::aligned_accessor<T, alignof(T)>;
> >> +};
> >> +
> >> +static_assert(test_class_properties<std::aligned_accessor<double,
> >> +
> >> sizeof(double)>>());
> >> +static_assert(test_accessor_policy<std::aligned_accessor<double,
> >> +
> >> sizeof(double)>>());
> >> +static_assert(test_accessor_policy<std::aligned_accessor<double,
> >> +
> >> 2*sizeof(double)>>());
> >> +static_assert(test_ctor<AlignedAccessorTrait>());
> >> +#endif
> >> +
> >> template<typename A>
> >> constexpr size_t
> >> accessor_alignment = sizeof(typename A::element_type);
> >>
> >> +#ifdef __glibcxx_aligned_accessor
> >> +template<typename T, size_t N>
> >> + constexpr size_t
> >> + accessor_alignment<std::aligned_accessor<T, N>> = N;
> >> +#endif
> >> +
> >> template<typename Accessor>
> >> constexpr void
> >> test_access(Accessor accessor)
> >> @@ -137,5 +159,10 @@ main()
> >> {
> >> test_all<std::default_accessor<double>>();
> >> static_assert(test_all<std::default_accessor<double>>());
> >> +
> >> +#ifdef __glibcxx_aligned_accessor
> >> + test_all<std::aligned_accessor<double, 4*sizeof(double)>>();
> >> + static_assert(test_all<std::aligned_accessor<double,
> >> 4*sizeof(double)>>());
> >> +#endif
> >> return 0;
> >> }
> >> --
> >> 2.49.0
> >>
> >>
> >
>
>
