[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Arthur O'Dwyer from comment #6)
> What do you mean "if an exception is thrown"? If we call std::copy from
> here, then it's because we are taking the memmove path.

The whole point of the bug is the conditions for which we call std::copy. My
comment is discussing when we can and can't call it. You seem to be assuming
that we've already got the conditions correct and therefore the conditions are
already correct. That's a tautology.

If we get the conditions wrong, then "it's because we are taking the memmove
path" might be false. We might end up calling a specialization of std::copy
that decides not to memmove.

So when discussing the conditions when it's valid to call std::copy, it's worth
enumerating the requirements on std::uninitialized_copy and how they differ
from std::copy. 

But as I said right after the part you quoted, trivially copyable implies
trivially destructible, so the trivially destructible requirement is trivially
met.

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[barry.revzin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

Barry Revzin <barry.revzin at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |barry.revzin at gmail dot com

--- Comment #8 from Barry Revzin <barry.revzin at gmail dot com> ---
Here's an example:

struct A { int i; };
struct B { int i{}; };

Both are trivially copyable, A is additionally trivially default constructible
while B is not. This distinction is irrelevant in this case - we're copying,
and that does not involve default constructing an A or B (trivial or
otherwise).

But this means that __is_trivial(B) is false, so we don't go into std::copy, so
these do different things:

#include <memory>

struct A { int i; };
struct B { int i{}; };

void copy_a(A* f, A* l, A* out) {
    std::uninitialized_copy(f, l, out);
}

void copy_b(B* f, B* l, B* out) {
    std::uninitialized_copy(f, l, out);
}

emits (with g++ 11.2, -std=c++20 -O3, https://godbolt.org/z/eG9hsbcTE):

copy_a(A*, A*, A*):
        mov     r8, rdi
        mov     rdi, rdx
        cmp     r8, rsi
        je      .L1
        mov     rdx, rsi
        mov     rsi, r8
        sub     rdx, r8
        jmp     memmove
.L1:
        ret
copy_b(B*, B*, B*):
        cmp     rdi, rsi
        je      .L4
        sub     rsi, rdi
        xor     eax, eax
.L6:
        mov     ecx, DWORD PTR [rdi+rax]
        mov     DWORD PTR [rdx+rax], ecx
        add     rax, 4
        cmp     rax, rsi
        jne     .L6
.L4:
        ret

Whereas the copy_b case could also use memmove.

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|ville.voutilainen at gmail dot com |redi at gcc dot gnu.org

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Barry Revzin from comment #8)
> Whereas the copy_b case could also use memmove.

I'd been working on the assumption that it can't, because for types that aren't
trivially default constructible we need a constructor to begin the lifetime.
But the rules for implicit-lifetime class types only require at least one
trivial constructor and a trivial destructor, so a trivial copy constructor is
enough. And memcpy/memmove implicitly create objects in the destination
storage.

I don't think that was really explicit before the implicit-lifetime rules in
C++20. The example in C++17 [basic.types] p3 implies that the destination
doesn't need to be an initialized object, but nothing said that normatively.
Quite the opposite: C++17 [basic.life] seems clear that initialisation must be
done for types with non-vacuous initialization (such as Barry's type B), and
nothing in C++17 said that memcpy/memmove do any initialization. C++20 is clear
though.

So:

- If the input and output type are the same size and both are trivially
  constructible, we can use memcpy. We should call memcpy directly from
  std::uninitialized_copy rather than via std::copy, because std::copy doesn't
  allow overlapping ranges, and has to use memmove instead.

- If the output type is trivially default constructible and is assignable from
  the input type, we can use std::copy (but it's unclear whether that gives any
  benefit if we can't turn it into memcpy/memmove).

- Otherwise, just use std::construct_at on each element.

I have a patch, but it's for stage 1 not GCC 12.

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #9)
> - If the input and output type are the same size and both are trivially
>   constructible, we can use memcpy.

Correction: they need to be the same type. We can't memcpy here:

struct A { };
struct B { B() = default; B(A) { do_stuff(); } };

void (A* f, A* l, B* out) {
  std::uninitialized_copy(f, l, out);
}

Both types are trivially copyable, but using memcpy is only valid for a single
type.

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #9)
> - If the output type is trivially default constructible and is assignable
> from
>   the input type, we can use std::copy (but it's unclear whether that gives
> any
>   benefit if we can't turn it into memcpy/memmove).

It's also not clear that it would be allowed.

struct A { };
struct B {
  B() = default;
  B(A) { do_stuff(); }
  B& operator=(A) { do_other_stuff(); }
};

void (A* f, A* l, B* out) {
  std::uninitialized_copy(f, l, out);
}

It's observable whether we construct B from A, or default construct B and then
assign to it from A.

My patch just removes all uses of std::copy/fill/fill_n in the
std::uninitialized_copy/fill/fill_n algos.

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[arthur.j.odwyer at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

--- Comment #12 from Arthur O'Dwyer <arthur.j.odwyer at gmail dot com> ---
jwakely wrote:
> Correction: they need to be the same type. We can't memcpy here:
>
> struct A { };
> struct B { B() = default; B(A) { do_stuff(); } };
>
> void (A* f, A* l, B* out) {
>   std::uninitialized_copy(f, l, out);
> }

Right, in your case, the ctor `B(A)` runs user-defined code (it's not
"trivial") so obviously we can't do the optimization. But even in general, see
my 2018 blog post "Trivially-constructible-from."
https://quuxplusone.github.io/blog/2018/07/03/trivially-constructible-from/

  using A2 = long long;
  using B2 = int64_t;  // for the sake of argument, this is "long int"
  void (A* f, A* l, B* out) {
    std::uninitialized_copy(f, l, out);
  }

The library can't, by itself, determine that memcpy would be safe here. The
library needs help from the compiler, e.g. via a new compiler builtin
__is_trivially_constructible_from(T, U). (This is *not* the same as the
existing `is_trivially_constructible<T,U>` type trait, because blog post.)

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|13.0                        |13.2

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 13.1 is being released, retargeting bugs to GCC 13.2.

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|13.2                        |13.3

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 13.2 is being released, retargeting bugs to GCC 13.3.

[Bug libstdc++/68350] std::uninitialized_copy overly restrictive for trivially_copyable types

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68350

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|13.3                        |13.4

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 13.3 is being released, retargeting bugs to GCC 13.4.