On 5/22/25 15:21, Tomasz Kaminski wrote:
Thanks for working on the patches, they look solid, comments.
Could you prepare a separate patch to fix initialization
default-initialization of extents,
that you have noticed, standard requires them to be value-initialized, and
add corresponding test?
Yes, I was thinking of something that statically allocates
sizeof(Extents) bytes, sets all the bits to 1 and then uses placement
new to "run" the default ctor. That way we can be confident that
the zeros we're observing aren't accidental.
We do a renaming within this patch series that affects the value
initialization patch. Should I submit a v4 that applies cleanly
to master *after* applying the value initialization patch? (And
cause conflict if applied to the current state of master.)
Similarly, we have test for default constructor of stride_mapping, I would
add them for other layouts,
and check:
* all extents being static -> depends on mapping
* first extent being dynamic, and rest static -> all strides are zero
* middle extent being dynamic
Yes, definitely.
For the stride and product computation, we should perform them in
Extent::size_type, not index_type.
The latter may be signed, and we may hit UB in multiplying non-zero
extents, before reaching the zero.
This seems unfortunate, because if I understood correctly, the idea
behind allowing signed `index_type`, was that signed integer operations
are faster (exactly because they don't need to behave "well" on overflow).
I've made the changes to use unsigned integers. However, I'd like to add
this to my pile of performance questions to be investigated.
I believe that since constexpr code must be UB free, we should be able
to create tests, that can detect overflow in the naive formulas.
For is_exhaustive question, I will write to LWG reflector to ask authors,
and see what their opinion is.
Will keep you posted.
Thank you! Somehow I'm failing to figure out a reasonable algorithm
to check it in a conformant way, so this is definitely welcome.
(Brute-force is not reasonable.)
In a lot of tests we are doing, where I believe we could skip template
parameters, and deduce it for argument.
verify_nothrow_convertible<Layout, std::extents<int>>(
+ std::extents<int>{});
Could you look into doing it?
I could only find them in cases like:
template<typename Layout, typename Extents, typename OExtents>
const void
verify_nothrow_convertible(OExtents oexts);
i.e. the first is explicitly given and the second is deduced. This
happens when checking properties of mappings of the same layout with
different extent_types. I didn't add anything to reduce the amount
of code when Extents == OExtents, because it felt non-uniform (I
don't like remembering special cases) and a little error prone. Let
me know if you see it differently and would like me to add a default
template argument to handle the case Extents == OExtents.
Regards,
Tomasz
On Thu, May 22, 2025 at 2:21 PM Tomasz Kaminski <tkami...@redhat.com> wrote:
On Wed, May 21, 2025 at 4:21 PM Luc Grosheintz <luc.groshei...@gmail.com>
wrote:
It's missing the "registration" of the three new classes in
std.cc.in.
Please remember to add it in next revisions.
On 5/21/25 11:40, Luc Grosheintz wrote:
Follows up on:
https://gcc.gnu.org/pipermail/libstdc++/2025-May/061535.html
To improve naming conventions, this series includes three new commits:
* Two commits to rename _ExtentsStorage::_M_dynamic_extents, and
extents::_M_dynamic_extents.
* One commit to cleanup whitespace errors in extents.
The changes to the existing commits are:
* Fix division by zero bug.
* Rename subextents -> extents.
* Default arguments for __{static,dynamic}_extents.
* Default argument for __static_quotient.
* Four times: use range-based for.
* Eliminate __has_static_zero
* Short-circuit in __static_quotient.
* Optimize __exts_prod for rank == rank_dynamic.
This review suggestion was intentionally skipped:
* Inline helper of __exts_prod, because with the additional
optimization for rank == rank_dynamic, having two separate
functions makes the highlevel structure a little bit more
obvious. Additionally, there's numerous changes planned that
might make one of the two functions much more verbose.
Luc Grosheintz (9):
libstdc++: Rename _ExtentsStorage::_M_dynamic_extents.
libstdc++: Rename extents::_M_dynamic_extents.
libstdc++: Cleanup formatting in mdspan.
libstdc++: Implement layout_left from mdspan.
libstdc++: Add tests for layout_left.
libstdc++: Implement layout_right from mdspan.
libstdc++: Add tests for layout_right.
libstdc++: Implement layout_stride from mdspan.
libstdc++: Add tests for layout_stride.
libstdc++-v3/include/std/mdspan | 692 +++++++++++++++++-
.../mdspan/layouts/class_mandate_neg.cc | 42 ++
.../23_containers/mdspan/layouts/ctors.cc | 401 ++++++++++
.../23_containers/mdspan/layouts/mapping.cc | 569 ++++++++++++++
.../23_containers/mdspan/layouts/stride.cc | 494 +++++++++++++
5 files changed, 2185 insertions(+), 13 deletions(-)
create mode 100644
libstdc++-v3/testsuite/23_containers/mdspan/layouts/class_mandate_neg.cc
create mode 100644
libstdc++-v3/testsuite/23_containers/mdspan/layouts/ctors.cc
create mode 100644
libstdc++-v3/testsuite/23_containers/mdspan/layouts/mapping.cc
create mode 100644
libstdc++-v3/testsuite/23_containers/mdspan/layouts/stride.cc
