On Thu, Dec 4, 2025 at 10:27 PM Luc Grosheintz <[email protected]>
wrote:
> These are the revised submdspan_mapping related patches from v2.
>
I will try to review it fully today.
>
> A few comments:
>
> - __any_past_the_end: the suggestion was to precheck for an empty
> extent. I kept the current logic, because the additional complexity
> is only at compile time (there was a `constexpr` missing on a `else
> if` branch in v2). At runtime, it's a very simple check:
> (__slice_begin[k] == exts.extent[k]) || ...
> Even if the pre-check would make the condition simpler, for regular
> (non-empty) cases we'd be forced to first check for empty, then
> check for out-of-bounds. Unless I'm missing something, that's
> strictly worse.
>
Yes, after some time of break I think you are making the right call here.
>
> The main motivation here is to eliminate these checks when the slice
> is a collapsing slice, because these checks adds a "lot" of code
> compared to the hand-rolled version. (With the current
> implementation: submdspan(m, i, j, k) and mdspan(&m[i, j, k], E{})
> generate the same code, with E = std::extents<int>).
>
> - As proposed during review, the code to figure out the layout mapping
> type given a combination of slices has been rewritten. Essentially,
> we have a `enum class _SliceKind` what's (forced to be) categorical.
> From there one can use for-loops and immediate (consteval) functions
> insteads of traditional recursive structs.
>
It looks much more readable now, only leaving small comments about not using
_Size as template parameter.
>
> The twist here is that a __full slice is not a __unit_strided_slice
> slice. This is because: I don't want to use weak/plain enums;
> strong enums (enum class) don't do bitwise operations without
> implementing a lot of boilerplate; bitset feels inappropriate.
> Hence, this solution felt the one with the least boilerplate.
> Again, I might be missing a trick.
>
> - In __substrides_standardized I chose not use _M_strides, because
> _M_strides has size `rank`; while __substrides_standardized only
> returns an array of lenght `__subrank`. The difference can be seen,
> e.g., for submdspan(m, 1, 2, 3, full) which (technically) performs 3
> multiplications when zero are needed. Therefore, one would rely on
> the optimizer to eliminate the extra operation. It might be
> recognizable as dead code, but I've not checked.
>
Yes, it makes sense.
>
>
> - Let me know if you want me to move the __subextents code in a
> separate commit.
>
Not necessary.
>
> - I started following the suggestion to always use qualified names for
> functions. Let me know if you want to qualify everything (typenames,
> concepts, etc.)
>
We qualify functions to avoid the compiler doing ADL, even if it will
result in only a
single candidate. No need to qualify anything else.
>
> Luc Grosheintz (5):
> libstdc++: Implement submdspan and submdspan_mapping for layout_left.
> [PR110352]
> libstdc++: Implement submdspan_mapping for layout_right. [PR110352]
> libstdc++: Implement submdspan_mapping for layout_stride. [PR110352]
> libstdc++: Implement submdspan_mapping for layout_left_padded.
> [PR110352]
> libstdc++: Implement submdspan_mapping for layout_right_padded.
> [PR110352]
>
> libstdc++-v3/include/std/mdspan | 713 ++++++++++++++++--
> libstdc++-v3/src/c++23/std.cc.in | 2 +-
> .../23_containers/mdspan/layout_traits.h | 4 +
> .../mdspan/submdspan/submdspan.cc | 377 +++++++++
> .../mdspan/submdspan/submdspan_mapping.cc | 301 ++++++++
> .../mdspan/submdspan/submdspan_neg.cc | 179 +++++
> 6 files changed, 1514 insertions(+), 62 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/23_containers/mdspan/submdspan/submdspan.cc
> create mode 100644
> libstdc++-v3/testsuite/23_containers/mdspan/submdspan/submdspan_mapping.cc
> create mode 100644
> libstdc++-v3/testsuite/23_containers/mdspan/submdspan/submdspan_neg.cc
>
> --
> 2.52.0
>
>
