[PATCH] D30590: Don't assume cleanup emission preserves dominance in expr evaluation

[Richard Smith via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Hal and I discussed exactly the same problem (in the context of coroutines) on Saturday and came up with exactly the same approach :) I think this is the right direction.



================
Comment at: lib/CodeGen/CGExprComplex.cpp:204-206
+    Scope.ensureDominatingValue(&Vals.first);
+    Scope.ensureDominatingValue(&Vals.second);
+    Scope.ForceCleanup();
----------------
I'm a little concerned about the loose connection between `ensureDominatingValue` and `ForrceCleanup` here -- if you forget the `ForceCleanup`, you get silent misbehavior. How about removing `ensureDominatingValue` and instead passing a `std::initializer_list<Value**>` to `ForceCleanup`?


================
Comment at: lib/CodeGen/CodeGenFunction.h:539
   private:
+    SmallVector<llvm::Value **, 1> ValuesToReload;
 
----------------
If you keep a `SmallVector` here, set its inline size to 2 to avoid allocations for the `_Complex` case.


https://reviews.llvm.org/D30590