[PATCH] Outline C++ exception handlers for MSVC targets.

[David Majnemer]

REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Transforms/Utils/Cloning.h:140-141
@@ +139,4 @@
+/// behavior while instructions are being cloned.
+class CloningDirector
+{
+public:
----------------
Style nit, please move the curly brace on the same line as the tag name.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:54
@@ +53,3 @@
+  bool prepareCPPEHHandlers(Function &F,
+                            SmallVector<LandingPadInst *, 4> &LPads);
+  bool outlineCatchHandler(Function *SrcFn, Constant *SelectorType,
----------------
I'd make this `SmallVectorImpl`.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:123
@@ -83,2 +122,3 @@
 
-  // FIXME: Cleanups are unimplemented. Replace them with unreachable.
+  // FIXME: This is only returning true if the C++ EH handlers were outlined.
+  //        When that code is complete, it should always return whatever
----------------
Perhaps: This only returns true if...

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:150
@@ +149,3 @@
+bool WinEHPrepare::prepareCPPEHHandlers(
+    Function &F, SmallVector<LandingPadInst *, 4> &LPads) {
+  // FIXME: Find all frame variable references in the handlers
----------------
`SmallVectorImpl`

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:156
@@ +155,3 @@
+  AllocStructTys.push_back(Type::getInt8PtrTy(F.getContext())); // EH object
+  Twine AllocStructName = "struct." + F.getName() + ".ehdata";
+  StructType *EHDataStructTy =
----------------
You shouldn't have variables of type twine.  They should be used directly in the expression which consumes them.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:167-171
@@ +166,7 @@
+      // FIXME: Make this an intrinsic.
+      CallInst *Call = dyn_cast<CallInst>(&Inst);
+      if (Call && Call->getCalledFunction()->getName() == "llvm.eh.actions") {
+        LPadHasActionList = true;
+        break;
+      }
+    }
----------------
It would be more idiomatic to do:
  if (auto *Call = dyn_cast<CallInst>(&Inst))
    if (Call->getCalledFunction->getName() == "llvm.eh.actions")

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:215
@@ +214,3 @@
+  Value *RecoverArgs[] = {Builder.CreateBitCast(SrcFn, Int8PtrType, ""),
+                          &(CatchHandler->getArgumentList().back())};
+  CallInst *EHAlloc =
----------------
This might deserve a comment.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:278-279
@@ +277,4 @@
+
+  const StoreInst *Store = dyn_cast<StoreInst>(Inst);
+  if (Store) {
+    // Look for and suppress stores of the extracted landingpad values.
----------------
Please fold this into a single line.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:295-296
@@ +294,4 @@
+
+  const LoadInst *Load = dyn_cast<LoadInst>(Inst);
+  if (Load) {
+    // Look for loads of (previously suppressed) landingpad values.
----------------
Please fold this into a single line.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:316-318
@@ +315,5 @@
+
+  const IntrinsicInst *IntrinCall = dyn_cast<IntrinsicInst>(Inst);
+  if (IntrinCall) {
+    if (IntrinCall->getIntrinsicID() == llvm::Intrinsic::eh_begincatch) {
+      // The argument to the call is some form of the first element of the
----------------
You can simplify this by using `if (match(Inst, m_Intrinsic<Intrinsic::eh_begincatch>()))`

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:328
@@ +327,3 @@
+    }
+    if (IntrinCall->getIntrinsicID() == llvm::Intrinsic::eh_endcatch) {
+      // It might be interesting to track whether or not we are inside a catch
----------------
Likewise here with `if (match(Inst, m_Intrinsic<Intrinsic::eh_endcatch>()))`

================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:684
@@ +683,3 @@
+                                     bool ModuleLevelChanges,
+                                     SmallVectorImpl<ReturnInst*> &Returns,
+                                     const char *NameSuffix, 
----------------
`SmallVectorImpl<ReturnInst *>`

http://reviews.llvm.org/D7363

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