On Wed, 11 Jun 2025 at 10:42, Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
>
> On Tue, Jun 10, 2025 at 6:17 PM Jonathan Wakely <jwak...@redhat.com> wrote:
>>
>> With improved memset optimizations in std::uninitialized_fill and
>> std::uninitialized_fill_n (see r15-4473-g3abe751ea86e34), we can make
>> the non-standard internal helpers __uninitialized_default and
>> __uninitialized_default_n use those directly instead of using std::fill
>> and std::fill_n respectively. And if we do that, we no longer need to
>> check whether the type is assignable, because avoiding std::fill means
>> no assignment happens.
>>
>> If the type being constructed is trivially default constructible and
>> trivially copy constructible, then it's unobservable if we construct one
>> object and copy it N-1 times, rather than constructing N objects. For
>> byte-sized integer types this allows the loop to be replaced with
>> memset.
>>
>> Because these functions are not defined for C++98 at all, we can use
>> if-constexpr to simplify them and remove the dispatching to members of
>> class template specializations.
>>
>> By removing the uses of std::fill and std::fill_n we no longer need to
>> include stl_algobase.h in stl_uninitialized.h which might improve
>> compilation time for some other parts of the library.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/bits/stl_uninitialized.h: Do not include
>> bits/stl_algobase.h.
>> (__uninitialized_default_1, __uninitialized_default_n_1):
>> Remove.
>> (__uninitialized_default, __uninitialized_default_n): Use
>> 'if constexpr' and only consider triviality constructibility
>> not assignability when deciding on the algorithm to use.
>> ---
>>
>> Tested x86_64-linux.
>
> Only one stylitic comment.
>>
>>
>> libstdc++-v3/include/bits/stl_uninitialized.h | 136 ++++++------------
>> 1 file changed, 42 insertions(+), 94 deletions(-)
>>
>> diff --git a/libstdc++-v3/include/bits/stl_uninitialized.h
>> b/libstdc++-v3/include/bits/stl_uninitialized.h
>> index bde787c2beaa..d5a399e16c90 100644
>> --- a/libstdc++-v3/include/bits/stl_uninitialized.h
>> +++ b/libstdc++-v3/include/bits/stl_uninitialized.h
>> @@ -60,7 +60,6 @@
>> # include <type_traits>
>> # include <bits/ptr_traits.h> // to_address
>> # include <bits/stl_pair.h> // pair
>> -# include <bits/stl_algobase.h> // fill, fill_n
>> #endif
>>
>> #include <bits/cpp_type_traits.h> // __is_pointer
>> @@ -829,92 +828,36 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> // Extensions: __uninitialized_default, __uninitialized_default_n,
>> // __uninitialized_default_a, __uninitialized_default_n_a.
>>
>> - template<bool _TrivialValueType>
>> - struct __uninitialized_default_1
>> - {
>> - template<typename _ForwardIterator>
>> - _GLIBCXX26_CONSTEXPR
>> - static void
>> - __uninit_default(_ForwardIterator __first, _ForwardIterator __last)
>> - {
>> - _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> - for (; __first != __last; ++__first)
>> - std::_Construct(std::addressof(*__first));
>> - __guard.release();
>> - }
>> - };
>> -
>> - template<>
>> - struct __uninitialized_default_1<true>
>> - {
>> - template<typename _ForwardIterator>
>> - _GLIBCXX26_CONSTEXPR
>> - static void
>> - __uninit_default(_ForwardIterator __first, _ForwardIterator __last)
>> - {
>> - if (__first == __last)
>> - return;
>> -
>> - typename iterator_traits<_ForwardIterator>::value_type* __val
>> - = std::addressof(*__first);
>> - std::_Construct(__val);
>> - if (++__first != __last)
>> - std::fill(__first, __last, *__val);
>> - }
>> - };
>> -
>> - template<bool _TrivialValueType>
>> - struct __uninitialized_default_n_1
>> - {
>> - template<typename _ForwardIterator, typename _Size>
>> - _GLIBCXX20_CONSTEXPR
>> - static _ForwardIterator
>> - __uninit_default_n(_ForwardIterator __first, _Size __n)
>> - {
>> - _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> - for (; __n > 0; --__n, (void) ++__first)
>> - std::_Construct(std::addressof(*__first));
>> - __guard.release();
>> - return __first;
>> - }
>> - };
>> -
>> - template<>
>> - struct __uninitialized_default_n_1<true>
>> - {
>> - template<typename _ForwardIterator, typename _Size>
>> - _GLIBCXX20_CONSTEXPR
>> - static _ForwardIterator
>> - __uninit_default_n(_ForwardIterator __first, _Size __n)
>> - {
>> - if (__n > 0)
>> - {
>> - typename iterator_traits<_ForwardIterator>::value_type* __val
>> - = std::addressof(*__first);
>> - std::_Construct(__val);
>> - ++__first;
>> - __first = std::fill_n(__first, __n - 1, *__val);
>> - }
>> - return __first;
>> - }
>> - };
>> -
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wc++17-extensions"
>> // __uninitialized_default
>> // Fills [first, last) with value-initialized value_types.
>> template<typename _ForwardIterator>
>> _GLIBCXX26_CONSTEXPR
>> inline void
>> - __uninitialized_default(_ForwardIterator __first,
>> - _ForwardIterator __last)
>> + __uninitialized_default(_ForwardIterator __first, _ForwardIterator
>> __last)
>> {
>> - typedef typename iterator_traits<_ForwardIterator>::value_type
>> - _ValueType;
>> - // trivial types can have deleted assignment
>> - const bool __assignable = is_copy_assignable<_ValueType>::value;
>> + using _ValueType = typename
>> iterator_traits<_ForwardIterator>::value_type;
>>
>> - std::__uninitialized_default_1<__is_trivial(_ValueType)
>> - && __assignable>::
>> - __uninit_default(__first, __last);
>> + if constexpr (__and_<is_trivially_default_constructible<_ValueType>,
>> +
>> is_trivially_copy_constructible<_ValueType>>::value)
>> + {
>> + if (__first == __last)
>> + return;
>
> This is different from _n, I would do them consistently and have:
> if (!std::__is_constant_evaluated() && __first != __last)
That would mean repeating the __first != __last check below when we
get to the loop and my intention was that once we enter the `if
constexpr (trivial)` block we always return early ... but I just
noticed that I failed to add a return after calling
std::uninitialized_fill - oops! So it always falls through to the loop
and checks __first != __last again anyway.
I'll send a new patch ...
>>
>> + if (!std::__is_constant_evaluated())
>> + {
>> + auto* __addr = std::addressof(*__first);
>> + std::_Construct(__addr);
>> + const auto& __val = *__addr;
>> + if (++__first != __last)
>> + std::uninitialized_fill(__first, __last, __val);
>> + }
>> + }
>> +
>> + _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> + for (; __first != __last; ++__first)
>> + std::_Construct(std::addressof(*__first));
>> + __guard.release();
>> }
>>
>> // __uninitialized_default_n
>> @@ -924,23 +867,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> inline _ForwardIterator
>> __uninitialized_default_n(_ForwardIterator __first, _Size __n)
>> {
>> -#ifdef __cpp_lib_is_constant_evaluated
>> - if (std::is_constant_evaluated())
>> - return __uninitialized_default_n_1<false>::
>> - __uninit_default_n(__first, __n);
>> -#endif
>> + using _ValueType = typename
>> iterator_traits<_ForwardIterator>::value_type;
>>
>> - typedef typename iterator_traits<_ForwardIterator>::value_type
>> - _ValueType;
>> - // See uninitialized_fill_n for the conditions for using std::fill_n.
>> - constexpr bool __can_fill
>> - = __and_<is_integral<_Size>, is_copy_assignable<_ValueType>>::value;
>> + if constexpr (__and_<is_trivially_default_constructible<_ValueType>,
>> + is_trivially_copy_constructible<_ValueType>,
>> + is_integral<_Size>>::value)
>> + {
>
> We do not need nested braces hre.
True.
>>
>> + if (!std::__is_constant_evaluated() && __n > 0)
>> + {
>>
>> + auto* __addr = std::addressof(*__first);
>> + std::_Construct(__addr);
>> + const auto& __val = *__addr;
>> + return std::uninitialized_fill_n(++__first, --__n, __val);
>> + }
>> + }
>>
>> - return __uninitialized_default_n_1<__is_trivial(_ValueType)
>> - && __can_fill>::
>> - __uninit_default_n(__first, __n);
>> + _UninitDestroyGuard<_ForwardIterator> __guard(__first);
>> + for (; __n > 0; --__n, (void) ++__first)
>> + std::_Construct(std::addressof(*__first));
>> + __guard.release();
>> + return __first;
>> }
>> -
>> +#pragma GCC diagnostic pop
>>
>> // __uninitialized_default_a
>> // Fills [first, last) with value_types constructed by the allocator
>> --
>> 2.49.0
>>
