Better framework for conditional cleanups; untested as yet.

I'm separately committing this because it incidentally changes some
block orderings and minor IR issues, like using a phi instead of
an unnecessary alloca.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@124277 91177308-0d34-0410-b5e6-96231b3b80d8

9 files changed, 297 insertions, 83 deletions

diff --git a/lib/CodeGen/CGException.cpp b/lib/CodeGen/CGException.cpp
index 78655bdc11..5845f1ec02 100644
--- a/lib/CodeGen/CGException.cpp
+++ b/lib/CodeGen/CGException.cpp
@@ -172,6 +172,22 @@ void EHScopeStack::popNullFixups() {
     BranchFixups.pop_back();
 }
 
+llvm::Value *CodeGenFunction::initFullExprCleanup() {
+  // Create a variable to decide whether the cleanup needs to be run.
+  llvm::AllocaInst *run = CreateTempAlloca(Builder.getInt1Ty(), "cleanup.cond");
+
+  // Initialize it to false at a site that's guaranteed to be run
+  // before each evaluation.
+  llvm::BasicBlock *block = OutermostConditional->getStartingBlock();
+  new llvm::StoreInst(Builder.getFalse(), run,
+                      block->getFirstNonPHIOrDbg());
+
+  // Initialize it to true at the current location.
+  Builder.CreateStore(Builder.getTrue(), run);
+
+  return run;
+}
+
 static llvm::Constant *getAllocateExceptionFn(CodeGenFunction &CGF) {
   // void *__cxa_allocate_exception(size_t thrown_size);
   const llvm::Type *SizeTy = CGF.ConvertType(CGF.getContext().getSizeType());
@@ -1656,3 +1672,23 @@ CodeGenFunction::UnwindDest CodeGenFunction::getRethrowDest() {
 EHScopeStack::Cleanup::~Cleanup() {
   llvm_unreachable("Cleanup is indestructable");
 }
+
+void EHScopeStack::ConditionalCleanup::Emit(CodeGenFunction &CGF,
+                                            bool IsForEHCleanup) {
+  // Determine whether we should run the cleanup.
+  llvm::Value *condVal = CGF.Builder.CreateLoad(cond, "cond.should-run");
+
+  llvm::BasicBlock *cleanup = CGF.createBasicBlock("cond-cleanup.run");
+  llvm::BasicBlock *cont = CGF.createBasicBlock("cond-cleanup.cont");
+
+  // If we shouldn't run the cleanup, jump directly to the continuation block.
+  CGF.Builder.CreateCondBr(condVal, cleanup, cont);
+  CGF.EmitBlock(cleanup);
+
+  // Emit the core of the cleanup.
+  EmitImpl(CGF, IsForEHCleanup);
+  assert(CGF.HaveInsertPoint() && "cleanup didn't end with valid IP!");
+
+  // Fall into the continuation block.
+  CGF.EmitBlock(cont);
+}
diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 8e5ee75972..35c2a2992b 100644
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -1692,6 +1692,8 @@ CodeGenFunction::EmitConditionalOperatorLValue(const ConditionalOperator *E) {
     llvm::BasicBlock *LHSBlock = createBasicBlock("cond.true");
     llvm::BasicBlock *RHSBlock = createBasicBlock("cond.false");
     llvm::BasicBlock *ContBlock = createBasicBlock("cond.end");
+
+    ConditionalEvaluation eval(*this);
     
     if (E->getLHS())
       EmitBranchOnBoolExpr(E->getCond(), LHSBlock, RHSBlock);
@@ -1705,35 +1707,34 @@ CodeGenFunction::EmitConditionalOperatorLValue(const ConditionalOperator *E) {
     }
     
     // Any temporaries created here are conditional.
-    BeginConditionalBranch();
     EmitBlock(LHSBlock);
+    eval.begin(*this);
     LValue LHS = EmitLValue(E->getTrueExpr());
-
-    EndConditionalBranch();
+    eval.end(*this);
     
     if (!LHS.isSimple())
       return EmitUnsupportedLValue(E, "conditional operator");
 
-    // FIXME: We shouldn't need an alloca for this.
-    llvm::Value *Temp = CreateTempAlloca(LHS.getAddress()->getType(),"condtmp");
-    Builder.CreateStore(LHS.getAddress(), Temp);
-    EmitBranch(ContBlock);
+    LHSBlock = Builder.GetInsertBlock();
+    Builder.CreateBr(ContBlock);
     
     // Any temporaries created here are conditional.
-    BeginConditionalBranch();
     EmitBlock(RHSBlock);
+    eval.begin(*this);
     LValue RHS = EmitLValue(E->getRHS());
-    EndConditionalBranch();
+    eval.end(*this);
     if (!RHS.isSimple())
       return EmitUnsupportedLValue(E, "conditional operator");
-
-    Builder.CreateStore(RHS.getAddress(), Temp);
-    EmitBranch(ContBlock);
+    RHSBlock = Builder.GetInsertBlock();
 
     EmitBlock(ContBlock);
-    
-    Temp = Builder.CreateLoad(Temp, "lv");
-    return MakeAddrLValue(Temp, E->getType());
+
+    llvm::PHINode *phi = Builder.CreatePHI(LHS.getAddress()->getType(),
+                                           "cond-lvalue");
+    phi->reserveOperandSpace(2);
+    phi->addIncoming(LHS.getAddress(), LHSBlock);
+    phi->addIncoming(RHS.getAddress(), RHSBlock);
+    return MakeAddrLValue(phi, E->getType());
   }
   
   // ?: here should be an aggregate.
diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp
index 0369077797..3a758a8102 100644
--- a/lib/CodeGen/CGExprAgg.cpp
+++ b/lib/CodeGen/CGExprAgg.cpp
@@ -367,8 +367,8 @@ void AggExprEmitter::VisitBinComma(const BinaryOperator *E) {
 }
 
 void AggExprEmitter::VisitStmtExpr(const StmtExpr *E) {
+  CodeGenFunction::StmtExprEvaluation eval(CGF);
   CGF.EmitCompoundStmt(*E->getSubStmt(), true, Dest);
-  CGF.EnsureInsertPoint();
 }
 
 void AggExprEmitter::VisitBinaryOperator(const BinaryOperator *E) {
@@ -423,20 +423,19 @@ void AggExprEmitter::VisitConditionalOperator(const ConditionalOperator *E) {
   llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("cond.false");
   llvm::BasicBlock *ContBlock = CGF.createBasicBlock("cond.end");
 
+  CodeGenFunction::ConditionalEvaluation eval(CGF);
   CGF.EmitBranchOnBoolExpr(E->getCond(), LHSBlock, RHSBlock);
 
-  CGF.BeginConditionalBranch();
-  CGF.EmitBlock(LHSBlock);
-
   // Save whether the destination's lifetime is externally managed.
   bool DestLifetimeManaged = Dest.isLifetimeExternallyManaged();
 
+  eval.begin(CGF);
+  CGF.EmitBlock(LHSBlock);
   Visit(E->getLHS());
-  CGF.EndConditionalBranch();
-  CGF.EmitBranch(ContBlock);
+  eval.end(CGF);
 
-  CGF.BeginConditionalBranch();
-  CGF.EmitBlock(RHSBlock);
+  assert(CGF.HaveInsertPoint() && "expression evaluation ended with no IP!");
+  CGF.Builder.CreateBr(ContBlock);
 
   // If the result of an agg expression is unused, then the emission
   // of the LHS might need to create a destination slot.  That's fine
@@ -444,9 +443,10 @@ void AggExprEmitter::VisitConditionalOperator(const ConditionalOperator *E) {
   // we shouldn't claim that its lifetime is externally managed.
   Dest.setLifetimeExternallyManaged(DestLifetimeManaged);
 
+  eval.begin(CGF);
+  CGF.EmitBlock(RHSBlock);
   Visit(E->getRHS());
-  CGF.EndConditionalBranch();
-  CGF.EmitBranch(ContBlock);
+  eval.end(CGF);
 
   CGF.EmitBlock(ContBlock);
 }
diff --git a/lib/CodeGen/CGExprComplex.cpp b/lib/CodeGen/CGExprComplex.cpp
index bdd245414b..21c298a96a 100644
--- a/lib/CodeGen/CGExprComplex.cpp
+++ b/lib/CodeGen/CGExprComplex.cpp
@@ -325,9 +325,8 @@ ComplexPairTy ComplexExprEmitter::VisitCallExpr(const CallExpr *E) {
 }
 
 ComplexPairTy ComplexExprEmitter::VisitStmtExpr(const StmtExpr *E) {
-  RValue result = CGF.EmitCompoundStmt(*E->getSubStmt(), true);
-  CGF.EnsureInsertPoint();
-  return result.getComplexVal();
+  CodeGenFunction::StmtExprEvaluation eval(CGF);
+  return CGF.EmitCompoundStmt(*E->getSubStmt(), true).getComplexVal();
 }
 
 /// EmitComplexToComplexCast - Emit a cast from complex value Val to DestType.
@@ -647,29 +646,32 @@ VisitConditionalOperator(const ConditionalOperator *E) {
   llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("cond.false");
   llvm::BasicBlock *ContBlock = CGF.createBasicBlock("cond.end");
 
+  CodeGenFunction::ConditionalEvaluation eval(CGF);
+
   if (E->getLHS())
     CGF.EmitBranchOnBoolExpr(E->getCond(), LHSBlock, RHSBlock);
   else {
     Expr *save = E->getSAVE();
     assert(save && "VisitConditionalOperator - save is null");
-    // Intentianlly not doing direct assignment to ConditionalSaveExprs[save] !!
+    // Intentionally not doing direct assignment to ConditionalSaveExprs[save] !!
     ComplexPairTy SaveVal = Visit(save);
     CGF.ConditionalSaveComplexExprs[save] = SaveVal;
     CGF.EmitBranchOnBoolExpr(E->getCond(), LHSBlock, RHSBlock);
   }
 
+  eval.begin(CGF);
   CGF.EmitBlock(LHSBlock);
   ComplexPairTy LHS = Visit(E->getTrueExpr());
   LHSBlock = Builder.GetInsertBlock();
   CGF.EmitBranch(ContBlock);
+  eval.end(CGF);
 
+  eval.begin(CGF);
   CGF.EmitBlock(RHSBlock);
-
   ComplexPairTy RHS = Visit(E->getRHS());
   RHSBlock = Builder.GetInsertBlock();
-  CGF.EmitBranch(ContBlock);
-
   CGF.EmitBlock(ContBlock);
+  eval.end(CGF);
 
   // Create a PHI node for the real part.
   llvm::PHINode *RealPN = Builder.CreatePHI(LHS.first->getType(), "cond.r");
diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index 745757977a..096430e5ce 100644
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -1204,10 +1204,9 @@ Value *ScalarExprEmitter::EmitCastExpr(CastExpr *CE) {
 }
 
 Value *ScalarExprEmitter::VisitStmtExpr(const StmtExpr *E) {
-  RValue value = CGF.EmitCompoundStmt(*E->getSubStmt(),
-                                      !E->getType()->isVoidType());
-  CGF.EnsureInsertPoint();
-  return value.getScalarVal();
+  CodeGenFunction::StmtExprEvaluation eval(CGF);
+  return CGF.EmitCompoundStmt(*E->getSubStmt(), !E->getType()->isVoidType())
+    .getScalarVal();
 }
 
 Value *ScalarExprEmitter::VisitBlockDeclRefExpr(const BlockDeclRefExpr *E) {
@@ -2226,6 +2225,8 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) {
   llvm::BasicBlock *ContBlock = CGF.createBasicBlock("land.end");
   llvm::BasicBlock *RHSBlock  = CGF.createBasicBlock("land.rhs");
 
+  CodeGenFunction::ConditionalEvaluation eval(CGF);
+
   // Branch on the LHS first.  If it is false, go to the failure (cont) block.
   CGF.EmitBranchOnBoolExpr(E->getLHS(), RHSBlock, ContBlock);
 
@@ -2239,10 +2240,10 @@ Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) {
        PI != PE; ++PI)
     PN->addIncoming(llvm::ConstantInt::getFalse(VMContext), *PI);
 
-  CGF.BeginConditionalBranch();
+  eval.begin(CGF);
   CGF.EmitBlock(RHSBlock);
   Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS());
-  CGF.EndConditionalBranch();
+  eval.end(CGF);
 
   // Reaquire the RHS block, as there may be subblocks inserted.
   RHSBlock = Builder.GetInsertBlock();
@@ -2276,6 +2277,8 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) {
   llvm::BasicBlock *ContBlock = CGF.createBasicBlock("lor.end");
   llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("lor.rhs");
 
+  CodeGenFunction::ConditionalEvaluation eval(CGF);
+
   // Branch on the LHS first.  If it is true, go to the success (cont) block.
   CGF.EmitBranchOnBoolExpr(E->getLHS(), ContBlock, RHSBlock);
 
@@ -2289,13 +2292,13 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) {
        PI != PE; ++PI)
     PN->addIncoming(llvm::ConstantInt::getTrue(VMContext), *PI);
 
-  CGF.BeginConditionalBranch();
+  eval.begin(CGF);
 
   // Emit the RHS condition as a bool value.
   CGF.EmitBlock(RHSBlock);
   Value *RHSCond = CGF.EvaluateExprAsBool(E->getRHS());
 
-  CGF.EndConditionalBranch();
+  eval.end(CGF);
 
   // Reaquire the RHS block, as there may be subblocks inserted.
   RHSBlock = Builder.GetInsertBlock();
@@ -2425,6 +2428,8 @@ VisitConditionalOperator(const ConditionalOperator *E) {
   llvm::BasicBlock *LHSBlock = CGF.createBasicBlock("cond.true");
   llvm::BasicBlock *RHSBlock = CGF.createBasicBlock("cond.false");
   llvm::BasicBlock *ContBlock = CGF.createBasicBlock("cond.end");
+
+  CodeGenFunction::ConditionalEvaluation eval(CGF);
   
   // If we don't have the GNU missing condition extension, emit a branch on bool
   // the normal way.
@@ -2456,24 +2461,20 @@ VisitConditionalOperator(const ConditionalOperator *E) {
     Builder.CreateCondBr(CondBoolVal, LHSBlock, RHSBlock);
   }
 
-  CGF.BeginConditionalBranch();
   CGF.EmitBlock(LHSBlock);
-
-  // Handle the GNU extension for missing LHS.
+  eval.begin(CGF);
   Value *LHS = Visit(E->getTrueExpr());
+  eval.end(CGF);
 
-  CGF.EndConditionalBranch();
   LHSBlock = Builder.GetInsertBlock();
-  CGF.EmitBranch(ContBlock);
+  Builder.CreateBr(ContBlock);
 
-  CGF.BeginConditionalBranch();
   CGF.EmitBlock(RHSBlock);
-
+  eval.begin(CGF);
   Value *RHS = Visit(E->getRHS());
-  CGF.EndConditionalBranch();
-  RHSBlock = Builder.GetInsertBlock();
-  CGF.EmitBranch(ContBlock);
+  eval.end(CGF);
 
+  RHSBlock = Builder.GetInsertBlock();
   CGF.EmitBlock(ContBlock);
 
   // If the LHS or RHS is a throw expression, it will be legitimately null.
diff --git a/lib/CodeGen/CGTemporaries.cpp b/lib/CodeGen/CGTemporaries.cpp
index 6dd3a2e8bd..cfbd2af990 100644
--- a/lib/CodeGen/CGTemporaries.cpp
+++ b/lib/CodeGen/CGTemporaries.cpp
@@ -60,7 +60,7 @@ void CodeGenFunction::EmitCXXTemporary(const CXXTemporary *Temporary,
 
   // Check if temporaries need to be conditional. If so, we'll create a
   // condition boolean, initialize it to 0 and
-  if (ConditionalBranchLevel != 0) {
+  if (isInConditionalBranch()) {
     CondPtr = CreateTempAlloca(llvm::Type::getInt1Ty(VMContext), "cond");
 
     // Initialize it to false. This initialization takes place right after
diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp
index 8575846162..3923816a18 100644
--- a/lib/CodeGen/CodeGenFunction.cpp
+++ b/lib/CodeGen/CodeGenFunction.cpp
@@ -41,7 +41,7 @@ CodeGenFunction::CodeGenFunction(CodeGenModule &cgm)
     SwitchInsn(0), CaseRangeBlock(0),
     DidCallStackSave(false), UnreachableBlock(0),
     CXXThisDecl(0), CXXThisValue(0), CXXVTTDecl(0), CXXVTTValue(0),
-    ConditionalBranchLevel(0), TerminateLandingPad(0), TerminateHandler(0),
+    OutermostConditional(0), TerminateLandingPad(0), TerminateHandler(0),
     TrapBB(0) {
       
   // Get some frequently used types.
@@ -447,13 +447,15 @@ void CodeGenFunction::EmitBranchOnBoolExpr(const Expr *Cond,
       // Emit the LHS as a conditional.  If the LHS conditional is false, we
       // want to jump to the FalseBlock.
       llvm::BasicBlock *LHSTrue = createBasicBlock("land.lhs.true");
+
+      ConditionalEvaluation eval(*this);
       EmitBranchOnBoolExpr(CondBOp->getLHS(), LHSTrue, FalseBlock);
       EmitBlock(LHSTrue);
 
       // Any temporaries created here are conditional.
-      BeginConditionalBranch();
+      eval.begin(*this);
       EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock);
-      EndConditionalBranch();
+      eval.end(*this);
 
       return;
     } else if (CondBOp->getOpcode() == BO_LOr) {
@@ -474,13 +476,15 @@ void CodeGenFunction::EmitBranchOnBoolExpr(const Expr *Cond,
       // Emit the LHS as a conditional.  If the LHS conditional is true, we
       // want to jump to the TrueBlock.
       llvm::BasicBlock *LHSFalse = createBasicBlock("lor.lhs.false");
+
+      ConditionalEvaluation eval(*this);
       EmitBranchOnBoolExpr(CondBOp->getLHS(), TrueBlock, LHSFalse);
       EmitBlock(LHSFalse);
 
       // Any temporaries created here are conditional.
-      BeginConditionalBranch();
+      eval.begin(*this);
       EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock);
-      EndConditionalBranch();
+      eval.end(*this);
 
       return;
     }
@@ -500,11 +504,20 @@ void CodeGenFunction::EmitBranchOnBoolExpr(const Expr *Cond,
       // br(c ? x : y, t, f) -> br(c, br(x, t, f), br(y, t, f))
       llvm::BasicBlock *LHSBlock = createBasicBlock("cond.true");
       llvm::BasicBlock *RHSBlock = createBasicBlock("cond.false");
+
+      ConditionalEvaluation cond(*this);
       EmitBranchOnBoolExpr(CondOp->getCond(), LHSBlock, RHSBlock);
+
+      cond.begin(*this);
       EmitBlock(LHSBlock);
       EmitBranchOnBoolExpr(CondOp->getLHS(), TrueBlock, FalseBlock);
+      cond.end(*this);
+
+      cond.begin(*this);
       EmitBlock(RHSBlock);
       EmitBranchOnBoolExpr(CondOp->getRHS(), TrueBlock, FalseBlock);
+      cond.end(*this);
+
       return;
     }
   }
diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 7686b33365..ea362d3bf1 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -97,6 +97,24 @@ struct BranchFixup {
   llvm::BranchInst *InitialBranch;
 };
 
+/// A metaprogramming class which decides whether a type is a subclass
+/// of llvm::Value that needs to be saved if it's used in a
+/// conditional cleanup.
+template
+  <class T,
+   bool mustSave =
+     llvm::is_base_of<llvm::Value, llvm::remove_pointer<T> >::value
+     && !llvm::is_base_of<llvm::Constant, llvm::remove_pointer<T> >::value
+     && !llvm::is_base_of<llvm::BasicBlock, llvm::remove_pointer<T> >::value>
+struct SavedValueInCond {
+  typedef T type;
+  typedef T saved_type;
+  static bool needsSaving(type value) { return false; }
+  static saved_type save(CodeGenFunction &CGF, type value) { return value; }
+  static type restore(CodeGenFunction &CGF, saved_type value) { return value; }
+};
+// Partial specialization for true arguments at end of file.
+
 enum CleanupKind {
   EHCleanup = 0x1,
   NormalCleanup = 0x2,
@@ -175,6 +193,52 @@ public:
     virtual void Emit(CodeGenFunction &CGF, bool IsForEHCleanup) = 0;
   };
 
+  /// A helper class for cleanups that execute conditionally.
+  class ConditionalCleanup : public Cleanup {
+    /// Either an i1 which directly indicates whether the cleanup
+    /// should be run or an i1* from which that should be loaded.
+    llvm::Value *cond;
+
+  public:
+    virtual void Emit(CodeGenFunction &CGF, bool IsForEHCleanup);
+
+  protected:
+    ConditionalCleanup(llvm::Value *cond) : cond(cond) {}
+
+    /// Emit the non-conditional code for the cleanup.
+    virtual void EmitImpl(CodeGenFunction &CGF, bool IsForEHCleanup) = 0;
+  };
+
+  /// UnconditionalCleanupN stores its N parameters and just passes
+  /// them to the real cleanup function.
+  template <class T, class A0, class A1>
+  class UnconditionalCleanup2 : public Cleanup {
+    A0 a0; A1 a1;
+    UnconditionalCleanup2(A0 a0, A1 a1) : a0(a0), a1(a1) {}
+    void Emit(CodeGenFunction &CGF, bool IsForEHCleanup) {
+      T::Emit(CGF, IsForEHCleanup, a0, a1);
+    }
+  };
+
+  /// ConditionalCleanupN stores the saved form of its N parameters,
+  /// then restores them and performs the cleanup.
+  template <class T, class A0, class A1>
+  class ConditionalCleanup2 : public ConditionalCleanup {
+    typedef typename SavedValueInCond<A0>::saved_type A0_saved;
+    typedef typename SavedValueInCond<A1>::saved_type A1_saved;
+    A0_saved a0; A1_saved a1;
+
+    void EmitImpl(CodeGenFunction &CGF, bool IsForEHCleanup) {
+      A0 a0 = SavedValueInCond<A0>::restore(CGF, a0);
+      A1 a1 = SavedValueInCond<A1>::restore(CGF, a1);
+      T::Emit(CGF, IsForEHCleanup, a0, a1);
+    }
+
+  public:
+    ConditionalCleanup2(llvm::Value *cond, A0_saved a0, A1_saved a1)
+      : ConditionalCleanup(cond), a0(a0), a1(a1) {}
+  };
+
 private:
   // The implementation for this class is in CGException.h and
   // CGException.cpp; the definition is here because it's used as a
@@ -536,6 +600,14 @@ public:
 
   llvm::BasicBlock *getInvokeDestImpl();
 
+  /// Sets up a condition for a full-expression cleanup.
+  llvm::Value *initFullExprCleanup();
+
+  template <class T>
+  typename SavedValueInCond<T>::saved_type saveValueInCond(T value) {
+    return SavedValueInCond<T>::save(*this, value);
+  }
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
@@ -552,6 +624,28 @@ public:
                                 llvm::Constant *RethrowFn);
   void ExitFinallyBlock(FinallyInfo &FinallyInfo);
 
+  /// pushFullExprCleanup - Push a cleanup to be run at the end of the
+  /// current full-expression.  Safe against the possibility that
+  /// we're currently inside a conditionally-evaluated expression.
+  template <class T, class A0, class A1>
+  void pushFullExprCleanup(CleanupKind kind, A0 a0, A1 a1) {
+    // If we're not in a conditional branch, or if none of the
+    // arguments requires saving, then use the unconditional cleanup.
+    if (!(isInConditionalBranch() ||
+          SavedValueInCond<A0>::needsSaving(a0) ||
+          SavedValueInCond<A1>::needsSaving(a1))) {
+      typedef EHScopeStack::UnconditionalCleanup2<T, A0, A1> CleanupType;
+      return EHStack.pushCleanup<CleanupType>(kind, a0, a1);
+    }
+
+    llvm::Value *condVar = initFullExprCleanup();
+    typename SavedValueInCond<A0>::saved_type a0_saved = saveValueInCond(a0);
+    typename SavedValueInCond<A1>::saved_type a1_saved = saveValueInCond(a1);
+
+    typedef EHScopeStack::ConditionalCleanup2<T, A0, A1> CleanupType;
+    EHStack.pushCleanup<CleanupType>(kind, condVar, a0_saved, a1_saved);
+  }
+
   /// PushDestructorCleanup - Push a cleanup to call the
   /// complete-object destructor of an object of the given type at the
   /// given address.  Does nothing if T is not a C++ class type with a
@@ -659,30 +753,58 @@ public:
   /// destination.
   UnwindDest getRethrowDest();
 
-  /// BeginConditionalBranch - Should be called before a conditional part of an
-  /// expression is emitted. For example, before the RHS of the expression below
-  /// is emitted:
-  ///
-  /// b && f(T());
-  ///
-  /// This is used to make sure that any temporaries created in the conditional
-  /// branch are only destroyed if the branch is taken.
-  void BeginConditionalBranch() {
-    ++ConditionalBranchLevel;
-  }
+  /// An object to manage conditionally-evaluated expressions.
+  class ConditionalEvaluation {
+    llvm::BasicBlock *StartBB;
 
-  /// EndConditionalBranch - Should be called after a conditional part of an
-  /// expression has been emitted.
-  void EndConditionalBranch() {
-    assert(ConditionalBranchLevel != 0 &&
-           "Conditional branch mismatch!");
+  public:
+    ConditionalEvaluation(CodeGenFunction &CGF)
+      : StartBB(CGF.Builder.GetInsertBlock()) {}
 
-    --ConditionalBranchLevel;
-  }
+    void begin(CodeGenFunction &CGF) {
+      assert(CGF.OutermostConditional != this);
+      if (!CGF.OutermostConditional)
+        CGF.OutermostConditional = this;
+    }
+
+    void end(CodeGenFunction &CGF) {
+      assert(CGF.OutermostConditional != 0);
+      if (CGF.OutermostConditional == this)
+        CGF.OutermostConditional = 0;
+    }
+
+    /// Returns a block which will be executed prior to each
+    /// evaluation of the conditional code.
+    llvm::BasicBlock *getStartingBlock() const {
+      return StartBB;
+    }
+  };
 
   /// isInConditionalBranch - Return true if we're currently emitting
   /// one branch or the other of a conditional expression.
-  bool isInConditionalBranch() const { return ConditionalBranchLevel != 0; }
+  bool isInConditionalBranch() const { return OutermostConditional != 0; }
+
+  /// An RAII object to record that we're evaluating a statement
+  /// expression.
+  class StmtExprEvaluation {
+    CodeGenFunction &CGF;
+
+    /// We have to save the outermost conditional: cleanups in a
+    /// statement expression aren't conditional just because the
+    /// StmtExpr is.
+    ConditionalEvaluation *SavedOutermostConditional;
+
+  public:
+    StmtExprEvaluation(CodeGenFunction &CGF)
+      : CGF(CGF), SavedOutermostConditional(CGF.OutermostConditional) {
+      CGF.OutermostConditional = 0;
+    }
+
+    ~StmtExprEvaluation() {
+      CGF.OutermostConditional = SavedOutermostConditional;
+      CGF.EnsureInsertPoint();
+    }
+  };
   
   /// getByrefValueFieldNumber - Given a declaration, returns the LLVM field
   /// number that holds the value.
@@ -750,10 +872,10 @@ private:
   ImplicitParamDecl *CXXVTTDecl;
   llvm::Value *CXXVTTValue;
 
-  /// ConditionalBranchLevel - Contains the nesting level of the current
-  /// conditional branch. This is used so that we know if a temporary should be
-  /// destroyed conditionally.
-  unsigned ConditionalBranchLevel;
+  /// OutermostConditional - Points to the outermost active
+  /// conditional control.  This is used so that we know if a
+  /// temporary should be destroyed conditionally.
+  ConditionalEvaluation *OutermostConditional;
 
 
   /// ByrefValueInfoMap - For each __block variable, contains a pair of the LLVM
@@ -1786,6 +1908,48 @@ private:
   void EmitDeclMetadata();
 };
 
+/// Helper class with most of the code for saving a value for a
+/// conditional expression cleanup.
+struct SavedValueInCondImpl {
+  typedef llvm::PointerIntPair<llvm::Value*, 1, bool> saved_type;
+
+  /// Answer whether the given value needs extra work to be saved.
+  static bool needsSaving(llvm::Value *value) {
+    // If it's not an instruction, we don't need to save.
+    if (!isa<llvm::Instruction>(value)) return false;
+
+    // If it's an instruction in the entry block, we don't need to save.
+    llvm::BasicBlock *block = cast<llvm::Instruction>(value)->getParent();
+    return (block != &block->getParent()->getEntryBlock());
+  }
+
+  /// Try to save the given value.
+  static saved_type save(CodeGenFunction &CGF, llvm::Value *value) {
+    if (!needsSaving(value)) return saved_type(value, false);
+
+    // Otherwise we need an alloca.
+    llvm::Value *alloca =
+      CGF.CreateTempAlloca(value->getType(), "cond-cleanup.save");
+    CGF.Builder.CreateStore(value, alloca);
+
+    return saved_type(alloca, true);
+  }
+
+  static llvm::Value *restore(CodeGenFunction &CGF, saved_type value) {
+    if (!value.getInt()) return value.getPointer();
+    return CGF.Builder.CreateLoad(value.getPointer());
+  }
+};
+
+/// Partial specialization of SavedValueInCond for when a value really
+/// requires saving.
+template <class T> struct SavedValueInCond<T,true> : SavedValueInCondImpl {
+  typedef T type;
+  static type restore(CodeGenFunction &CGF, saved_type value) {
+    return static_cast<T>(SavedValueInCondImpl::restore(CGF, value));
+  }
+};
+
 /// CGBlockInfo - Information to generate a block literal.
 class CGBlockInfo {
 public:
diff --git a/test/CodeGenCXX/volatile-1.cpp b/test/CodeGenCXX/volatile-1.cpp
index 0569150101..3ae17bd41b 100644
--- a/test/CodeGenCXX/volatile-1.cpp
+++ b/test/CodeGenCXX/volatile-1.cpp
@@ -143,20 +143,17 @@ void test() {
   // CHECK-NEXT: volatile load
   // CHECK-NEXT: volatile store
 
-  // FIXME: the phi-equivalent is unnecessary
   k ? (i=i) : (j=j);
   // CHECK-NEXT: volatile load
   // CHECK-NEXT: icmp
   // CHECK-NEXT: br i1
   // CHECK: volatile load
   // CHECK-NEXT: volatile store
-  // CHECK-NEXT: store [[INT]]* @i
   // CHECK-NEXT: br label
   // CHECK: volatile load
   // CHECK-NEXT: volatile store
-  // CHECK-NEXT: store [[INT]]* @j
   // CHECK-NEXT: br label
-  // CHECK: load [[INT]]**
+  // CHECK:      phi
 
   (void)(i,(i=i));
   // CHECK-NEXT: volatile load