On 19/07/18 07:59 -0400, Glen Fernandes wrote:
Updated patch to simplify the helper trait, and to include <memory>
instead of <algorithm> in the unit test for copy_uninitialized:
Use __builtin_memmove for trivially copy assignable types
2018-07-19 Glen Joseph Fernandes <glenj...@gmail.com>
* include/bits/stl_algobase.h
(__is_simple_copy_move): Defined helper.
(__copy_move_a): Used helper.
(__copy_move_backward_a): Likewise.
* testsuite/20_util/specialized_algorithms/uninitialized_copy/1.cc:
New test.
* testsuite/25_algorithms/copy/58982.cc: Updated tests.
* testsuite/25_algorithms/copy_n/58982.cc: Likewise.
Attached: patch.txt
Glen
commit 1af8d465545fda2451928fe100901db37c3e632c
Author: Glen Fernandes <glen.fernan...@gmail.com>
Date: Thu Jul 19 07:40:17 2018 -0400
Use __builtin_memmove for trivially copy assignable types
2018-07-19 Glen Joseph Fernandes <glenj...@gmail.com>
* include/bits/stl_algobase.h
(__is_simple_copy_move): Defined helper.
(__copy_move_a): Used helper.
(__copy_move_backward_a): Likewise.
* testsuite/20_util/specialized_algorithms/uninitialized_copy/1.cc:
New test.
* testsuite/25_algorithms/copy/58982.cc: Updated tests.
* testsuite/25_algorithms/copy_n/58982.cc: Likewise.
diff --git a/libstdc++-v3/include/bits/stl_algobase.h
b/libstdc++-v3/include/bits/stl_algobase.h
index 16a3f83b6..4488207f0 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -72,10 +72,16 @@
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
+ template<typename _Tp>
+ struct __is_simple_copy_move
+ {
+ enum { __value = __is_trivially_assignable(_Tp, const _Tp&) };
+ };
+
#if __cplusplus < 201103L
// See http://gcc.gnu.org/ml/libstdc++/2004-08/msg00167.html: in a
// nutshell, we are partially implementing the resolution of DR 187,
// when it's safe, i.e., the value_types are equal.
template<bool _BoolType>
@@ -389,11 +395,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__copy_move_a(_II __first, _II __last, _OI __result)
{
typedef typename iterator_traits<_II>::value_type _ValueTypeI;
typedef typename iterator_traits<_OI>::value_type _ValueTypeO;
typedef typename iterator_traits<_II>::iterator_category _Category;
- const bool __simple = (__is_trivial(_ValueTypeI)
+ const bool __simple = (__is_simple_copy_move<_ValueTypeI>::__value
Sorry for the delay in reviewing this properly, as I've only just
realised that this introduces undefined behaviour, doesn't it?
It's undefined to use memmove for a type that is not trivially
copyable. All trivial types are trivially copyable, so __is_trivial
was too conservative, but safe (IIRC we used it because there was no
__is_trivially_copyable trait at the time, so __is_trivial was the
best we had).
There are types which are trivially assignable but not trivially
copyable, and it's undefined to use memmove for such types. With your
patch applied I get a warning for this code, where there was none
before:
#include <memory>
#include <type_traits>
struct T
{
T() { }
T(const T&) { }
};
static_assert(std::is_trivially_copy_assignable<T>::value
&& !std::is_trivially_copyable<T>::value,
"T is only trivially copy assignable, not trivially copyable");
void
test01(T* result)
{
T t[1];
std::copy(t, t+1, result);
}
In file included from /home/jwakely/gcc/9/include/c++/9.0.0/memory:62,
from copy.cc:1:
/home/jwakely/gcc/9/include/c++/9.0.0/bits/stl_algobase.h: In instantiation of
'static _Tp* std::__copy_move<_IsMove, true,
std::random_access_iterator_tag>::__copy_m(const _Tp*, const _Tp*, _Tp*) [with _Tp
= T; bool _IsMove = false]':
/home/jwakely/gcc/9/include/c++/9.0.0/bits/stl_algobase.h:406:30: required
from '_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II =
T*; _OI = T*]'
/home/jwakely/gcc/9/include/c++/9.0.0/bits/stl_algobase.h:443:30: required
from '_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II =
T*; _OI = T*]'
/home/jwakely/gcc/9/include/c++/9.0.0/bits/stl_algobase.h:476:7: required
from '_OI std::copy(_II, _II, _OI) [with _II = T*; _OI = T*]'
copy.cc:18:27: required from here
/home/jwakely/gcc/9/include/c++/9.0.0/bits/stl_algobase.h:388:23: warning:
'void* __builtin_memmove(void*, const void*, long unsigned int)' writing to an
object of non-trivially copyable type 'struct T'; use copy-assignment or
copy-initialization instead [-Wclass-memaccess]
__builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
copy.cc:4:8: note: 'struct T' declared here
struct T
^
I think the best we can do here is simply replace __is_trivial with
__is_trivially_copyable, which will enable memmove for trivially
copyable types for which !is_trivially_default_constructible_v<T>.
