[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

On the one hand, I like that someone else is pushing a patch that's basically the same as D50119 <https://reviews.llvm.org/D50119>, because maybe if enough people reimplement the same thing, eventually Clang will accept one of them. ;)
However, I'd like to point out:

- D50119 <https://reviews.llvm.org/D50119> has more extensive tests, and has been live on https://p1144.godbolt.org/z/axx9Wj3r5 for a couple years now; if the consensus is that Clang wants "part of D50119 <https://reviews.llvm.org/D50119> but not all of it," then IMHO it would be reasonable to put some comments on D50119 <https://reviews.llvm.org/D50119> with the goal of stripping it //down//, rather than trying to build //up// a smaller version of it in parallel.
- (Major point of contention) To me, `[[trivial_abi]]` does not imply `[[trivially_relocatable]]` at all. I believe @rjmccall disagreed a few years ago: basically I said "The two attributes are orthogonal," with practical demonstration, and basically he said "yes, in practice you're right, but philosophically that's a bug and we do actually //intend// `[[trivial_abi]]` to imply `[[trivially_relocatable]]` even though the compiler doesn't enforce it" (but then AFAIK nothing was ever done about that).

Here are my practical examples: https://p1144.godbolt.org/z/EYcMM1nTW
With `ATTR=[[clang::trivial_abi]]`, we see that `take(g)` passes in a register; but notice that it still calls the copy-ctor, and it will still call the dtor (inside `take` — the ABI changes to "callee-destroy" for arguments of these types). And we see that `std::swap` is completely unaffected: we know that the calling convention for passing `T` by value is altered, but we don't know anything //semantic// about the behavior of `T` itself (e.g. when it is relocated or swapped).
With `ATTR=[[clang::trivially_relocatable]]` (D50119 <https://reviews.llvm.org/D50119>), we see that `take(g)` still passes on the stack (p1144 trivial relocation is backward-compatible with C++20 by design: it doesn't alter the calling convention at all). But notice that because we know something //semantic// about the behavior of `T` (namely, that it is trivially relocatable), `std::swap` is able to skip calling its ctors and dtors and just swap the bits directly.

Therefore, I support something close to this patch, but I strongly believe that you need to split up the "trivially relocatable" attribute itself from "trivial abi". They are represented orthogonally in the AST, and they should be toggleable orthogonally by the end-user. The end-user might want a trivial-abi type that is not trivially-relocatable, or a trivially-relocatable type that is not trivial-abi.

Related reading: https://quuxplusone.github.io/blog/2018/05/02/trivial-abi-101/
Related watching, on p1144 `[[trivially_relocatable]]` and adjacent topics such as `trivial_abi` and `move_relocates`: https://www.youtube.com/watch?v=SGdfPextuAU


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114732/new/

https://reviews.llvm.org/D114732