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.
>
>> 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
>> >>
>> >>
>> >
>>
>>
