Quuxplusone added inline comments.
================ Comment at: clang/include/clang/Basic/AttrDocs.td:3211-3215 +The compiler will pass and return a trivially relocatable type using the C ABI +for the underlying type, even when the type would otherwise be considered +non-trivially-relocatable. If a type is trivially relocatable, has a +non-trivial destructor, and is passed as an argument by value, the convention +is that the callee will destroy the object before returning. ---------------- rsmith wrote: > Quuxplusone wrote: > > devin.jeanpierre wrote: > > > rsmith wrote: > > > > I think this documentation change has mixed together two different > > > > things. The revised wording says that `[[trivial_abi]]` implies > > > > trivially-relocatable and that trivially-relocatable implies passing > > > > using the C ABI, which is wrong: the second implication does not hold. > > > > What we should say is that `[[trivial_abi]]` (if we're not in the "has > > > > no effect" case described below) implies both that the type is > > > > trivially-relocatable, and that it is passed using the C ABI (that is, > > > > that we trivially relocate in argument passing). > > > > > > > > Instead of the wording changes you have here, perhaps we could leave > > > > the old wording alone and add a paragraph that says that a type with > > > > the attribute is assumed to be trivially-relocatable for the purpose of > > > > `__is_trivially_relocatable`, but that Clang doesn't yet take advantage > > > > of this fact anywhere other than argument passing. > > > Yeah, this was too aggressive a wording change, and might easily change > > > later when `[[trivially_relocatable]]` is added. > > > > > > I did drop the "Clang doesn't yet take advantage of this fact anywhere > > > other than argument passing.", because I feel like -- does that sort of > > > sentiment belong instead in the docs for `__is_trivially_relocatable`? > > > Maybe I should change `__is_trivially_relocatable` to note that this is > > > never taken advantage of anywhere except in by-value argument passing > > > (when the type is trivial for the purpose of calls) and library code. > > Richard's comment jogged my brain and made we worry about this again. IIUC, > > you're saying: > > > > 1. All trivial-abi types //are in fact// trivially relocated, specifically > > when they are passed to functions. > > 2. Therefore, all trivial-abi types are "trivially relocatable" //in > > general//. > > 3. Therefore, all trivial-abi types are safe to optimize in all the ways > > depicted in D67524. > > > > I agree with 1 for sure, but I'm skeptical of the logical soundness of 2 > > and 3. Are we willing to set it in stone? > > > > If we're //not// willing to set that whole syllogism in stone, then I would > > object to this patch. Because of //this// syllogism: > > > > 4. This patch makes Clang's `__is_trivially_relocatable(T)` return `true` > > for all trivial-abi types. > > 5. D67524 (rightly) applies its optimizations to all types for which > > `__is_trivially_relocatable(T)` is `true`. > > 6. Therefore, if we adopt this patch, we'd better be dang sure that all > > trivial-abi types are safe to optimize in all the ways depicted in D67524. > > > > That is, we must set in stone the idea that all trivial-abi types can > > safely: > > - be swapped trivially, in terms of bytewise swap, inside e.g. std::sort > > - use trivial (bytewise) relocation inside vector reallocation > > - use trivial (bytewise) relocation when shifting inside vector::insert or > > vector::erase > > > > If @devin.jeanpierre and @rsmith and @rjmccall and @ahatanak are all > > nodding along to this and saying "yeah, totally, every premise above is > > true and the syllogisms are all valid and those optimizations are all > > valid, for every trivial-abi type, forever," then I'm happy. > What use cases would we be giving up on if we go this way? What would a type > look like for which trivial relocation when passing to functions is correct, > but for which trivial relocation in general is either incorrect or > undesirable? >! @rsmith wrote: > What would a type look like for which trivial relocation when passing to > functions is correct, but for which trivial relocation in general is either > incorrect or undesirable? I have no examples in mind myself, but that (and my skepticism) are largely due to my lack of practical experience with `[[trivial_abi]]`. If I can't come up with any //and// you can't come up with any //and//... etc..., then that's good news for this PR. >! @rjmccall wrote: > > I am perfectly happy accepting that syllogism. [...] If there is a reason > your type [ever] shouldn't be trivially relocated, you should not make it > trivial_abi. Okay, that's good news for this PR also. :) > Trivial relocation doesn't imply that types have to be safe against being > suddenly relocated during the middle of operations while they're not in a > safe internal state. That is not a consideration. +100. 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
