[PATCH] Outline functions for SEH with the MSVC environment

[Reid Kleckner]

Cool! I have some ideas for making this more robust.

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;
+};
----------------
These should probably merge into maybeEndCloning() or something like that, given that you had to thread state from shouldStopAt into createTerminateInst.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:163
@@ +162,3 @@
+
+bool MSVCEHPrepare::parseSEHLandingPad(BasicBlock::iterator &II,
+                                       LandingPadInst*      &Lpad,
----------------
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.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:183-190
@@ +182,10 @@
+
+  // This check is redundant.  The code that leads here already checked that the
+  // basic block we started in is a landing pad with the function named here.
+  // I'm leaving this for now because I'm starting with everything I could
+  // possibly check. I'll strip things back a bit once it all works.
+  if (!Lpad || 
+      Lpad->getPersonalityFn()->stripPointerCasts()->getName()
+                                                      != "__C_specific_handler")
+    return false;
+  ++II;
----------------
This makes sense, but I'd just make it an assert, since we'll want that assert in the final code anyway.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:193-220
@@ +192,30 @@
+
+  // %eh_ptrs = extractvalue { i8*, i32 } %vals, 0
+  //   or
+  // %sel = extractvalue { i8*, i32 } %vals, 1
+  ExtractValueInst *Extract = dyn_cast<ExtractValueInst>(II);
+  if (!Extract || 
+      Extract->getAggregateOperand() != Lpad ||
+      Extract->getNumIndices() != 1 ||
+      !(*(Extract->idx_begin()) == 0 ||
+        *(Extract->idx_begin()) == 1))
+    return false;
+  ++II;
+
+  if (*(Extract->idx_begin()) == 0)
+    EHPtrs = Extract;
+  else
+    EHSel = Extract;
+
+  // %sel = extractvalue { i8*, i32 } %vals, 1
+  //   or
+  // %eh_ptrs = extractvalue { i8*, i32 } %vals, 0
+  Extract = dyn_cast<ExtractValueInst>(II);
+  if (!Extract || 
+      Extract->getAggregateOperand() != Lpad ||
+      Extract->getNumIndices() != 1 ||
+      !(*(Extract->idx_begin()) == 0 ||
+        *(Extract->idx_begin()) == 1))
+    return false;
+  ++II;
+
----------------
This code can be made a lot more robust by reusing some of the techniques in the SjLj EH preparation code, which replaces the landingpad aggregate with something else. It optimistically iterates the use chain of the landingpad and replaces extractvalue instrs with the actual pointer and selector values. However, if it there are uses remaining, it synthesizes an aggregate and replaces all uses of the aggregate with that.

For the outliner, we don't want to replace things in place, we want to add to the value map instead. We probably want to produce something like:
  Instruction *EHAggregate; // Null if there are no uses of the aggregate other than extracts.
  SmallVector<Instruction*, 4> EHPtrExtracts;
  SmallVector<Instruction*, 4> EHSelExtracts;

And in each outlined filter, add mappings for all of those instructions.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:236
@@ +235,3 @@
+
+bool MSVCEHPrepare::parseSEHHandlerSelect(BasicBlock::iterator &II,
+                                          const Value          *EHSel,
----------------
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.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:304
@@ +303,3 @@
+
+  // TODO: Map other values referenced in the filter handler.
+
----------------
Yeah, this analysis is going to be fun. We can set it aside for later. I think there is some interesting prior art in SjLj preparation that will help.

================
Comment at: lib/CodeGen/MSVCEHPrepare.cpp:326
@@ +325,3 @@
+
+  assert(!FilterResult && "An SEH filter handler contained multiple endpoints");
+
----------------
We should be able to insert multiple return instructions to handle this case.

================
Comment at: test/CodeGen/X86/seh-outline.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple x86_64-pc-windows-msvc < %s
+
----------------
I think you can run `opt -codegenprepare` and FileCheck the output. Grep around test/ for `-codegenprep` to see what the other preparation tests do.

http://reviews.llvm.org/D6556