rsmith added inline comments.

================
Comment at: lib/CodeGen/CGCall.cpp:3498
   bool HasAggregateEvalKind = hasAggregateEvaluationKind(type);
+  bool TypeDestructedInCallee = false;
+  if (const auto *RD = type->getAsCXXRecordDecl())
----------------
This is a confusing name for this flag, because there are other reasons why we 
can choose to destroy in the callee (specifically the MS ABI reason). Rather, I 
think this is capturing that "this parameter's ABI was affected by a 
`trivial_abi` attribute", which implies that it should be destroyed (only) in 
the callee. Maybe calling this something like `HasTrivialABIOverride` would 
help.


================
Comment at: lib/CodeGen/CGDecl.cpp:1822
+  bool IsScalar = hasScalarEvaluationKind(Ty);
+  bool TypeDestructedInCallee = false;
+  if (const auto *RD = Ty->getAsCXXRecordDecl())
----------------
Same comment regarding this name.


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:833
     // passed in registers, which is non-conforming.
     if (RD->hasNonTrivialDestructor() &&
         getContext().getTypeSize(RD->getTypeForDecl()) > 64)
----------------
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.


https://reviews.llvm.org/D41039



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to