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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
