[PATCH] [ms-cxxabi] Construct and destroy call arguments in the correct order

[Reid Kleckner]

================
Comment at: lib/CodeGen/CGCall.cpp:1249
@@ -1248,1 +1248,3 @@
 
+  // Holds defered cleanups issued in the prologue.
+  SmallVector<std::pair<llvm::Value *, QualType>, 2> PrologueCleanups;
----------------
Richard Smith wrote:
> Two 'r's in deferred, please =)
Done.

================
Comment at: lib/CodeGen/CGCall.cpp:2032-2033
@@ +2031,4 @@
+
+  // We *have* to evaluate arguments from right to left in the MS C++ ABI,
+  // because arguments are destroyed left to right in the callee.
+  if (CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) {
----------------
Richard Smith wrote:
> There are other places that need updating for this: at least `EmitCXXNewExpr`, not sure if there are others.
There was another in `EmitSynthesizedCXXCopyCtorCall`.  I templated EmitCallArgs over the argument type iterator to handle all these cases.  It ends up duplicating some work when the prototype is unknown, but it saves a lot of work when callers need to emit special handling for early arguments.

================
Comment at: lib/CodeGen/CGCall.cpp:2038
@@ +2037,3 @@
+      CallExpr::const_arg_iterator Arg = ArgBeg + I;
+      EmitCallArg(Args, *Arg, ArgTypes[I]);
+      // Restore the debug location.
----------------
Richard Smith wrote:
> I assume you're going to need to update this again to perform all the allocas for a call, and then emit all the arguments, once we support `inalloca` here? (Otherwise the stack layout will be backwards.)
Yes.  My current plan is to save the insertion point before we begin evaluating, and then go back and insert the stacksave and allocas at the appropriate place if we needed to use inalloca.  This matches how we emit allocas into the entry block, except I'm not adding to the entry block.

This is kind of gross and overly imperative though.  Emitting the allocas up front might be cleaner.

================
Comment at: lib/CodeGen/CGDecl.cpp:1649
@@ -1647,3 +1648,3 @@
     DeclPtr = Arg;
     // Push a destructor cleanup for this parameter if the ABI requires it.
     if (HasNonScalarEvalKind &&
----------------
Richard Smith wrote:
> Hmm, why do we only handle destructor cleanups in the callee-cleanup code? Do we have some common code we could reuse to perform the appropriate cleanup for the ARC cases too?
Unsurprisingly, I haven't tested reverse argument evaluation with ARC.  =/

Getting that right would require doing a full reversal of the prologue emission loop.  It's hard to reverse that loop because of the way that it's iterating the LLVM IR args in parallel.  I was trying to sidestep that by assuming there'd be no unwinding or cleanups emitted elsewhere in the prologue, but that's incorrect.

I don't think this is OK to commit without fixing that.


http://llvm-reviews.chandlerc.com/D2275