[PATCH] Extended support for native Windows C++ EH outlining

[Andy Kaylor]

REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:1116-1117
@@ +1115,4 @@
+//
+// FIXME: Update this to avoid re-parsing blocks that are shared by multiple
+//        landingpads.
+void WinEHPrepare::mapLandingPadBlocks(LandingPadInst *LPad,
----------------
rnk wrote:
> I think if we can resolve this, it'll greatly simplify the code and reduce the amount of repeated selector analysis.
Actually I think what I meant by this comment is already accomplished by the new handling of the catch and cleanup maps.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:1284
@@ +1283,3 @@
+    if (CleanupHandlerMap.count(BB)) {
+      ActionHandler;
+      if (auto *Action = CleanupHandlerMap[BB]) {
----------------
rnk wrote:
> Does this do anything?
No.  I have no idea why that's there.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:1301-1304
@@ +1300,6 @@
+        assert(Compare->isEquality());
+        if (Compare->getPredicate() == CmpInst::ICMP_EQ)
+          BB = Branch->getSuccessor(1);
+        else
+          BB = Branch->getSuccessor(0);
+        continue;
----------------
rnk wrote:
> Don't we need to analyze the condition more to know which BB is the fallthrough side?
I don't think so.  I believe earlier analysis of this block has determined that the block ends with a branch that is either unconditional or is predicated on the comparison of the result of an llvm.eh.typeid.for call and that the predicate is either ICMP_EQ or ICMP_NE, so just looking at the predicate tells us which successor corresponds to a selector match.

If the predicate is ICMP_EQ, then successor 0 is the catch block and successor 1 is the fall through, otherwise the reverse.

I can add some more comments to explain this.  I can probably also clean this up a bit with PatternMatch.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:1318-1322
@@ +1317,7 @@
+    bool IsLandingPadBB = BB->isLandingPad();
+    LandingPadMapper LPadMapper;
+    if (IsLandingPadBB) {
+      LandingPadInst *LPad = BB->getLandingPadInst();
+      LPadMapper.mapLandingPad(LPad);
+    }
+
----------------
rnk wrote:
> This seems like a lot of work, we end up mapping the same landingpads over and over in inner loops. Maybe we should have a map for each landing pad and just look them up inside the cloning directors?
You're right.  There's no reason to create more than one LandingPadMapper for each landing pad.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:1336-1338
@@ +1335,5 @@
+        if (!Insert2) {
+          CleanupHandler *Action = new CleanupHandler(BB);
+          CleanupHandlerMap[BB] = Action;
+          return Action;
+        }
----------------
rnk wrote:
> I feel like this repeated stanza could be simplified to something like:
>   return createCleanupHandler(CleanupHandlerMap, BB);
> 
>   void createCleanupHandler(CleanupHandlerMapTy &Map, BasicBlock *BB) {
>     auto *Action = new CleanupHandler(BB);
>     Map[BB] = Action;
>     return Action;
>   }
Sure.  We can do that.

http://reviews.llvm.org/D7886

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