| Message ID | 20251204212846.248887-1-luc.grosheintz@gmail.com |
|---|---|
| Headers |
Return-Path: <gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 313344B1A2B5 for <patchwork@sourceware.org>; Thu, 4 Dec 2025 21:28:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 313344B1A2B5 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=RgImg8I9 X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) by sourceware.org (Postfix) with ESMTPS id A8C0D4BC89B6 for <gcc-patches@gcc.gnu.org>; Thu, 4 Dec 2025 21:26:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A8C0D4BC89B6 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A8C0D4BC89B6 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.221.53 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1764883602; cv=none; b=VHLDt3rmmmaO+QdXTfvBDoGcLkealG6r9uQFylHO84AOLCzbW3R7IhX0NIDG9iMztxC0XJmCRzADVbWy/COhVndKjjYPdTss4SJw4z3Sd/MCiuSnv/3E+SCMQPuRFHYRsPmaHqPmPj34u5PcqYXpSbc2hJRdLGuTzdzp8k4HPJA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1764883602; c=relaxed/simple; bh=QTohjC13YmsOpr4jCyf+KkfnpyRGEqAAXWzd3T8HGNY=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=rvyoOovLToI0c2Dka4w02ybZ3BR/KUVCGwYlnmqAMSKRSHS+kmcsiSKFuWqeptt0kjXXTOK2VXhuoXOUI+m1nIHE3ieMY0igVZkDttuUYYPPainfHvnk0d+UxMh+1AFXXUTdv38yRHC3+Vlzi/E14B/8TYDPgBoTGTt2D48TL6Y= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A8C0D4BC89B6 Received: by mail-wr1-f53.google.com with SMTP id ffacd0b85a97d-42e2e445dbbso745079f8f.2 for <gcc-patches@gcc.gnu.org>; Thu, 04 Dec 2025 13:26:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764883601; x=1765488401; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=Yv3ECs4eyk6xPbV38kR8CfHoJXm8/E67WxBSH/iNjk4=; b=RgImg8I9DqDS50EO9Qjvg/YmxLeNyeBhHpd2302m8XbXkn/Q+ZBHrHnYoH8zQ7RSbD QLznUPjpEm/NYAEZD0Acsr7t2Y8xBCSKHYb+3lC0lUzcbelVLL0wcRxiE0Qa5p+fdhQo sMSlYKYJa9chQ1amD5mOWhyOpiCqf3F5+Aq9QhXMMt+bEGSwtvi38yA4VL6FzVyh7M6q y7c9//W2BtpPVu9vNPf2rGdKtRdFV5TgFE4tK1yduL9zl0//v8lFunjiPlO8Or5u248E I6sGt7OLhmOZlGb5pGJmFsENEwAA0faJ/3NRGN5nzqKs6j5sQ+qSO5FEjoI/jxBmzKc0 jhiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764883601; x=1765488401; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Yv3ECs4eyk6xPbV38kR8CfHoJXm8/E67WxBSH/iNjk4=; b=gu9n9Onul63t9Pfq1jzCRvDV1lCUjvLjepRNShqlz47b8V64Sn/ufI7claLWlydXea KSWqByMh/l/yLuDs+12qGqUBdvXL4g7lSWseshrxX8C5f5C4PwynIsO/kO80aqUSo56y gOjdFgNRP4AZDb5md4bb4KIpkxUtrExm+H1XqD63QaWIvJYPEqM/of3s/GD2GtZG9WtU BLoiwbZDsFSJBMOCyNZRf0UTSnHCt8XP1stCy9mYucBPXGtp8Z+ASeNP8DS5vHDh+Oj+ 3CbaZyvsENEXH3VNu06EN/it4jx7KK/Gf1zYFVHrb3Ar1p5E9ZuzI8dxTwWkzSNBNhNl 6WyA== X-Gm-Message-State: AOJu0YxaDRz1ueYYx6T79stJ1l+l528GEc0MUkHK0xLrSVqOCDo8QI85 lKRRxiobp1F2NJywSXzThamSsB5LZuFMhiibO35kXMJQSOQbjHnyA8C7 X-Gm-Gg: ASbGnctqKzMM7oJqpznTfpbE62/i4j+YCtdQplTeGTTr+S+Avcb+gg0ESjKAYoGb6NF Qx4XfaEQJDx0PyiMrxrTSZeQ6RCN0w9SXUJWSA9EFOHyIf5yZMNHFW3eV/XP6K9kux18OuDlLoh 9A4ZgIynwROua5OxIHzBC0xjBzTZpeGgaOvKmN+1iEmvw+0Vla0NFelwde9ZfE8z/j97cSkrjCh eWKKvNqF7MRiPHG2SBsG1GqHGxzq1GB7qwtjXVkVprSZIfMb/GKy+wyrK6KqkC4lx41npoVpdMV m+fJrCP3PE+fbn0iAmSnhc/+QJlmUP4vaa8fNbL8Yh96Cy2ljuCqN+LOMh1VK/hKuUwr9SJZqOd dEIN322T+eDF2QX/0+dIBItiHzuzU4lJGOxm5/fL/ebhSrfA5Vjdq16+0tKs5ujUgBpnH4ZDOZL ktUPJvbeZEsKuiM4DhXr4l9LETW2ku3Ulxx2tavEfKkmDWuT8lFNE9Wig3IsHn X-Google-Smtp-Source: AGHT+IEXUCdXpU3qVfH/slKBvJ2Ks5VSHppjo3Bs+celZqc4LH9p/+X+yUH+ArjbBTGfbKB/Q28Sag== X-Received: by 2002:a5d:5c89:0:b0:42b:3339:c7ff with SMTP id ffacd0b85a97d-42f731bcd33mr7910169f8f.43.1764883601307; Thu, 04 Dec 2025 13:26:41 -0800 (PST) Received: from piglet.localdomain (212-51-146-223.fiber7.init7.net. [212.51.146.223]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-42f7d331e29sm5354432f8f.32.2025.12.04.13.26.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Dec 2025 13:26:40 -0800 (PST) From: Luc Grosheintz <luc.grosheintz@gmail.com> To: libstdc++@gcc.gnu.org Cc: gcc-patches@gcc.gnu.org, Luc Grosheintz <luc.grosheintz@gmail.com> Subject: [PATCH v4 0/5] libstdc++: submdspan (part 2) Date: Thu, 4 Dec 2025 22:28:41 +0100 Message-ID: <20251204212846.248887-1-luc.grosheintz@gmail.com> X-Mailer: git-send-email 2.52.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_BLOCKED, RCVD_IN_MSPIKE_H2, RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org |
| Series |
libstdc++: submdspan (part 2)
|
|
Message
Luc Grosheintz
Dec. 4, 2025, 9:28 p.m. UTC
These are the revised submdspan_mapping related patches from v2.
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.
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.
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.
- Let me know if you want me to move the __subextents code in a
separate commit.
- I started following the suggestion to always use qualified names for
functions. Let me know if you want to qualify everything (typenames,
concepts, etc.)
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
Comments
On Thu, Dec 4, 2025 at 10:27 PM Luc Grosheintz <luc.grosheintz@gmail.com> 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 > >
The implementation looks solid now, only very small comments there. Please add a patch updating feature test macro, my preference would be to keep internal padded layouts FTM and give it a value 202403, with comment that corresponds to new layouts part of https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p2642r6.pdf. I assume this is because, you were unsure about the value of FTM, fortunately sd-6 ( https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations ) already list the value for paper 202411. Regards, Tomasz On Fri, Dec 5, 2025 at 10:29 AM Tomasz Kaminski <tkaminsk@redhat.com> wrote: > > > On Thu, Dec 4, 2025 at 10:27 PM Luc Grosheintz <luc.grosheintz@gmail.com> > 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 >> >>