On 09/12/24 13:22 +0100, Giuseppe D'Angelo wrote:
Hello,

The range-based uninitialized_* algorithm have codepaths that turn constructions into assignments if the output type is trivial. Change this detection to be more accurate. There's a couple of cases which are suspicious and deserve accurate reasoning.

I think those algos should just be updated in the same way (and for
the same reasons) that I did in r15-4473-g3abe751ea86e34

The current optimizations are suboptimal because they are restricted
to cases where the std::copy/std::fill algo can be used, which means
we fail to optimize some cases where memcpy/memset could be used.

But that can be done later, the patch looks OK as-is.


Thanks,
--
Giuseppe D'Angelo

From 41e39c72edba5ab0559881d3b4136d551649e4cc Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
Date: Mon, 9 Dec 2024 02:22:23 +0100
Subject: [PATCH 4/6] libstdc++: port the ranges::uninitialized_* algorithms
away from is_trivial

In preparation for the deprecation of is_trivial (P3247R2).
The rangified uninitialized_* specialized memory algorithms have code
paths where they call the non-uninitialized versions, because the latter
are usually optimized. The detection in these code paths uses is_trivial;
port it away from it towards more specific checks.

The detection for the copy/move algorithms was suspicious: it checked
that the output type was trivial, and that assignment from the input
range reference type was nothrow. If so, the algorithm would copy/move
assign (by calling the ranges::copy/move algorithms) instead of
constructing elements. I think this is off because:

1) the constructor that would be called by the algorithm (which may be
  neither a copy or a move constructor) wasn't checked. If that
  constructor isn't trivial the caller might detect that we're not
  calling it, and that goes against the algorithms' specifications.
2) a nothrow assignment is necessary but not sufficient, as again we
  need to check for triviality, or the caller can detect we're calling
  an assignment operator we were never meant to be calling from these
  algorithms.

Therefore I've amended the respective detections.

libstdc++-v3/ChangeLog:

        * include/bits/ranges_uninitialized.h: port some if
          constexpr away from is_trivial, and towards more specific
          detections instead.

Signed-off-by: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
---
.../include/bits/ranges_uninitialized.h       | 36 ++++++++++---------
1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/libstdc++-v3/include/bits/ranges_uninitialized.h 
b/libstdc++-v3/include/bits/ranges_uninitialized.h
index f16f2ef39f5..ff673d58be7 100644
--- a/libstdc++-v3/include/bits/ranges_uninitialized.h
+++ b/libstdc++-v3/include/bits/ranges_uninitialized.h
@@ -202,8 +202,8 @@ namespace ranges
      operator()(_Iter __first, _Sent __last) const
      {
        using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
-       if constexpr (is_trivial_v<_ValueType>
-                     && is_copy_assignable_v<_ValueType>)
+       if constexpr (is_trivially_default_constructible_v<_ValueType>
+                     && is_trivially_copy_assignable_v<_ValueType>)
          return ranges::fill(__first, __last, _ValueType());
        else
          {
@@ -235,8 +235,8 @@ namespace ranges
      operator()(_Iter __first, iter_difference_t<_Iter> __n) const
      {
        using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
-       if constexpr (is_trivial_v<_ValueType>
-                     && is_copy_assignable_v<_ValueType>)
+       if constexpr (is_trivially_default_constructible_v<_ValueType>
+                     && is_trivially_copy_assignable_v<_ValueType>)
          return ranges::fill_n(__first, __n, _ValueType());
        else
          {
@@ -268,8 +268,9 @@ namespace ranges
        using _OutType = remove_reference_t<iter_reference_t<_Out>>;
        if constexpr (sized_sentinel_for<_ISent, _Iter>
                      && sized_sentinel_for<_OSent, _Out>
-                     && is_trivial_v<_OutType>
-                     && is_nothrow_assignable_v<_OutType&,
+                     && is_trivially_constructible_v<_OutType, 
iter_reference_t<_Iter>>
+                     && is_trivially_default_constructible_v<_OutType>
+                     && is_trivially_assignable_v<_OutType&,
                                                 iter_reference_t<_Iter>>)
          {
            auto __d1 = __ilast - __ifirst;
@@ -316,8 +317,9 @@ namespace ranges
      {
        using _OutType = remove_reference_t<iter_reference_t<_Out>>;
        if constexpr (sized_sentinel_for<_Sent, _Out>
-                     && is_trivial_v<_OutType>
-                     && is_nothrow_assignable_v<_OutType&,
+                     && is_trivially_constructible_v<_OutType, 
iter_reference_t<_Iter>>
+                     && is_trivially_default_constructible_v<_OutType>
+                     && is_trivially_assignable_v<_OutType&,
                                                 iter_reference_t<_Iter>>)
          {
            auto __d = __olast - __ofirst;
@@ -355,8 +357,9 @@ namespace ranges
        using _OutType = remove_reference_t<iter_reference_t<_Out>>;
        if constexpr (sized_sentinel_for<_ISent, _Iter>
                      && sized_sentinel_for<_OSent, _Out>
-                     && is_trivial_v<_OutType>
-                     && is_nothrow_assignable_v<_OutType&,
+                     && is_trivially_constructible_v<_OutType, 
iter_rvalue_reference_t<_Iter>>
+                     && is_trivially_default_constructible_v<_OutType>
+                     && is_trivially_assignable_v<_OutType&,
                                                 
iter_rvalue_reference_t<_Iter>>)
          {
            auto __d1 = __ilast - __ifirst;
@@ -407,8 +410,9 @@ namespace ranges
      {
        using _OutType = remove_reference_t<iter_reference_t<_Out>>;
        if constexpr (sized_sentinel_for<_Sent, _Out>
-                     && is_trivial_v<_OutType>
-                     && is_nothrow_assignable_v<_OutType&,
+                     && is_trivially_constructible_v<_OutType, 
iter_rvalue_reference_t<_Iter>>
+                     && is_trivially_default_constructible_v<_OutType>
+                     && is_trivially_assignable_v<_OutType&,
                                                 
iter_rvalue_reference_t<_Iter>>)
          {
            auto __d = __olast - __ofirst;
@@ -441,8 +445,8 @@ namespace ranges
      operator()(_Iter __first, _Sent __last, const _Tp& __x) const
      {
        using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
-       if constexpr (is_trivial_v<_ValueType>
-                     && is_nothrow_assignable_v<_ValueType&, const _Tp&>)
+       if constexpr (is_trivially_constructible_v<_ValueType, const _Tp&>
+                     && is_trivially_assignable_v<_ValueType&, const _Tp&>)
          return ranges::fill(__first, __last, __x);
        else
          {
@@ -474,8 +478,8 @@ namespace ranges
                 const _Tp& __x) const
      {
        using _ValueType = remove_reference_t<iter_reference_t<_Iter>>;
-       if constexpr (is_trivial_v<_ValueType>
-                     && is_nothrow_assignable_v<_ValueType&, const _Tp&>)
+       if constexpr (is_trivially_constructible_v<_ValueType, const _Tp&>
+                     && is_trivially_assignable_v<_ValueType&, const _Tp&>)
          return ranges::fill_n(__first, __n, __x);
        else
          {
--
2.34.1




Reply via email to