[PATCH] Use WinEHPrepare to outline SEH finally blocks

[Reid Kleckner]

I'm going to take a shot at using StartBB instead of using HandlerBB like I did in this patch.


================
Comment at: lib/CodeGen/WinEHPrepare.cpp:94
@@ -91,1 +93,3 @@
+
+  EHPersonality Personality;
   CatchHandlerMapTy CatchHandlerMap;
----------------
andrew.w.kaylor wrote:
> We should probably add comments to say which of these fields (I guess all of them currently) are reset with each Function the pass is run on.
Yeah, all of em. :)

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:465
@@ +464,3 @@
+    // block.
+    LPadBB->replaceSuccessorsPhiUsesWith(NewLPadBB);
+    LPadBB->getTerminator()->eraseFromParent();
----------------
andrew.w.kaylor wrote:
> What exactly does this do?  I've seen some cases recently where cleanup outlining is choking because of PHINodes that are referring to the return value of the landingpad instruction.  This is too late to handle that (which just needs a few more VMap entries before outlining) but it raises the question in my mind of general handling of PHINodes which use values that are being erased from the original landing pads here.
> 
> I think that all of the code in question will be pruned as part of what we're doing here (it was resume blocks using either the landingpad value or values extracted from it).  I'm just raising the issue to see if you can think of anything else that might have a similar issue.
I was trying to handle except_phi test case:
```
define i32 @except_phi() {
entry:
  invoke void @might_crash()
          to label %return unwind label %lpad

lpad:
  %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 [0, %entry], [1, %lpad]
  ret i32 %r

eh.resume:
  resume { i8*, i32 } %ehvals
}
```

I needed to update the `%r` phi node. Maybe what I should do instead is split the landingpad block directly, because we might have this kind of code:
```
define i32 @except_phi(i32 %a) {
entry:
  invoke void @might_crash()
          to label %return unwind label %lpad

lpad:
  %ehvals = landingpad { i8*, i32 } personality i32 (...)* @__C_specific_handler
          catch i32 ()* @filt
  %sel = extractvalue { i8*, i32 } %ehvals, 1
  %a1 = add i32 %a, 1 ; live SSA value will become unreachable.
  %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 [%a, %entry], [%a1, %lpad]
  ret i32 %r

eh.resume:
  resume { i8*, i32 } %ehvals
}
```

The pass as is will probably crash. I'm OK with that for now, honestly.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:474
@@ -447,3 +473,3 @@
       if (auto *CatchAction = dyn_cast<CatchHandler>(Action)) {
         ActionArgs.push_back(ConstantInt::get(Int32Type, 0));
         ActionArgs.push_back(CatchAction->getSelector());
----------------
andrew.w.kaylor wrote:
> This should be 1 according to your recent documentation.
I'd rather make that a separate change.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:728
@@ +727,3 @@
+  Constant *Selector;
+  bool Res = isSelectorDispatch(StartBB, HandlerBB, Selector, NextBB);
+  (void)Res;
----------------
andrew.w.kaylor wrote:
> In the case of "__except(EXCEPTION_EXECUTE_HANDLER)" I'm seeing the landing pad branch unconditionally to the catch handler.
> 
Right, any catch-all block will ignore the selector and unconditionally branch into the recovery. I haven't handled that yet. I think a better approach might be to keep StartBB and simply discard all EH related instructions. This would also fix the phi case.

http://reviews.llvm.org/D8370

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/