[WinEH] Demote values live across exception handlers up front

[rnk]

Summary

In particular, this handles SSA values that are live *out* of a handler.
The existing code only handles values that are live *in* to a handler.

[majnemer]

lib/CodeGen/WinEHPrepare.cpp

22 ↗

(On Diff #24141)

Please sort this include with the others.

388 ↗

(On Diff #24141)

auto?

452–457 ↗

(On Diff #24141)

Is it possible to demote a dynamic alloca in the exceptional path?

521–522 ↗

(On Diff #24141)

You could make this just:

if (match(&Inst, m_Intrinsic<Intrinsic::eh_actions>())
  return false;

1468–1469 ↗

(On Diff #24141)

Wrap these in DEBUG?

[rnk]

lib/CodeGen/WinEHPrepare.cpp

452–457 ↗

(On Diff #24141)

Yes, it will produce a dangling pointer. It has the same semantics as the source one would write for it:

void f() {
  int *data = nullptr;
  try {
    throw 42;
  } catch (int e) {
    data = (int*)alloca(e);
  }
  data[0] = 42; // store to dead stack memory
}

Rejecting this isn't our job, it's Clang's.

521–522 ↗

(On Diff #24141)

Won't that require that the intrinsic call has no arguments?

1468–1469 ↗

(On Diff #24141)

Let's make this errs(). I really want this check to stay in for a release build. It's not expensive, and it's the #1 place where outlining can fall down and go off the rails. We should emit a reasonable crash diagnostic.

[rnk]

• Address comments

[majnemer]

lib/CodeGen/WinEHPrepare.cpp

452–457 ↗

(On Diff #24141)

What if the programmer wasn't at fault?

Let's say we had:

int n;
void f() {
  int *x = (int *)alloca(n);
  try {
    throw 42;
  } catch (int e) {
    do_something(x);
    return;
  }
}

Is it unreasonable for LLVM to sink the dynamic alloca into the catch block?

521–522 ↗

(On Diff #24141)

No: http://llvm.org/docs/doxygen/html/PatternMatch_8h_source.html#l01193

1468–1469 ↗

(On Diff #24141)

Sounds good.

[andrew.w.kaylor]

lib/CodeGen/WinEHPrepare.cpp

383 ↗

(On Diff #24148)

Can you add a comment here explaining that the blocks have been split to ensure that the endcatch call will immediately precede the terminator?

427 ↗

(On Diff #24148)

This doesn't seem right. A nested landing pad block generally begins with a call to endcatch, but it isn't a normal block.

460 ↗

(On Diff #24148)

Does this demote arguments in EH blocks even if the value is defined in an EH block?

Also, isn't this going to demote the landing pad values which we will later promote?

508 ↗

(On Diff #24148)

Is this inserting the store in the entry block? That doesn't sound right.

[rnk]

The next patch has a lot of changes because I realized I had to do more work for phi nodes using constants in EH return points.

lib/CodeGen/WinEHPrepare.cpp

383 ↗

(On Diff #24148)

Sure.

427 ↗

(On Diff #24148)

I think we should stop emitting those endcatch calls in the frontend. I don't think they help identify regions to outline. They way I see it, barring code commoning optimizations, the handlers are just the BBs delimited by the begincatch / endcatch calls. A nested landingpad simply starts a new set of handlers.

460 ↗

(On Diff #24148)

This all runs before outlining, so the only arguments it will demote are ones in the parent function.

This shouldn't demote selector and ehptr values because they typically aren't live *out* of a landingpad. Even if we demote them by accident in a corner case, I'm not worried about the overhead, so long as we don't demote and then promote in the common cases.

508 ↗

(On Diff #24148)

Why not? Arguments are SSA values that are defined once at function entry. This is only less than optimal when you have paths through the function that don't reach an invoke with a handler using the argument.

We could try to sink the store somewhere closer to a dominating invoke to stay out of any fast paths through the funciton, but we'd want to stay hoisted out of any loop bodies. This analysis seems hard and not really worth it. If you're still concerned, an easier direction might be to have llvm.argument.escape and llvm.argument.recover intrinsics to recover the address of the argument and ensure that the parent homes escaped register parameters.

452–457 ↗

(On Diff #24141)

I think in general that's too hard for LLVM. If you add a cleanup to your example, do_something(x) could escape x, and then the cleanup would run in the parent frame and use the escaped x, which is now dead.

What's wrong with leaving it alone and calling DemoteRegToStack on the dynamic alloca? If it doesn't do the right thing (I haven't tested it) it sounds like a bug in DemoteRegToStack.

521–522 ↗

(On Diff #24141)

done

[rnk]

• [WinEH] Split edges representing returns from exception handlers

[rnk]

• Demote landingpad phis, they are also not real

[andrew.w.kaylor]

lib/CodeGen/WinEHPrepare.cpp

427 ↗

(On Diff #24148)

That's a further divergence from the Itanium model, which I believe does need that call, but I guess we've got enough of those now that one more wouldn't matter.

We're jumping through a fair number of hoops to maintain at least a semblance of conformance to the basic structure of the Itanium model. My understanding is that the primary benefit of this is that the back end passes know how to handle landing pads and we don't need to teach them something new. Is that right? I suppose it doesn't matter much which intrinsics we use along the way.

508 ↗

(On Diff #24148)

Oh, nevermind. I was seeing 'argument' but thinking 'operand' because of where this set was originally populated, which also explains the irrelevancy of my comment above. Sorry about that.

[rnk]

lib/CodeGen/WinEHPrepare.cpp

427 ↗

(On Diff #24148)

Yes, it is another Itanium divergence. We probably want to rename these intrinsics, as they are pretty WinEH specific at this point.

By backend passes, I'm assuming you mean the LLVM IR mid-level optimization passes? Everything in lib/CodeGen had to be redone, so we didn't save anything there. At this point, now that I can see the difficulties entailed in implementing WinEHPrepare, I'm happy to move to some other mid-level representation that doesn't resemble Itanium EH, even if we have to change the mid-level passes somewhat.

The main thing that I want to preserve is that we should outline late to support optimization. If we can't run SROA on objects with destructors because they are all escaped to an uninlineable call to the destructor on the exceptional codepath, all follow-on optimization is dead in the water. I don't want to have to make an interprocedural version of SROA or teach SROA to be (slightly) interprocedural. If we can't run SROA on objects with destructors, we will never be able to promote the heap memory from unique_ptr<int> to an i32 alloca by matching up the new/delete pairs, for example.

Ultimately, I think we can save ourselves the Itanium-style EH hoop jumping by switching to landingswitch (alternatively named "dispatchblock"), or something like it.

[andrew.w.kaylor]

I have one question, but I expect you have a good answer.

Looks good.

lib/CodeGen/WinEHPrepare.cpp

417 ↗

(On Diff #24172)

Does this put the endcatch call in the split successor or leave it in the original block?

[rnk]

lib/CodeGen/WinEHPrepare.cpp

417 ↗

(On Diff #24172)

Good catch! This code splits before the call, but I didn't want that. It's not easy to observe in a test because we remove these calls and branches after outlining.

llvm/trunk/lib/CodeGen/WinEHPrepare.cpp

 // this pass delegates to the DWARF EH preparation pass.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Analysis/LibCallSemantics.h"
 #include "llvm/CodeGen/WinEHFuncInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
public:
   const char *getPassName() const override {
     return "Windows exception handling preparation";
   }
 
 private:
   bool prepareExceptionHandlers(Function &F,
                                 SmallVectorImpl<LandingPadInst *> &LPads);
   void promoteLandingPadValues(LandingPadInst *LPad);
+  void demoteValuesLiveAcrossHandlers(Function &F,
+                                      SmallVectorImpl<LandingPadInst *> &LPads);
   void completeNestedLandingPad(Function *ParentFn,
                                 LandingPadInst *OutlinedLPad,
                                 const LandingPadInst *OriginalLPad,
                                 FrameVarInfoMap &VarInfo);
   bool outlineHandler(ActionHandler *Action, Function *SrcFn,
                       LandingPadInst *LPad, BasicBlock *StartBB,
                       FrameVarInfoMap &VarInfo);
   void addStubInvokeToHandlerIfNeeded(Function *Handler, Value *PersonalityFn);

 }
 
 // FIXME: Remove this once the backend can handle the prepared IR.
 static cl::opt<bool>
     SEHPrepare("sehprepare", cl::Hidden,
                cl::desc("Prepare functions with SEH personalities"));
 
 bool WinEHPrepare::runOnFunction(Function &Fn) {
+  // No need to prepare outlined handlers.
+  if (Fn.hasFnAttribute("wineh-parent"))
+    return false;
+
   SmallVector<LandingPadInst *, 4> LPads;
   SmallVector<ResumeInst *, 4> Resumes;
   for (BasicBlock &BB : Fn) {
     if (auto *LP = BB.getLandingPadInst())
       LPads.push_back(LP);
     if (auto *Resume = dyn_cast<ResumeInst>(BB.getTerminator()))
       Resumes.push_back(Resume);
   }

 }
 
 bool WinEHPrepare::doFinalization(Module &M) { return false; }
 
 void WinEHPrepare::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<DominatorTreeWrapperPass>();
 }
 
+static bool isSelectorDispatch(BasicBlock *BB, BasicBlock *&CatchHandler,
+                               Constant *&Selector, BasicBlock *&NextBB);
+
+// Finds blocks reachable from the starting set Worklist. Does not follow unwind
+// edges or blocks listed in StopPoints.
+static void findReachableBlocks(SmallPtrSetImpl<BasicBlock *> &ReachableBBs,
+                                SetVector<BasicBlock *> &Worklist,
+                                const SetVector<BasicBlock *> *StopPoints) {
+  while (!Worklist.empty()) {
+    BasicBlock *BB = Worklist.pop_back_val();
+
+    // Don't cross blocks that we should stop at.
+    if (StopPoints && StopPoints->count(BB))
+      continue;
+
+    if (!ReachableBBs.insert(BB).second)
+      continue; // Already visited.
+
+    // Don't follow unwind edges of invokes.
+    if (auto *II = dyn_cast<InvokeInst>(BB->getTerminator())) {
+      Worklist.insert(II->getNormalDest());
+      continue;
+    }
+
+    // Otherwise, follow all successors.
+    Worklist.insert(succ_begin(BB), succ_end(BB));
+  }
+}
+
+/// Find all points where exceptional control rejoins normal control flow via
+/// llvm.eh.endcatch. Add them to the normal bb reachability worklist.
+static void findCXXEHReturnPoints(Function &F,
+                                  SetVector<BasicBlock *> &EHReturnBlocks) {
+  for (auto BBI = F.begin(), BBE = F.end(); BBI != BBE; ++BBI) {
+    BasicBlock *BB = BBI;
+    for (Instruction &I : *BB) {
+      if (match(&I, m_Intrinsic<Intrinsic::eh_endcatch>())) {
+        // Split the block after the call to llvm.eh.endcatch if there is
+        // anything other than an unconditional branch, or if the successor
+        // starts with a phi.
+        auto *Br = dyn_cast<BranchInst>(I.getNextNode());
+        if (!Br || !Br->isUnconditional() ||
+            isa<PHINode>(Br->getSuccessor(0)->begin())) {
+          DEBUG(dbgs() << "splitting block " << BB->getName()
+                       << " with llvm.eh.endcatch\n");
+          BBI = BB->splitBasicBlock(I.getNextNode(), "ehreturn");
+        }
+        // The next BB is normal control flow.
+        EHReturnBlocks.insert(BB->getTerminator()->getSuccessor(0));
+        break;
+      }
+    }
+  }
+}
+
+static bool isCatchAllLandingPad(const BasicBlock *BB) {
+  const LandingPadInst *LP = BB->getLandingPadInst();
+  if (!LP)
+    return false;
+  unsigned N = LP->getNumClauses();
+  return (N > 0 && LP->isCatch(N - 1) &&
+          isa<ConstantPointerNull>(LP->getClause(N - 1)));
+}
+
+/// Find all points where exceptions control rejoins normal control flow via
+/// selector dispatch.
+static void findSEHEHReturnPoints(Function &F,
+                                  SetVector<BasicBlock *> &EHReturnBlocks) {
+  for (auto BBI = F.begin(), BBE = F.end(); BBI != BBE; ++BBI) {
+    BasicBlock *BB = BBI;
+    // If the landingpad is a catch-all, treat the whole lpad as if it is
+    // reachable from normal control flow.
+    // FIXME: This is imprecise. We need a better way of identifying where a
+    // catch-all starts and cleanups stop. As far as LLVM is concerned, there
+    // is no difference.
+    if (isCatchAllLandingPad(BB)) {
+      EHReturnBlocks.insert(BB);
+      continue;
+    }
+
+    BasicBlock *CatchHandler;
+    BasicBlock *NextBB;
+    Constant *Selector;
+    if (isSelectorDispatch(BB, CatchHandler, Selector, NextBB)) {
+      // Split the edge if there is a phi node. Returning from EH to a phi node
+      // is just as impossible as having a phi after an indirectbr.
+      if (isa<PHINode>(CatchHandler->begin())) {
+        DEBUG(dbgs() << "splitting EH return edge from " << BB->getName()
+                     << " to " << CatchHandler->getName() << '\n');
+        BBI = CatchHandler = SplitCriticalEdge(
+            BB, std::find(succ_begin(BB), succ_end(BB), CatchHandler));
+      }
+      EHReturnBlocks.insert(CatchHandler);
+    }
+  }
+}
+
+/// Ensure that all values live into and out of exception handlers are stored
+/// in memory.
+/// FIXME: This falls down when values are defined in one handler and live into
+/// another handler. For example, a cleanup defines a value used only by a
+/// catch handler.
+void WinEHPrepare::demoteValuesLiveAcrossHandlers(
+    Function &F, SmallVectorImpl<LandingPadInst *> &LPads) {
+  DEBUG(dbgs() << "Demoting values live across exception handlers in function "
+               << F.getName() << '\n');
+
+  // Build a set of all non-exceptional blocks and exceptional blocks.
+  // - Non-exceptional blocks are blocks reachable from the entry block while
+  //   not following invoke unwind edges.
+  // - Exceptional blocks are blocks reachable from landingpads. Analysis does
+  //   not follow llvm.eh.endcatch blocks, which mark a transition from
+  //   exceptional to normal control.
+  SmallPtrSet<BasicBlock *, 4> NormalBlocks;
+  SmallPtrSet<BasicBlock *, 4> EHBlocks;
+  SetVector<BasicBlock *> EHReturnBlocks;
+  SetVector<BasicBlock *> Worklist;
+
+  if (Personality == EHPersonality::MSVC_CXX)
+    findCXXEHReturnPoints(F, EHReturnBlocks);
+  else
+    findSEHEHReturnPoints(F, EHReturnBlocks);
+
+  DEBUG({
+    dbgs() << "identified the following blocks as EH return points:\n";
+    for (BasicBlock *BB : EHReturnBlocks)
+      dbgs() << "  " << BB->getName() << '\n';
+  });
+
+  // Join points should not have phis at this point, unless they are a
+  // landingpad, in which case we will demote their phis later.
+#ifndef NDEBUG
+  for (BasicBlock *BB : EHReturnBlocks)
+    assert((BB->isLandingPad() || !isa<PHINode>(BB->begin())) &&
+           "non-lpad EH return block has phi");
+#endif
+
+  // Normal blocks are the blocks reachable from the entry block and all EH
+  // return points.
+  Worklist = EHReturnBlocks;
+  Worklist.insert(&F.getEntryBlock());
+  findReachableBlocks(NormalBlocks, Worklist, nullptr);
+  DEBUG({
+    dbgs() << "marked the following blocks as normal:\n";
+    for (BasicBlock *BB : NormalBlocks)
+      dbgs() << "  " << BB->getName() << '\n';
+  });
+
+  // Exceptional blocks are the blocks reachable from landingpads that don't
+  // cross EH return points.
+  Worklist.clear();
+  for (auto *LPI : LPads)
+    Worklist.insert(LPI->getParent());
+  findReachableBlocks(EHBlocks, Worklist, &EHReturnBlocks);
+  DEBUG({
+    dbgs() << "marked the following blocks as exceptional:\n";
+    for (BasicBlock *BB : EHBlocks)
+      dbgs() << "  " << BB->getName() << '\n';
+  });
+
+  SetVector<Argument *> ArgsToDemote;
+  SetVector<Instruction *> InstrsToDemote;
+  for (BasicBlock &BB : F) {
+    bool IsNormalBB = NormalBlocks.count(&BB);
+    bool IsEHBB = EHBlocks.count(&BB);
+    if (!IsNormalBB && !IsEHBB)
+      continue; // Blocks that are neither normal nor EH are unreachable.
+    for (Instruction &I : BB) {
+      for (Value *Op : I.operands()) {
+        // Don't demote static allocas, constants, and labels.
+        if (isa<Constant>(Op) || isa<BasicBlock>(Op) || isa<InlineAsm>(Op))
+          continue;
+        auto *AI = dyn_cast<AllocaInst>(Op);
+        if (AI && AI->isStaticAlloca())
+          continue;
+
+        if (auto *Arg = dyn_cast<Argument>(Op)) {
+          if (IsEHBB) {
+            DEBUG(dbgs() << "Demoting argument " << *Arg
+                         << " used by EH instr: " << I << "\n");
+            ArgsToDemote.insert(Arg);
+          }
+          continue;
+        }
+
+        auto *OpI = cast<Instruction>(Op);
+        BasicBlock *OpBB = OpI->getParent();
+        // If a value is produced and consumed in the same BB, we don't need to
+        // demote it.
+        if (OpBB == &BB)
+          continue;
+        bool IsOpNormalBB = NormalBlocks.count(OpBB);
+        bool IsOpEHBB = EHBlocks.count(OpBB);
+        if (IsNormalBB != IsOpNormalBB || IsEHBB != IsOpEHBB) {
+          DEBUG({
+            dbgs() << "Demoting instruction live in-out from EH:\n";
+            dbgs() << "Instr: " << *OpI << '\n';
+            dbgs() << "User: " << I << '\n';
+          });
+          InstrsToDemote.insert(OpI);
+        }
+      }
+    }
+  }
+
+  // Demote values live into and out of handlers.
+  // FIXME: This demotion is inefficient. We should insert spills at the point
+  // of definition, insert one reload in each handler that uses the value, and
+  // insert reloads in the BB used to rejoin normal control flow.
+  Instruction *AllocaInsertPt = F.getEntryBlock().getFirstInsertionPt();
+  for (Instruction *I : InstrsToDemote)
+    DemoteRegToStack(*I, false, AllocaInsertPt);
+
+  // Demote arguments separately, and only for uses in EH blocks.
+  for (Argument *Arg : ArgsToDemote) {
+    auto *Slot = new AllocaInst(Arg->getType(), nullptr,
+                                Arg->getName() + ".reg2mem", AllocaInsertPt);
+    SmallVector<User *, 4> Users(Arg->user_begin(), Arg->user_end());
+    for (User *U : Users) {
+      auto *I = dyn_cast<Instruction>(U);
+      if (I && EHBlocks.count(I->getParent())) {
+        auto *Reload = new LoadInst(Slot, Arg->getName() + ".reload", false, I);
+        U->replaceUsesOfWith(Arg, Reload);
+      }
+    }
+    new StoreInst(Arg, Slot, AllocaInsertPt);
+  }
+
+  // Demote landingpad phis, as the landingpad will be removed from the machine
+  // CFG.
+  for (LandingPadInst *LPI : LPads) {
+    BasicBlock *BB = LPI->getParent();
+    while (auto *Phi = dyn_cast<PHINode>(BB->begin()))
+      DemotePHIToStack(Phi, AllocaInsertPt);
+  }
+
+  DEBUG(dbgs() << "Demoted " << InstrsToDemote.size() << " instructions and "
+               << ArgsToDemote.size() << " arguments for WinEHPrepare\n\n");
+}
+
 bool WinEHPrepare::prepareExceptionHandlers(
     Function &F, SmallVectorImpl<LandingPadInst *> &LPads) {
+  // Don't run on functions that are already prepared.
+  for (LandingPadInst *LPad : LPads) {
+    BasicBlock *LPadBB = LPad->getParent();
+    for (Instruction &Inst : *LPadBB)
+      if (match(&Inst, m_Intrinsic<Intrinsic::eh_actions>()))
+        return false;
+  }
+
+  demoteValuesLiveAcrossHandlers(F, LPads);
+
   // These containers are used to re-map frame variables that are used in
   // outlined catch and cleanup handlers.  They will be populated as the
   // handlers are outlined.
   FrameVarInfoMap FrameVarInfo;
 
   bool HandlersOutlined = false;
 
   Module *M = F.getParent();
   LLVMContext &Context = M->getContext();
 
   // Create a new function to receive the handler contents.
   PointerType *Int8PtrType = Type::getInt8PtrTy(Context);
   Type *Int32Type = Type::getInt32Ty(Context);
   Function *ActionIntrin = Intrinsic::getDeclaration(M, Intrinsic::eh_actions);
 
   for (LandingPadInst *LPad : LPads) {
     // Look for evidence that this landingpad has already been processed.
     bool LPadHasActionList = false;
     BasicBlock *LPadBB = LPad->getParent();
     for (Instruction &Inst : *LPadBB) {
-      if (auto *IntrinCall = dyn_cast<IntrinsicInst>(&Inst)) {
-        if (IntrinCall->getIntrinsicID() == Intrinsic::eh_actions) {
-          LPadHasActionList = true;
-          break;
-        }
-      }
+      if (match(&Inst, m_Intrinsic<Intrinsic::eh_actions>())) {
       // FIXME: This is here to help with the development of nested landing pad
       //        outlining.  It should be removed when that is finished.
       if (isa<UnreachableInst>(Inst)) {
         LPadHasActionList = true;
         break;
       }
     }
 
for (ActionHandler *Action : Actions) {
         ActionArgs.push_back(ConstantInt::get(Int32Type, 0));
       }
       ActionArgs.push_back(Action->getHandlerBlockOrFunc());
     }
     CallInst *Recover =
         CallInst::Create(ActionIntrin, ActionArgs, "recover", NewLPadBB);
 
     // Add an indirect branch listing possible successors of the catch handlers.
-    IndirectBrInst *Branch = IndirectBrInst::Create(Recover, 0, NewLPadBB);
+    SetVector<BasicBlock *> ReturnTargets;
     for (ActionHandler *Action : Actions) {
       if (auto *CatchAction = dyn_cast<CatchHandler>(Action)) {
-        for (auto *Target : CatchAction->getReturnTargets()) {
-          Branch->addDestination(Target);
-        }
+        const auto &CatchTargets = CatchAction->getReturnTargets();
+        ReturnTargets.insert(CatchTargets.begin(), CatchTargets.end());
       }
     }
+    IndirectBrInst *Branch =
+        IndirectBrInst::Create(Recover, ReturnTargets.size(), NewLPadBB);
+    for (BasicBlock *Target : ReturnTargets)
+      Branch->addDestination(Target);
   } // End for each landingpad
 
   // If nothing got outlined, there is no more processing to be done.
   if (!HandlersOutlined)
     return false;
 
   // Replace any nested landing pad stubs with the correct action handler.
   // This must be done before we remove unreachable blocks because it
if (II && II->getIntrinsicID() == Intrinsic::frameescape) {
       AllocasToEscape.append(Args.begin(), Args.end());
       II->eraseFromParent();
       break;
     }
   }
 
   // Finally, replace all of the temporary allocas for frame variables used in
   // the outlined handlers with calls to llvm.framerecover.
-  BasicBlock::iterator II = Entry->getFirstInsertionPt();
-  Instruction *AllocaInsertPt = II;
   for (auto &VarInfoEntry : FrameVarInfo) {
     Value *ParentVal = VarInfoEntry.first;
     TinyPtrVector<AllocaInst *> &Allocas = VarInfoEntry.second;
-
-    // If the mapped value isn't already an alloca, we need to spill it if it
-    // is a computed value or copy it if it is an argument.
-    AllocaInst *ParentAlloca = dyn_cast<AllocaInst>(ParentVal);
-    if (!ParentAlloca) {
-      if (auto *Arg = dyn_cast<Argument>(ParentVal)) {
-        // Lower this argument to a copy and then demote that to the stack.
-        // We can't just use the argument location because the handler needs
-        // it to be in the frame allocation block.
-        // Use 'select i8 true, %arg, undef' to simulate a 'no-op' instruction.
-        Value *TrueValue = ConstantInt::getTrue(Context);
-        Value *UndefValue = UndefValue::get(Arg->getType());
-        Instruction *SI =
-            SelectInst::Create(TrueValue, Arg, UndefValue,
-                               Arg->getName() + ".tmp", AllocaInsertPt);
-        Arg->replaceAllUsesWith(SI);
-        // Reset the select operand, because it was clobbered by the RAUW above.
-        SI->setOperand(1, Arg);
-        ParentAlloca = DemoteRegToStack(*SI, true, SI);
-      } else if (auto *PN = dyn_cast<PHINode>(ParentVal)) {
-        ParentAlloca = DemotePHIToStack(PN, AllocaInsertPt);
-      } else {
-        Instruction *ParentInst = cast<Instruction>(ParentVal);
-        // FIXME: This is a work-around to temporarily handle the case where an
-        //        instruction that is only used in handlers is not sunk.
-        //        Without uses, DemoteRegToStack would just eliminate the value.
-        //        This will fail if ParentInst is an invoke.
-        if (ParentInst->getNumUses() == 0) {
-          BasicBlock::iterator InsertPt = ParentInst;
-          ++InsertPt;
-          ParentAlloca =
-              new AllocaInst(ParentInst->getType(), nullptr,
-                             ParentInst->getName() + ".reg2mem",
-                             AllocaInsertPt);
-          new StoreInst(ParentInst, ParentAlloca, InsertPt);
-        } else {
-          ParentAlloca = DemoteRegToStack(*ParentInst, true, AllocaInsertPt);
-        }
-      }
-    }
+    AllocaInst *ParentAlloca = cast<AllocaInst>(ParentVal);
 
     // FIXME: We should try to sink unescaped allocas from the parent frame into
     // the child frame. If the alloca is escaped, we have to use the lifetime
     // markers to ensure that the alloca is only live within the child frame.
 
     // Add this alloca to the list of things to escape.
     AllocasToEscape.push_back(ParentAlloca);
 

 WinEHFrameVariableMaterializer::WinEHFrameVariableMaterializer(
     Function *OutlinedFn, FrameVarInfoMap &FrameVarInfo)
     : FrameVarInfo(FrameVarInfo), Builder(OutlinedFn->getContext()) {
   BasicBlock *EntryBB = &OutlinedFn->getEntryBlock();
   Builder.SetInsertPoint(EntryBB, EntryBB->getFirstInsertionPt());
 }
 
 Value *WinEHFrameVariableMaterializer::materializeValueFor(Value *V) {
-  // If we're asked to materialize a value that is an instruction, we
-  // temporarily create an alloca in the outlined function and add this
-  // to the FrameVarInfo map.  When all the outlining is complete, we'll
-  // collect these into a structure, spilling non-alloca values in the
-  // parent frame as necessary, and replace these temporary allocas with
-  // GEPs referencing the frame allocation block.
-
-  // If the value is an alloca, the mapping is direct.
+  // If we're asked to materialize a static alloca, we temporarily create an
+  // alloca in the outlined function and add this to the FrameVarInfo map.  When
+  // all the outlining is complete, we'll replace these temporary allocas with
+  // calls to llvm.framerecover.
   if (auto *AV = dyn_cast<AllocaInst>(V)) {
+    assert(AV->isStaticAlloca() &&
+           "cannot materialize un-demoted dynamic alloca");
     AllocaInst *NewAlloca = dyn_cast<AllocaInst>(AV->clone());
     Builder.Insert(NewAlloca, AV->getName());
     FrameVarInfo[AV].push_back(NewAlloca);
     return NewAlloca;
   }
 
-  // For other types of instructions or arguments, we need an alloca based on
-  // the value's type and a load of the alloca.  The alloca will be replaced
-  // by a GEP, but the load will stay.  In the parent function, the value will
-  // be spilled to a location in the frame allocation block.
   if (isa<Instruction>(V) || isa<Argument>(V)) {
-    AllocaInst *NewAlloca =
-        Builder.CreateAlloca(V->getType(), nullptr, "eh.temp.alloca");
-    FrameVarInfo[V].push_back(NewAlloca);
-    LoadInst *NewLoad = Builder.CreateLoad(NewAlloca, V->getName() + ".reload");
-    return NewLoad;
+    errs() << "Failed to demote instruction used in exception handler:\n";
+    errs() << "  " << *V << '\n';
+    report_fatal_error("WinEHPrepare failed to demote instruction");
   }
 
   // Don't materialize other values.
   return nullptr;
 }
 
 void WinEHFrameVariableMaterializer::escapeCatchObject(Value *V) {
   // Catch parameter objects have to live in the parent frame. When we see a use

llvm/trunk/test/CodeGen/WinEH/cppeh-demote-liveout.ll

+; RUN: opt -winehprepare -S < %s | FileCheck %s
+
+; Notionally based on this C++ source:
+; int liveout_catch(int p) {
+;   int val = p + 1;
+;   try {
+;     might_throw();
+;   } catch (int) {
+;     val++;
+;   }
+;   return val;
+; }
+
+declare void @llvm.eh.begincatch(i8*, i8*)
+declare void @llvm.eh.endcatch()
+declare void @might_throw()
+declare i32 @__CxxFrameHandler3(...)
+declare i32 @llvm.eh.typeid.for(i8*)
+
[e-mail] = external global i32
+
+define i32 @liveout_catch(i32 %p) {
+entry:
+  %val.entry = add i32 %p, 1
+  invoke void @might_throw()
+      to label %ret unwind label %lpad
+
+lpad:
+  %ehvals = landingpad { i8*, i32 } personality i32 (...)* @__CxxFrameHandler3
+      cleanup
+      catch i32* @typeinfo.int
+  %ehptr = extractvalue { i8*, i32 } %ehvals, 0
+  %sel = extractvalue { i8*, i32 } %ehvals, 1
+  %int_sel = call i32 @llvm.eh.typeid.for(i8* bitcast (i32* @typeinfo.int to i8*))
+  %match = icmp eq i32 %sel, %int_sel
+  br i1 %match, label %catchit, label %resume
+
+catchit:
+  call void @llvm.eh.begincatch(i8* %ehptr, i8* null)
+  %val.lpad = add i32 %val.entry, 1
+  call void @llvm.eh.endcatch()
+  br label %ret
+
+ret:
+  %rv = phi i32 [%val.entry, %entry], [%val.lpad, %catchit]
+  ret i32 %rv
+
+resume:
+  resume {i8*, i32} %ehvals
+}
+
+; CHECK-LABEL: define i32 @liveout_catch(i32 %p)
+; CHECK: %val.entry = add i32 %p, 1
+; CHECK-NEXT: store i32 %val.entry, i32* %val.entry.reg2mem
+; CHECK: invoke void @might_throw()
+;
+; CHECK: landingpad
+; CHECK: indirectbr i8* {{.*}}, [label %ehreturn]
+;
+; CHECK: ehreturn:
+; CHECK: load i32, i32* %val.lpad.reg2mem
+; CHECK: br label %ret
+;
+; CHECK: ret:
+; CHECK: %rv = phi i32 [ {{.*}}, %entry ], [ {{.*}}, %ehreturn ]
+; CHECK: ret i32
+
+; CHECK-LABEL: define internal i8* @liveout_catch.catch(i8*, i8*)
+; CHECK: %[[val:[^ ]*]] = load i32, i32*
+; CHECK-NEXT: %[[val_lpad:[^ ]*]] = add i32 %[[val]], 1
+; CHECK-NEXT: store i32 %[[val_lpad]], i32*
+; CHECK: ret i8* blockaddress(@liveout_catch, %ehreturn)

llvm/trunk/test/CodeGen/WinEH/cppeh-inalloca.ll

 
 @"\01??_7type_info@@6B@" = external constant i8*
 @"\01??_R0H@8" = linkonce_odr global %rtti.TypeDescriptor2 { i8** @"\01??_7type_info@@6B@", i8* null, [3 x i8] c".H\00" }, comdat
 
 ; The function entry should be rewritten like this.
 ; CHECK: define i32 @"\01?test@@YAHUA@@@Z"(<{ %struct.A }>* inalloca)
 ; CHECK: entry:
 ; CHECK:   [[TMP_REGMEM:\%.+]] = alloca <{ %struct.A }>*
-; CHECK:   [[TMP:\%.+]] = select i1 true, <{ %struct.A }>* %0, <{ %struct.A }>* undef
-; CHECK:   store <{ %struct.A }>* [[TMP]], <{ %struct.A }>** [[TMP_REGMEM]]
+; CHECK:   store <{ %struct.A }>* %0, <{ %struct.A }>** [[TMP_REGMEM]]
 ; CHECK:   [[RETVAL:\%.+]] = alloca i32, align 4
 ; CHECK:   [[E_PTR:\%.+]] = alloca i32, align 4
 ; CHECK:   [[CLEANUP_SLOT:\%.+]] = alloca i32
 ; CHECK:   call void (...) @llvm.frameescape(i32* %e, <{ %struct.A }>** [[TMP_REGMEM]], i32* [[RETVAL]], i32* [[CLEANUP_SLOT]])
 ; CHECK:   invoke void @"\01?may_throw@@YAXXZ"()
 ; CHECK:           to label %invoke.cont unwind label %[[LPAD_LABEL:lpad[0-9]*]]
 
 define i32 @"\01?test@@YAHUA@@@Z"(<{ %struct.A }>* inalloca) #0 {

 
 try.cont:                                         ; preds = %invoke.cont
   store i32 0, i32* %retval
   store i32 1, i32* %cleanup.dest.slot
   br label %cleanup
 
 ; The cleanup block should be re-written like this.
 ; CHECK: cleanup:{{[ ]+}}; preds = %[[LPAD_LABEL]], %try.cont
-; CHECK-NOT:  %a2 = getelementptr inbounds <{ %struct.A }>, <{ %struct.A }>* %0, i32 0, i32 0
-; CHECK:   [[TMP_RELOAD:\%.+]] = load volatile <{ %struct.A }>*, <{ %struct.A }>** [[TMP_REGMEM]]
-; CHECK:   [[A2:\%.+]] = getelementptr inbounds <{ %struct.A }>, <{ %struct.A }>* [[TMP_RELOAD]], i32 0, i32 0
-; CHECK:   call x86_thiscallcc void @"\01??1A@@QAE@XZ"(%struct.A* [[A2]])
+; CHECK:   %a2 = getelementptr inbounds <{ %struct.A }>, <{ %struct.A }>* %0, i32 0, i32 0
+; CHECK:   call x86_thiscallcc void @"\01??1A@@QAE@XZ"(%struct.A* %a2)
 ; CHECK:   [[TMP1:\%.+]] = load i32, i32* [[RETVAL]]
 ; CHECK:   ret i32 [[TMP1]]
 
 cleanup:                                          ; preds = %try.cont, %catch
   %a2 = getelementptr inbounds <{ %struct.A }>, <{ %struct.A }>* %0, i32 0, i32 0
   call x86_thiscallcc void @"\01??1A@@QAE@XZ"(%struct.A* %a2) #3
   %tmp10 = load i32, i32* %retval
   ret i32 %tmp10

 
 ; The following catch handler should be outlined.
 ; CHECK: define internal i8* @"\01?test@@YAHUA@@@Z.catch"(i8*, i8*)
 ; CHECK: entry:
 ; CHECK:   [[RECOVER_E:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (i32 (<{ %struct.A }>*)* @"\01?test@@YAHUA@@@Z" to i8*), i8* %1, i32 0)
 ; CHECK:   [[E_PTR:\%.+]] = bitcast i8* [[RECOVER_E]] to i32*
 ; CHECK:   [[RECOVER_EH_TEMP:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (i32 (<{ %struct.A }>*)* @"\01?test@@YAHUA@@@Z" to i8*), i8* %1, i32 1)
 ; CHECK:   [[EH_TEMP:\%.+]] = bitcast i8* [[RECOVER_EH_TEMP]] to <{ %struct.A }>**
-; CHECK:   [[TMP_RELOAD:\%.+]] = load <{ %struct.A }>*, <{ %struct.A }>** [[EH_TEMP]]
 ; CHECK:   [[RECOVER_RETVAL:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (i32 (<{ %struct.A }>*)* @"\01?test@@YAHUA@@@Z" to i8*), i8* %1, i32 2)
 ; CHECK:   [[RETVAL1:\%.+]] = bitcast i8* [[RECOVER_RETVAL]] to i32*
 ; CHECK:   [[RECOVER_CLEANUPSLOT:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (i32 (<{ %struct.A }>*)* @"\01?test@@YAHUA@@@Z" to i8*), i8* %1, i32 3)
 ; CHECK:   [[CLEANUPSLOT1:\%.+]] = bitcast i8* [[RECOVER_CLEANUPSLOT]] to i32*
 ; CHECK:   [[E_I8PTR:\%.+]] = bitcast i32* [[E_PTR]] to i8*
+; CHECK:   [[TMP_RELOAD:\%.+]] = load <{ %struct.A }>*, <{ %struct.A }>** [[EH_TEMP]]
 ; CHECK:   [[RECOVER_A:\%.+]] = getelementptr inbounds <{ %struct.A }>, <{ %struct.A }>* [[TMP_RELOAD]], i32 0, i32 0
 ; CHECK:   [[A1:\%.+]] = getelementptr inbounds %struct.A, %struct.A* [[RECOVER_A]], i32 0, i32 0
 ; CHECK:   [[TMP2:\%.+]] = load i32, i32* [[A1]], align 4
 ; CHECK:   [[TMP3:\%.+]] = load i32, i32* [[E_PTR]], align 4
 ; CHECK:   [[ADD:\%.+]] = add nsw i32 [[TMP2]], [[TMP3]]
 ; CHECK:   store i32 [[ADD]], i32* [[RETVAL1]]
 ; CHECK:   store i32 1, i32* [[CLEANUPSLOT1]]
 ; CHECK:   ret i8* blockaddress(@"\01?test@@YAHUA@@@Z", %cleanup)

llvm/trunk/test/CodeGen/WinEH/cppeh-multi-catch.ll

 ; CHECK-NEXT:           catch %eh.HandlerMapEntry* @llvm.eh.handlermapentry._J
 ; CHECK-NEXT:           catch %eh.HandlerMapEntry* @"llvm.eh.handlermapentry.reference.?AVSomeClass@@"
 ; CHECK-NEXT:           catch i8* null
 ; CHECK-NEXT:   [[RECOVER:\%.+]] = call i8* (...) @llvm.eh.actions(
 ; CHECK-SAME:      i32 1, i8* bitcast (%eh.HandlerMapEntry* @llvm.eh.handlermapentry.H to i8*), i32 0, i8* (i8*, i8*)* @"\01?test@@YAXXZ.catch",
 ; CHECK-SAME:        i32 1, i8* bitcast (%eh.HandlerMapEntry* @llvm.eh.handlermapentry._J to i8*), i32 1, i8* (i8*, i8*)* @"\01?test@@YAXXZ.catch1",
 ; CHECK-SAME:        i32 1, i8* bitcast (%eh.HandlerMapEntry* @"llvm.eh.handlermapentry.reference.?AVSomeClass@@" to i8*), i32 2, i8* (i8*, i8*)* @"\01?test@@YAXXZ.catch2",
 ; CHECK-SAME:        i32 1, i8* null, i32 -1, i8* (i8*, i8*)* @"\01?test@@YAXXZ.catch3")
-; CHECK-NEXT:   indirectbr i8* [[RECOVER]], [label %catch14.split, label %catch10.split, label %catch6.split, label %catch.split]
+; CHECK-NEXT:   indirectbr i8* [[RECOVER]], [label %ret]
 
 lpad:                                             ; preds = %entry
   %0 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*)
           catch %eh.HandlerMapEntry* @llvm.eh.handlermapentry.H
           catch %eh.HandlerMapEntry* @llvm.eh.handlermapentry._J
           catch %eh.HandlerMapEntry* @"llvm.eh.handlermapentry.reference.?AVSomeClass@@"
           catch i8* null
   %1 = extractvalue { i8*, i32 } %0, 0
   store i8* %1, i8** %exn.slot
   %2 = extractvalue { i8*, i32 } %0, 1
   store i32 %2, i32* %ehselector.slot
   br label %catch.dispatch
 
 ; CHECK-NOT: catch.dispatch:
 catch.dispatch:                                   ; preds = %lpad
   %sel = load i32, i32* %ehselector.slot
   %3 = call i32 @llvm.eh.typeid.for(i8* bitcast (%eh.HandlerMapEntry* @llvm.eh.handlermapentry.H to i8*)) #3
   %matches = icmp eq i32 %sel, %3
   br i1 %matches, label %catch14, label %catch.fallthrough
 
+ret:
+  ret void
+
 ; CHECK-NOT: catch14:
-; CHECK: catch14.split:
+; CHECK: ret:
 ; CHECK-NEXT:   ret void
 catch14:                                          ; preds = %catch.dispatch
   %exn15 = load i8*, i8** %exn.slot
   %4 = bitcast i32* %i to i8*
   call void @llvm.eh.begincatch(i8* %exn15, i8* %4) #3
   %5 = load i32, i32* %i, align 4
   call void @"\01?handle_int@@YAXH@Z"(i32 %5)
   call void @llvm.eh.endcatch() #3
-  ret void
+  br label %ret
 
 try.cont:                                         ; preds = %invoke.cont
-  ret void
+  br label %ret
 
 ; CHECK-NOT: catch.fallthrough:
 catch.fallthrough:                                ; preds = %catch.dispatch
   %6 = call i32 @llvm.eh.typeid.for(i8* bitcast (%eh.HandlerMapEntry* @llvm.eh.handlermapentry._J to i8*)) #3
   %matches1 = icmp eq i32 %sel, %6
   br i1 %matches1, label %catch10, label %catch.fallthrough2
 
 ; CHECK-NOT: catch10:
-; CHECK: catch10.split:
-; CHECK-NEXT:   ret void
 catch10:                                          ; preds = %catch.fallthrough
   %exn11 = load i8*, i8** %exn.slot
   %7 = bitcast i64* %ll to i8*
   call void @llvm.eh.begincatch(i8* %exn11, i8* %7) #3
   %8 = load i64, i64* %ll, align 8
   call void @"\01?handle_long_long@@YAX_J@Z"(i64 %8)
   call void @llvm.eh.endcatch() #3
-  ret void
+  br label %ret
 
 ; CHECK-NOT: catch.fallthrough2:
 catch.fallthrough2:                               ; preds = %catch.fallthrough
   %9 = call i32 @llvm.eh.typeid.for(i8* bitcast (%eh.HandlerMapEntry* @"llvm.eh.handlermapentry.reference.?AVSomeClass@@" to i8*)) #3
   %matches3 = icmp eq i32 %sel, %9
   br i1 %matches3, label %catch6, label %catch
 
 ; CHECK-NOT: catch6:
-; CHECK: catch6.split:
-; CHECK-NEXT:   ret void
 catch6:                                           ; preds = %catch.fallthrough2
   %exn7 = load i8*, i8** %exn.slot
   %10 = bitcast %class.SomeClass** %obj to i8*
   call void @llvm.eh.begincatch(i8* %exn7, i8* %10) #3
   %11 = load %class.SomeClass*, %class.SomeClass** %obj, align 8
   call void @"\01?handle_obj@@YAXPEAVSomeClass@@@Z"(%class.SomeClass* %11)
   call void @llvm.eh.endcatch() #3
-  ret void
+  br label %ret
 
 ; CHECK-NOT: catch:
-; CHECK: catch.split:
-; CHECK-NEXT:   ret void
 catch:                                            ; preds = %catch.fallthrough2
   %exn = load i8*, i8** %exn.slot
   call void @llvm.eh.begincatch(i8* %exn, i8* null) #3
   call void @"\01?handle_exception@@YAXXZ"()  call void @llvm.eh.endcatch() #3
-  ret void
+  br label %ret
 ; CHECK: }
 }
 
 ; CHECK-LABEL: define internal i8* @"\01?test@@YAXXZ.catch"(i8*, i8*)
 ; CHECK: entry:
 ; CHECK:   [[RECOVER_I:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 0)
 ; CHECK:   [[I_PTR:\%.+]] = bitcast i8* [[RECOVER_I]] to i32*
 ; CHECK:   [[TMP1:\%.+]] = load i32, i32* [[I_PTR]], align 4
 ; CHECK:   call void @"\01?handle_int@@YAXH@Z"(i32 [[TMP1]])
-; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %catch14.split)
+; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %ret)
 ; CHECK: }
 
 ; CHECK-LABEL: define internal i8* @"\01?test@@YAXXZ.catch1"(i8*, i8*)
 ; CHECK: entry:
 ; CHECK:   [[RECOVER_LL:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 1)
 ; CHECK:   [[LL_PTR:\%.+]] = bitcast i8* [[RECOVER_LL]] to i64*
 ; CHECK:   [[TMP2:\%.+]] = load i64, i64* [[LL_PTR]], align 8
 ; CHECK:   call void @"\01?handle_long_long@@YAX_J@Z"(i64 [[TMP2]])
-; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %catch10.split)
+; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %ret)
 ; CHECK: }
 
 ; CHECK-LABEL: define internal i8* @"\01?test@@YAXXZ.catch2"(i8*, i8*)
 ; CHECK: entry:
 ; CHECK:   [[RECOVER_OBJ:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 2)
 ; CHECK:   [[OBJ_PTR:\%.+]] = bitcast i8* [[RECOVER_OBJ]] to %class.SomeClass**
 ; CHECK:   [[TMP3:\%.+]] = load %class.SomeClass*, %class.SomeClass** [[OBJ_PTR]], align 8
 ; CHECK:   call void @"\01?handle_obj@@YAXPEAVSomeClass@@@Z"(%class.SomeClass* [[TMP3]])
-; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %catch6.split)
+; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %ret)
 ; CHECK: }
 
 ; CHECK-LABEL: define internal i8* @"\01?test@@YAXXZ.catch3"(i8*, i8*)
 ; CHECK: entry:
 ; CHECK:   call void @"\01?handle_exception@@YAXXZ"()
-; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %catch.split)
+; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %ret)
 ; CHECK: }
 
 
 declare void @"\01?may_throw@@YAXXZ"() #1
 
 declare i32 @__CxxFrameHandler3(...)
 
 ; Function Attrs: nounwind readnone

llvm/trunk/test/CodeGen/WinEH/cppeh-nonalloca-frame-values.ll

 @"\01??_7type_info@@6B@" = external constant i8*
 @"\01??_R0H@8" = linkonce_odr global %rtti.TypeDescriptor2 { i8** @"\01??_7type_info@@6B@", i8* null, [3 x i8] c".H\00" }, comdat
 
 ; The function entry should be rewritten like this.
 ; CHECK: define void @"\01?test@@YAXXZ"()
 ; CHECK: entry:
 ; CHECK:   [[NUMEXCEPTIONS_REGMEM:\%.+]] = alloca i32
 ; CHECK:   [[I_REGMEM:\%.+]] = alloca i32
-; CHECK:   [[A_REGMEM:\%.+]] = alloca i32*
 ; CHECK:   [[B_REGMEM:\%.+]] = alloca i32*
+; CHECK:   [[A_REGMEM:\%.+]] = alloca i32*
 ; CHECK:   [[E_PTR:\%.+]] = alloca i32, align 4
 ; CHECK:   [[EXCEPTIONVAL:\%.+]] = alloca [10 x i32], align 16
 ; CHECK:   [[DATA_PTR:\%.+]] = alloca i64, align 8
 ; CHECK:   [[TMPCAST:\%.+]] = bitcast i64* [[DATA_PTR]] to %struct.SomeData*
 ; CHECK:   [[TMP:\%.+]] = bitcast [10 x i32]* [[EXCEPTIONVAL]] to i8*
 ; CHECK:   call void @llvm.lifetime.start(i64 40, i8* [[TMP]])
 ; CHECK:   store i64 0, i64* [[DATA_PTR]], align 8
 ; CHECK:   [[A_PTR:\%.+]] = bitcast i64* [[DATA_PTR]] to i32*
 ; CHECK:   store i32* [[A_PTR]], i32** [[A_REGMEM]]
 ; CHECK:   [[B_PTR:\%.+]] = getelementptr inbounds %struct.SomeData, %struct.SomeData* [[TMPCAST]], i64 0, i32 1
 ; CHECK:   store i32* [[B_PTR]], i32** [[B_REGMEM]]
-; CHECK:   store i32 0, i32* [[NUMEXCEPTIONS_REGMEM]]
-; CHECK:   store i32 0, i32* [[I_REGMEM]]
-; CHECK:   call void (...) @llvm.frameescape(i32* %e, i32* %NumExceptions.020.reg2mem, [10 x i32]* [[EXCEPTIONVAL]], i32* [[I_REGMEM]], i32** [[A_REGMEM]], i32** [[B_REGMEM]])
+; CHECK:   call void (...) @llvm.frameescape(i32* %e, i32* %NumExceptions.020.reg2mem, [10 x i32]* [[EXCEPTIONVAL]], i32* %inc.reg2mem, i32* [[I_REGMEM]], i32** [[A_REGMEM]], i32** [[B_REGMEM]])
 ; CHECK:   br label %for.body
 
 ; Function Attrs: uwtable
 define void @"\01?test@@YAXXZ"() #0 {
 entry:
   %e = alloca i32, align 4
   %ExceptionVal = alloca [10 x i32], align 16
   %Data = alloca i64, align 8
   %tmpcast = bitcast i64* %Data to %struct.SomeData*
   %0 = bitcast [10 x i32]* %ExceptionVal to i8*
   call void @llvm.lifetime.start(i64 40, i8* %0) #1
   store i64 0, i64* %Data, align 8
   %a = bitcast i64* %Data to i32*
   %b = getelementptr inbounds %struct.SomeData, %struct.SomeData* %tmpcast, i64 0, i32 1
   br label %for.body
 
 ; CHECK: for.body:
-; CHECK-NOT:  phi i32 [ 0, %entry ], [ {{\%NumExceptions.*}}, %try.cont ]
-; CHECK-NOT:  phi i32 [ 0, %entry ], [ {{\%inc.*}}, %try.cont ]
-; CHECK:  [[I_RELOAD:\%.+]] = load i32, i32* [[I_REGMEM]]
-; CHECK:  [[NUMEXCEPTIONS_RELOAD:\%.+]] = load i32, i32* [[NUMEXCEPTIONS_REGMEM]]
+; CHECK:   [[NUMEXCEPTIONS_PHI:\%.*]] = phi i32 [ 0, %entry ], [ {{\%NumExceptions.*}}, %try.cont ]
+; CHECK:   [[I_PHI:\%.*]] = phi i32 [ 0, %entry ], [ {{\%inc.*}}, %try.cont ]
+; CHECK:   store i32 [[I_PHI]], i32* [[I_REGMEM]]
+; CHECK:   store i32 [[NUMEXCEPTIONS_PHI]], i32* [[NUMEXCEPTIONS_REGMEM]]
+; CHECK:   invoke void @"\01?may_throw@@YAXXZ"()
 for.body:                                         ; preds = %entry, %try.cont
   %NumExceptions.020 = phi i32 [ 0, %entry ], [ %NumExceptions.1, %try.cont ]
   %i.019 = phi i32 [ 0, %entry ], [ %inc5, %try.cont ]
   invoke void @"\01?may_throw@@YAXXZ"()
           to label %invoke.cont unwind label %lpad
 
 ; CHECK: invoke.cont:                                      ; preds = %for.body
-; CHECK:   [[A_RELOAD:\%.+]] = load volatile i32*, i32** [[A_REGMEM]]
+; CHECK:   [[A_RELOAD:\%.+]] = load i32*, i32** [[A_REGMEM]]
 ; CHECK:   [[TMP1:\%.+]] = load i32, i32* [[A_RELOAD]], align 8
+; CHECK:   [[I_RELOAD:\%.+]] = load i32, i32* [[I_REGMEM]]
 ; CHECK:   [[ADD:\%.+]] = add nsw i32 [[TMP1]], [[I_RELOAD]]
-; CHECK:   [[A_RELOAD1:\%.+]] = load volatile i32*, i32** [[A_REGMEM]]
-; CHECK:   store i32 [[ADD]], i32* [[A_RELOAD1]], align 8
+; CHECK:   [[A_RELOAD1:\%.+]] = load i32*, i32** [[A_REGMEM]]
+; CHECK:   [[NUMEXCEPTIONS_RELOAD:\%.+]] = load i32, i32* [[NUMEXCEPTIONS_REGMEM]]
 ; CHECK:   br label %try.cont
 invoke.cont:                                      ; preds = %for.body
   %1 = load i32, i32* %a, align 8, !tbaa !2
   %add = add nsw i32 %1, %i.019
   store i32 %add, i32* %a, align 8, !tbaa !2
   br label %try.cont
 
 ; CHECK: [[LPAD_LABEL:lpad[0-9]*]]:{{[ ]+}}; preds = %for.body
 ; CHECK:   landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*)
 ; CHECK-NEXT:           catch i8* bitcast (%rtti.TypeDescriptor2* @"\01??_R0H@8" to i8*)
 ; CHECK-NEXT:   [[RECOVER:\%.+]] = call i8* (...) @llvm.eh.actions(i32 1, i8* bitcast (%rtti.TypeDescriptor2* @"\01??_R0H@8" to i8*), i32 0, i8* (i8*, i8*)* @"\01?test@@YAXXZ.catch")
-; CHECK-NEXT:   indirectbr i8* [[RECOVER]], [label %try.cont]
+; CHECK-NEXT:   indirectbr i8* [[RECOVER]], [label %[[SPLIT_RECOVER_BB:.*]]]
 
 lpad:                                             ; preds = %for.body
   %2 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*)
           catch i8* bitcast (%rtti.TypeDescriptor2* @"\01??_R0H@8" to i8*)
   %3 = extractvalue { i8*, i32 } %2, 1
   %4 = tail call i32 @llvm.eh.typeid.for(i8* bitcast (%rtti.TypeDescriptor2* @"\01??_R0H@8" to i8*)) #1
   %matches = icmp eq i32 %3, %4
   br i1 %matches, label %catch, label %eh.resume
 
 ; CHECK-NOT: catch:
 
 catch:                                            ; preds = %lpad
   %5 = extractvalue { i8*, i32 } %2, 0
   %e.i8 = bitcast i32* %e to i8*
   call void @llvm.eh.begincatch(i8* %5, i8* %e.i8) #1
   %tmp8 = load i32, i32* %e, align 4, !tbaa !7
   %idxprom = sext i32 %NumExceptions.020 to i64
   %arrayidx = getelementptr inbounds [10 x i32], [10 x i32]* %ExceptionVal, i64 0, i64 %idxprom
   store i32 %tmp8, i32* %arrayidx, align 4, !tbaa !7
   %inc = add nsw i32 %NumExceptions.020, 1
   %cmp1 = icmp eq i32 %tmp8, %i.019
   br i1 %cmp1, label %if.then, label %if.else
 
-; CHECK-NOT: if.then:
-
 if.then:                                          ; preds = %catch
   %tmp9 = load i32, i32* %b, align 4, !tbaa !8
   %add2 = add nsw i32 %tmp9, %i.019
   store i32 %add2, i32* %b, align 4, !tbaa !8
   br label %if.end
 
 ; CHECK-NOT: if.else:
 
 if.else:                                          ; preds = %catch
   %tmp10 = load i32, i32* %a, align 8, !tbaa !2
   %add4 = add nsw i32 %tmp10, %tmp8
   store i32 %add4, i32* %a, align 8, !tbaa !2
   br label %if.end
 
 ; CHECK-NOT: if.end:
+; CHECK: [[SPLIT_RECOVER_BB]]:
+; CHECK: [[INC_RELOAD:\%.*]] = load i32, i32*
+; CHECK: br label %try.cont
 
 if.end:                                           ; preds = %if.else, %if.then
   tail call void @llvm.eh.endcatch() #1
   br label %try.cont
 
-; CHECK: try.cont:{{[ ]+}}; preds = %[[LPAD_LABEL]], %invoke.cont
-; CHECK-NOT:  phi i32
-; CHECK:   tail call void @"\01?does_not_throw@@YAXH@Z"(i32 [[NUMEXCEPTIONS_RELOAD]])
+; CHECK: try.cont:{{[ ]+}}; preds = %[[SPLIT_RECOVER_BB]], %invoke.cont
+; CHECK:   [[NUMEXCEPTIONS_PHI:\%.*]] = phi i32 [ [[NUMEXCEPTIONS_RELOAD]], %invoke.cont ], [ [[INC_RELOAD]], %[[SPLIT_RECOVER_BB]] ]
+; CHECK:   tail call void @"\01?does_not_throw@@YAXH@Z"(i32 [[NUMEXCEPTIONS_PHI]])
+; CHECK:   [[I_RELOAD:\%.+]] = load i32, i32* [[I_REGMEM]]
 ; CHECK:   [[INC:\%.+]] = add nuw nsw i32 [[I_RELOAD]], 1
 ; CHECK:   [[CMP:\%.+]] = icmp slt i32 [[INC]], 10
-; CHECK:   store i32 [[NUMEXCEPTIONS_RELOAD]], i32* [[NUMEXCEPTIONS_REGMEM]]
-; CHECK:   store i32 [[INC]], i32* [[I_REGMEM]]
 ; CHECK:   br i1 [[CMP]], label %for.body, label %for.end
 
 try.cont:                                         ; preds = %if.end, %invoke.cont
   %NumExceptions.1 = phi i32 [ %NumExceptions.020, %invoke.cont ], [ %inc, %if.end ]
   tail call void @"\01?does_not_throw@@YAXH@Z"(i32 %NumExceptions.1)
   %inc5 = add nuw nsw i32 %i.019, 1
   %cmp = icmp slt i32 %inc5, 10
   br i1 %cmp, label %for.body, label %for.end
eh.resume:                                        ; preds = %lpad
   resume { i8*, i32 } %.lcssa
 }
 
 ; The following catch handler should be outlined.
 ; CHECK: define internal i8* @"\01?test@@YAXXZ.catch"(i8*, i8*)
 ; CHECK: entry:
 ; CHECK:   [[RECOVER_E:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 0)
 ; CHECK:   [[E_PTR:\%.+]] = bitcast i8* [[RECOVER_E]] to i32*
-; CHECK:   [[RECOVER_EH_TEMP:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 1)
-; CHECK:   [[EH_TEMP:\%.+]] = bitcast i8* [[RECOVER_EH_TEMP]] to i32*
-; CHECK:   [[NUMEXCEPTIONS_RELOAD:\%.+]] = load i32, i32* [[EH_TEMP]]
+; CHECK:   [[RECOVER_NUMEXCEPTIONS:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 1)
+; CHECK:   [[NUMEXCEPTIONS_REGMEM:\%.+]] = bitcast i8* [[RECOVER_NUMEXCEPTIONS]] to i32*
 ; CHECK:   [[RECOVER_EXCEPTIONVAL:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 2)
 ; CHECK:   [[EXCEPTIONVAL:\%.+]] = bitcast i8* [[RECOVER_EXCEPTIONVAL]] to [10 x i32]*
-; CHECK:   [[RECOVER_EH_TEMP1:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 3)
-; CHECK:   [[EH_TEMP1:\%.+]] = bitcast i8* [[RECOVER_EH_TEMP1]] to i32*
-; CHECK:   [[I_RELOAD:\%.+]] = load i32, i32* [[EH_TEMP1]]
-; CHECK:   [[RECOVER_EH_TEMP2:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 4)
-; CHECK:   [[EH_TEMP2:\%.+]] = bitcast i8* [[RECOVER_EH_TEMP2]] to i32**
-; CHECK:   [[A_RELOAD:\%.+]] = load i32*, i32** [[EH_TEMP2]]
-; CHECK:   [[RECOVER_EH_TEMP3:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 5)
-; CHECK:   [[EH_TEMP3:\%.+]] = bitcast i8* [[RECOVER_EH_TEMP3]] to i32**
-; CHECK:   [[B_RELOAD:\%.+]] = load i32*, i32** [[EH_TEMP3]]
+; CHECK:   [[RECOVER_INC:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 3)
+; CHECK:   [[INC_REGMEM:\%.+]] = bitcast i8* [[RECOVER_INC]] to i32*
+; CHECK:   [[RECOVER_I:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 4)
+; CHECK:   [[I_REGMEM:\%.+]] = bitcast i8* [[RECOVER_I]] to i32*
+; CHECK:   [[RECOVER_A:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 5)
+; CHECK:   [[A_REGMEM:\%.+]] = bitcast i8* [[RECOVER_A]] to i32**
+; CHECK:   [[RECOVER_B:\%.+]] = call i8* @llvm.framerecover(i8* bitcast (void ()* @"\01?test@@YAXXZ" to i8*), i8* %1, i32 6)
+; CHECK:   [[B_REGMEM:\%.+]] = bitcast i8* [[RECOVER_B]] to i32**
 ; CHECK:   [[E_I8PTR:\%.+]] = bitcast i32* [[E_PTR]] to i8*
 ; CHECK:   [[TMP:\%.+]] = load i32, i32* [[E_PTR]], align 4
+; CHECK:   [[NUMEXCEPTIONS_RELOAD:\%.+]] = load i32, i32* [[NUMEXCEPTIONS_REGMEM]]
 ; CHECK:   [[IDXPROM:\%.+]] = sext i32 [[NUMEXCEPTIONS_RELOAD]] to i64
 ; CHECK:   [[ARRAYIDX:\%.+]] = getelementptr inbounds [10 x i32], [10 x i32]* [[EXCEPTIONVAL]], i64 0, i64 [[IDXPROM]]
 ; CHECK:   store i32 [[TMP]], i32* [[ARRAYIDX]], align 4
+; CHECK:   [[NUMEXCEPTIONS_RELOAD:\%.+]] = load i32, i32* [[NUMEXCEPTIONS_REGMEM]]
 ; CHECK:   [[INC:\%.+]] = add nsw i32 [[NUMEXCEPTIONS_RELOAD]], 1
 ; CHECK:   [[CMP:\%.+]] = icmp eq i32 [[TMP]], [[I_RELOAD]]
 ; CHECK:   br i1 [[CMP]], label %if.then, label %if.else
 ;
 ; CHECK: if.then:{{[ ]+}}; preds = %entry
+; CHECK:   [[B_RELOAD:\%.+]] = load i32*, i32** [[B_REGMEM]]
 ; CHECK:   [[TMP1:\%.+]] = load i32, i32* [[B_RELOAD]], align 4
+; CHECK:   [[I_RELOAD:\%.+]] = load i32, i32* [[I_REGMEM]]
 ; CHECK:   [[ADD:\%.+]] = add nsw i32 [[TMP1]], [[I_RELOAD]]
+; CHECK:   [[B_RELOAD:\%.+]] = load i32*, i32** [[B_REGMEM]]
 ; CHECK:   store i32 [[ADD]], i32* [[B_RELOAD]], align 4
 ; CHECK:   br label %if.end
 ;
 ; CHECK: if.else:{{[ ]+}}; preds = %entry
+; CHECK:   [[A_RELOAD:\%.+]] = load i32*, i32** [[A_REGMEM]]
 ; CHECK:   [[TMP2:\%.+]] = load i32, i32* [[A_RELOAD]], align 8
 ; CHECK:   [[ADD2:\%.+]] = add nsw i32 [[TMP2]], [[TMP]]
+; CHECK:   [[A_RELOAD:\%.+]] = load i32*, i32** [[A_REGMEM]]
 ; CHECK:   store i32 [[ADD2]], i32* [[A_RELOAD]], align 8
 ; CHECK:   br label %if.end
 ;
 ; CHECK: if.end:{{[ ]+}}; preds = %if.else, %if.then
-; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %try.cont)
+; CHECK:   ret i8* blockaddress(@"\01?test@@YAXXZ", %[[SPLIT_RECOVER_BB]])
 ; CHECK: }
 
 ; Function Attrs: nounwind
 declare void @llvm.lifetime.start(i64, i8* nocapture) #1
 
 declare void @"\01?may_throw@@YAXXZ"() #2
 
 declare i32 @__CxxFrameHandler3(...)

llvm/trunk/test/CodeGen/WinEH/seh-simple.ll

 
 eh.resume:
   resume { i8*, i32 } %ehvals
 }
 
 ; CHECK-LABEL: define i32 @except_phi()
 ; CHECK: landingpad { i8*, i32 }
 ; CHECK-NEXT: catch i32 ()* @filt
-; CHECK-NEXT: call i8* (...) @llvm.eh.actions(i32 1, i8* bitcast (i32 ()* @filt to i8*), i32 -1, i8* blockaddress(@except_phi, %return))
-; CHECK-NEXT: indirectbr {{.*}} [label %return]
+; CHECK-NEXT: call i8* (...) @llvm.eh.actions(i32 1, i8* bitcast (i32 ()* @filt to i8*), i32 -1, i8* blockaddress(@except_phi, %lpad.return_crit_edge))
+; CHECK-NEXT: indirectbr {{.*}} [label %lpad.return_crit_edge]
+;
+; CHECK: lpad.return_crit_edge:
+; CHECK: br label %return
+;
+; CHECK: return:
+; CHECK-NEXT: %r = phi i32 [ 0, %entry ], [ 1, %lpad.return_crit_edge ]
+; CHECK-NEXT: ret i32 %r
+
+define i32 @lpad_phi() {
+entry:
+  invoke void @might_crash()
+          to label %cont unwind label %lpad
+
+cont:
+  invoke void @might_crash()
+          to label %return unwind label %lpad
+
+lpad:
+  %ncalls.1 = phi i32 [ 0, %entry ], [ 1, %cont ]
+  %ehvals = landingpad { i8*, i32 } personality i32 (...)* @__C_specific_handler
+          catch i32 ()* @filt
+  %sel = extractvalue { i8*, i32 } %ehvals, 1
+  %filt_sel = tail call i32 @llvm.eh.typeid.for(i8* bitcast (i32 ()* @filt to i8*))
+  %matches = icmp eq i32 %sel, %filt_sel
+  br i1 %matches, label %return, label %eh.resume
+
+return:
+  %r = phi i32 [2, %cont], [%ncalls.1, %lpad]
+  ret i32 %r
+
+eh.resume:
+  resume { i8*, i32 } %ehvals
+}
+
+; CHECK-LABEL: define i32 @lpad_phi()
+; CHECK: alloca i32
+; CHECK: store i32 0, i32*
+; CHECK: invoke void @might_crash()
+; CHECK: store i32 1, i32*
+; CHECK: invoke void @might_crash()
+; CHECK: landingpad { i8*, i32 }
+; CHECK-NEXT: cleanup
+; CHECK-NEXT: catch i32 ()* @filt
+; CHECK-NEXT: call i8* (...) @llvm.eh.actions(i32 0, void (i8*, i8*)* @lpad_phi.cleanup, i32 1, i8* bitcast (i32 ()* @filt to i8*), i32 -1, i8* blockaddress(@lpad_phi, %lpad.return_crit_edge))
+; CHECK-NEXT: indirectbr {{.*}} [label %lpad.return_crit_edge]
+;
+; CHECK: lpad.return_crit_edge:
+; CHECK: load i32, i32*
+; CHECK: br label %return
 ;
 ; CHECK: return:
-; CHECK-NEXT: %r = phi i32 [ 0, %entry ], [ 1, %lpad1 ]
+; CHECK-NEXT: %r = phi i32 [ 2, %cont ], [ %{{.*}}, %lpad.return_crit_edge ]
 ; CHECK-NEXT: ret i32 %r
 
 define i32 @cleanup_and_except() {
 entry:
   invoke void @might_crash()
           to label %return unwind label %lpad
 
 lpad:

 }
 
 ; CHECK-LABEL: define i32 @cleanup_and_except()
 ; CHECK: landingpad { i8*, i32 }
 ; CHECK-NEXT: cleanup
 ; CHECK-NEXT: catch i32 ()* @filt
 ; CHECK-NEXT: call i8* (...) @llvm.eh.actions(
 ; CHECK: i32 0, void (i8*, i8*)* @cleanup_and_except.cleanup,
-; CHECK: i32 1, i8* bitcast (i32 ()* @filt to i8*), i32 -1, i8* blockaddress(@cleanup_and_except, %return))
-; CHECK-NEXT: indirectbr {{.*}} [label %return]
+; CHECK: i32 1, i8* bitcast (i32 ()* @filt to i8*), i32 -1, i8* blockaddress(@cleanup_and_except, %lpad.return_crit_edge))
+; CHECK-NEXT: indirectbr {{.*}} [label %lpad.return_crit_edge]
+;
+; CHECK: lpad.return_crit_edge:
+; CHECK: br label %return
 ;
 ; CHECK: return:
-; CHECK-NEXT: %r = phi i32 [ 0, %entry ], [ 1, %lpad1 ]
+; CHECK-NEXT: %r = phi i32 [ 0, %entry ], [ 1, %lpad.return_crit_edge ]
 ; CHECK-NEXT: ret i32 %r
+
+; FIXME: This cleanup is an artifact of bad demotion.
+; CHECK-LABEL: define internal void @lpad_phi.cleanup(i8*, i8*)
+; CHECK: load i32
+; CHECK: store i32 %{{.*}}, i32*