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

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

Wow, thanks for the quick response! I really appreciate it.

In D114732#3159247 <https://reviews.llvm.org/D114732#3159247>, @Quuxplusone wrote:

> - (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).

I agree with you that `[[clang::trivial_abi]]` could be taken not to imply `[[trivially_relocatable]]`. However, I also think we can choose for it to imply it -- the meaning of `[[clang::trivial_abi]]` is up to us! It does not seem very **useful** to have a type which is `trivial_abi`, but not trivially relocatable: such a type can be relocated when being passed by value, but is leaving performance on the floor everywhere else. It might be that such a type really depends on the number of paired constructor/destructor calls, but this seems unlikely to be a realistic concern, especially since the number of paired constructor/destructor calls can change as a result of implementation changes in e.g. `std::vector`'s reallocation algorithm anyway.

As you noted, "nothing was ever done about that". This change seeks to address that problem. :B

> 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.

Leaving aside differences of opinion on how important it is, a version of the standards proposal could maintain linear independence, though not orthogonality: we could imagine writing `struct [[clang::trivial_abi]] [[trivially_relocatable(false]] Foo { ... };` and the like to achieve every combination.

I do agree that it's not true that every trivially relocatable type should be trivial for calls. This can produce bad performance outcomes in the case of e.g. non-inline functions which accept the trivial-for-calls value by pointer/reference, such as methods. Calling those can force the value to be put back in the stack, adding additional (trivial) copies where a non-trivial type would've had none. And so, even with this change, we would still want to be able to specify trivial relocatability separately, without implying trivial-for-calls.

A bigger problem is that my perception from reading that review thread was that any `[[trivially_relocatable]]` attribute proposal was a non-starter: this is the subject of a standards proposal which has not yet been accepted and clang doesn't want to "pick a winner". So my hope is that rather than picking a winner there, we extend the behavior of `trivial_abi`, in a way that is compatible with and desirable to have in combination with any accepted standards proposal. Hopefully, by being a bit less ambitious and complete, and reducing scope only to changes in behavior for `trivial_abi`, the resulting change to clang will be more acceptable.

This does mean that some performance is left on the floor -- any types which can't be marked `trivial_abi` will not get the benefit of trivial relocatability -- but that's what the standards proposal can aim to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114732