[PATCH] Outline functions for SEH with the MSVC environment

[Andy Kaylor]

REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Transforms/Utils/Cloning.h:144-146
@@ +143,5 @@
+  virtual ~CloningStopHandler() {}
+
+  virtual void createTerminateInst(BasicBlock *BB) = 0;
+  virtual bool shouldStopAt(const Instruction *Inst) = 0;
+};
----------------
rnk wrote:
> These should probably merge into maybeEndCloning() or something like that, given that you had to thread state from shouldStopAt into createTerminateInst.
Yeah, that makes sense.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:163
@@ +162,3 @@
+
+bool MSVCEHPrepare::parseSEHLandingPad(BasicBlock::iterator &II,
+                                       LandingPadInst*      &Lpad,
----------------
rnk wrote:
> At the end of the day, EH lowering should not fail if the IR passes verification. I have some ideas for how cut back the fragile pattern matching, but I'm sure there are some edge cases I haven't thought of. For now, it's OK to assert on really uncommon ones.
At this point the assertions are as much documentation of my assumptions as anything else.  Basically, I put assertions in for any condition that would cause this code not to do what I meant for it to do.  I think that once this code is sufficiently robust the assertions will either go away or will be things that we genuinely don't believe can happen.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:236
@@ +235,3 @@
+
+bool MSVCEHPrepare::parseSEHHandlerSelect(BasicBlock::iterator &II,
+                                          const Value          *EHSel,
----------------
rnk wrote:
> I think this can be handled more robustly by setting up the value map for the selector and these intrinsic calls to return the constant 1 for the active selector and 0 for inactive selectors. The cloner already knows how to prune conditional branches like the ones you're pattern matching away, so we'd get that more or less for free. If the pruning analysis fails, we'd get something that is conservatively correct.
> 
> In this model, we'd start the cloning just after the landing pad instruction. Filters would have to be emitted before cleanups. This creates some challenges for clang IR emission, but I think I can handle it.
I think I understand what you're suggesting here.  I'll rework the code and update the review.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:326
@@ +325,3 @@
+
+  assert(!FilterResult && "An SEH filter handler contained multiple endpoints");
+
----------------
rnk wrote:
> We should be able to insert multiple return instructions to handle this case.
I was thinking that this is an indication of poorly formed IR, but maybe I'm being overly narrow in my thinking.  I was viewing this as code that clang would generate to represent the code between the parens of an __except statement, which should always evaluate to a single value, but I suppose I can see an arbitrary client of llvm calling the llvm.eh.seh.filter intrinsic to terminate any number of branches.  Since this code will replace each of them with a return instruction, that should be fine.

The real problem I'm trying to solve here is finding the beginning and end of the filter handler.  Following your suggestion of starting at the landing pad with mapped values to reduce the following instructions to the flow for the selector I mean to be handling, what I want to be sure of is that all control paths from the landing pad instruction will lead to a call to the llvm.eh.seh.filter intrinsic (or something that terminates in the case where an exception occurs within the filter function?).  But I can probably handle that better by asserting that this code doesn't see a return instruction as input.

http://reviews.llvm.org/D6556