On 8/4/25 17:42, Tomasz Kaminski wrote:
On Mon, Aug 4, 2025 at 1:14 PM Tomasz Kaminski <tkami...@redhat.com> wrote:



On Mon, Aug 4, 2025 at 1:08 PM Luc Grosheintz <luc.groshei...@gmail.com>
wrote:

Hi Tomasz,

Thank you for the review! Sorry about the missing parens, even after
"Ctrl+F"ing each of the emails I can't find the requires clause (the
two I found both need the parens).

I was mentioning this one, but it is possible that I am wrong on this one,
I haven't yet checked it, as I was planning to do it during full review.
     template<array _Extents>
       requires (__all_dynamic<_Extents>())
       class _StaticExtents<_Extents>;
But regardless, I will make that change locally if it does work, so you do
not
need to update patch series for it.

Parenthesis seem to be indeed required here, so my comment was a red
herring.

I'm pretty sure that this is the reason why I wrongly concluded
that the pattern is `requires(cond)`, i.e. the parens are always
needed.



Do you think it's possible to merge this series before the other two
outstanding patch series? There's some risk of collision; making changes
to this patch series is much more time consuming than rebasing (and
retesting the other two patch series).

This sounds very reasonable. I will prepare a stack of patches in your
suggested
order locally, and push them once they are approved. But it still make


Thank you,

Thank you for your continued contributions.

Luc

On 8/4/25 12:43, Tomasz Kaminski wrote:
Hi,

I this time I made a quick pass through all changes, before commenting
on
first commits,
and they look solid to me, and I haven't noticed anything I would like
to
change (except parentheses
around requires, but I will handle that locally). I will try to do a
full
review during this week.

Regards,
Tomasz

On Sun, Aug 3, 2025 at 10:59 PM Luc Grosheintz <
luc.groshei...@gmail.com>
wrote:

The combined effect of this sequence of change is:

    * a reduction in the number of template instantiations, by
      - avoiding needless dependency of IndexType,
      - special formulas for low-rank extents,
      - special formulas for (nearly) fully dynamic extents.

    * improved code quality, by
      - precomputing partial products of the static extents,
      - special cases for low-rank extents,
      - rewriting the condition E[i] == dynamic_extent in a more
        optimizer friendly manner.
      - effectively loop-unrolling extents::operator==.

While simplistic micro-benchmarking shows the effectiveness of these
changes, likely the stronger argument is presented in each commit:
    a) each change removes needless complexity,
    b) before/after examples of generated code show the effectiveness.

Luc Grosheintz (8):
    libstdc++: Reduce template instantiations in <mdspan>.
    libstdc++: Precompute products of static extents.
    libstdc++: Improve low-rank layout_{left,right}::stride.
    libstdc++: Improve fully dynamic extents in mdspan.
    libstdc++: Improve nearly fully dynamic extents in mdspan.
    libstdc++: Reduce indirection in extents::extent.
    libstdc++: Improve extents::operator==.
    libstdc++: Replace numeric_limit with __int_traits in mdspan.

   libstdc++-v3/include/std/mdspan               | 282
+++++++++++++-----
   .../mdspan/extents/class_mandates_neg.cc      |   3 +
   2 files changed, 208 insertions(+), 77 deletions(-)

--
2.50.0







Reply via email to