[PATCH] D13424: [WinEH] Create a separate MBB for funclet prologues

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:219-220
@@ -218,2 +218,4 @@
       const Instruction *I = BB->getFirstNonPHI();
+      // FIXME: Is this correct for SEH __try/__except? I thought only __finally
+      // blocks are funclets for SEH personality routines...
       if (!isa<LandingPadInst>(I))
----------------
I guess we could enable tail merging, for example, on functions with SEH `__try/__except` only. Alternatively, we could reinterpret this variable as something like `usesWindowsEH` to indicate that it's using something other than the landingpad scheme for EH.

The minimal change is to add a comment like this and deal with it in another change:
  // FIXME: Don't mark SEH functions without __finally blocks as having funclets.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1163-1164
@@ -1162,2 +1162,4 @@
 void SelectionDAGBuilder::visitCatchPad(const CatchPadInst &I) {
-  llvm_unreachable("should never codegen catchpads");
+  bool IsMSVCCXX = classifyEHPersonality(FuncInfo.Fn->getPersonalityFn()) ==
+                   EHPersonality::MSVC_CXX;
+  MachineBasicBlock *CatchPadMBB = FuncInfo.MBB;
----------------
If we're going to change the way we do catchpad lowering for the personality, then we should eventually change the way we do SEH lowering to handle this IR:
```
  invoke void @crash()
    to .... unwind label %catchpad
catchpad:
  %p = catchpad ...
    to label %catchit ...
catchit:
  call void @code_before_the_catchret()
  catchret %p label %parentbb
```

Right now `code_before_the_catchret` in 32-bit SEH will run without the stack frame being setup correctly.

Basically, for SEH, we should terminate the catchpad block with a CATCHRET SD node, and codegen catchret instructions to branches. Should be easy.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1175-1176
@@ +1174,4 @@
+
+  // If this is not a fall-through branch or optimizations are switched off,
+  // emit the branch.
+  if (NormalDestMBB != NextBlock(CatchPadMBB) ||
----------------
Huh, I didn't realize we had fallthroughs in the machine CFG this early... We should change the way we do 32-bit CATCHRETs in X86FrameLowering to use fallthrough next.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1243-1244
@@ -1224,3 +1242,4 @@
     } else if (const auto *CPI = dyn_cast<CatchPadInst>(Pad)) {
-      // Add the catchpad handler to the possible destinations.
-      UnwindDests.push_back(FuncInfo.MBBMap[CPI->getNormalDest()]);
+      // Stop on catchpads. Catches are always funclet entries for all known
+      // personalities.
+      UnwindDests.push_back(FuncInfo.MBBMap[EHPadBB]);
----------------
We aren't stopping, though. We're continuing around the loop, unlike the lpad or cleanup case.


http://reviews.llvm.org/D13424