[PATCH] D41039: Add support for attribute "trivial_abi"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak marked an inline comment as done.
ahatanak added inline comments.


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:833
     // passed in registers, which is non-conforming.
     if (RD->hasNonTrivialDestructor() &&
         getContext().getTypeSize(RD->getTypeForDecl()) > 64)
----------------
rsmith wrote:
> Should we be checking `hasNonTrivialDestructorForCalls` here? What should happen if we have a small class whose copy constructor is trivial for calls but whose destructor is not? The existing machinations of this ABI only work because they can make an additional trivial copy of the function parameter when passing it direct, *even if* it has a non-trivial destructor. I don't think that's correct in general if the copy constructor is only trival-for-calls. Consider:
> 
> ```
> struct X {
>   shared_ptr<T> sp; // shared_ptr<T> is trivial-for-calls
>   ~X();
> };
> ```
> 
> In the absence of the destructor, X should be passed direct.
> In the presence of the destructor, though, passing direct would result in the X destructor running in both caller and callee, and X being passed through registers without running the copy constructor. Which results in a double-delete.
> 
> So I think what we want here is this:
> 
>  * If the copy ctor and dtor are both trivial-for-calls, pass direct.
>  * Otherwise, if the copy ctor is trivial (not just trivial-for-calls) and the object is small, pass direct (regardless of the triviality of the dtor -- we'll make a copy, notionally using the trivial copy ctor, and destroy both the original and the copy).
>  * Otherwise, pass indirect.
I'm not very familiar with microsoft's ABI, but I think that makes sense.


https://reviews.llvm.org/D41039