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