[WinEH] Fix single-block cleanup coloring

Summary:
The coloring code in WinEHPrepare queues cleanuprets' successors with the
correct color (the parent one) when it sees their cleanuppad, and so later
when iterating successors knows to skip processing cleanuprets since
they've already been queued.  This latter check was incorrectly under an
'else' condition and so inadvertently was not kicking in for single-block
cleanups.  This change sinks the check out of the 'else' to fix the bug.

Reviewers: majnemer, andrew.w.kaylor, rnk

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D12751

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@247299 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/WinEHPrepare.cpp b/lib/CodeGen/WinEHPrepare.cpp
index ea43cb2ffeba82d8de89b05315c2886fec92078b..3f666c944e2dfd5655a4c78addcd6abc6546651c 100644 (file)
--- a/lib/CodeGen/WinEHPrepare.cpp
+++ b/lib/CodeGen/WinEHPrepare.cpp
@@ -3245,14 +3245,15 @@ void WinEHPrepare::colorFunclets(Function &F,
     } else {
       // Note that this is a member of the given color.
       FuncletBlocks[Color].insert(Visiting);
-      TerminatorInst *Terminator = Visiting->getTerminator();
-      if (isa<CleanupReturnInst>(Terminator) ||
-          isa<CatchReturnInst>(Terminator) ||
-          isa<CleanupEndPadInst>(Terminator)) {
-        // These block's successors have already been queued with the parent
-        // color.
-        continue;
-      }
+    }
+
+    TerminatorInst *Terminator = Visiting->getTerminator();
+    if (isa<CleanupReturnInst>(Terminator) ||
+        isa<CatchReturnInst>(Terminator) ||
+        isa<CleanupEndPadInst>(Terminator)) {
+      // These blocks' successors have already been queued with the parent
+      // color.
+      continue;
     }
     for (BasicBlock *Succ : successors(Visiting)) {
       if (isa<CatchEndPadInst>(Succ->getFirstNonPHI())) {

diff --git a/test/CodeGen/WinEH/wineh-cloning.ll b/test/CodeGen/WinEH/wineh-cloning.ll
index 39e5d40a48f6ea79a83b7c47a85659f652c63ee2..f7fe58f844fb57f4bf61c1006d58984e85055228 100644 (file)
--- a/test/CodeGen/WinEH/wineh-cloning.ll
+++ b/test/CodeGen/WinEH/wineh-cloning.ll
@@ -27,7 +27,7 @@ endcatch:
 ; Need two copies of the call to @h, one under entry and one under catch.
 ; Currently we generate a load for each, though we shouldn't need one
 ; for the use in entry's copy.
-; CHECK-LABEL: @test1(
+; CHECK-LABEL: define void @test1(
 ; CHECK: entry:
 ; CHECK:   store i32 %x, i32* [[Slot:%[^ ]+]]
 ; CHECK:   invoke void @f()
@@ -56,7 +56,7 @@ exit:
 ; Need two copies of %exit's call to @f -- the subsequent ret is only
 ; valid when coming from %entry, but on the path from %cleanup, this
 ; might be a valid call to @f which might dynamically not return.
-; CHECK-LABEL: @test2(
+; CHECK-LABEL: define void @test2(
 ; CHECK: entry:
 ; CHECK:   invoke void @f()
 ; CHECK:     to label %[[exit:[^ ]+]] unwind label %cleanup
@@ -91,7 +91,7 @@ exit:
 }
 ; Need two copies of %shared's call to @f (similar to @test2 but
 ; the two regions here are siblings, not parent-child).
-; CHECK-LABEL: @test3(
+; CHECK-LABEL: define void @test3(
 ; CHECK:   invoke void @f()
 ; CHECK:   invoke void @f()
 ; CHECK:     to label %[[exit:[^ ]+]] unwind
@@ -143,7 +143,7 @@ exit:
 ; Make sure we can clone regions that have internal control
 ; flow and SSA values.  Here we need two copies of everything
 ; from %shared to %exit.
-; CHECK-LABEL: @test4(
+; CHECK-LABEL: define void @test4(
 ; CHECK:  entry:
 ; CHECK:    to label %[[shared_E:[^ ]+]] unwind label %catch
 ; CHECK:  catch:
@@ -221,7 +221,7 @@ exit:
 ; Simple nested case (catch-inside-cleanup).  Nothing needs
 ; to be cloned.  The def and use of %x are both in %outer
 ; and so don't need to be spilled.
-; CHECK-LABEL: @test5(
+; CHECK-LABEL: define void @test5(
 ; CHECK:      outer:
 ; CHECK:        %x = call i32 @g()
 ; CHECK-NEXT:   invoke void @f()
@@ -277,7 +277,7 @@ exit:
 ; %left still needs to be created because it's possible
 ; the dynamic path enters %left, then enters %inner,
 ; then calls @h, and that the call to @h doesn't return.
-; CHECK-LABEL: @test6(
+; CHECK-LABEL: define void @test6(
 ; TODO: CHECKs
 
 
@@ -309,7 +309,7 @@ unreachable:
 ; Another case of a two-parent child (like @test6), this time
 ; with the join at the entry itself instead of following a
 ; non-pad join.
-; CHECK-LABEL: @test7(
+; CHECK-LABEL: define void @test7(
 ; TODO: CHECKs
 
 
@@ -347,7 +347,7 @@ unreachable:
 }
 ; %inner is a two-parent child which itself has a child; need
 ; to make two copies of both the %inner and %inner.child.
-; CHECK-LABEL: @test8(
+; CHECK-LABEL: define void @test8(
 ; TODO: CHECKs
 
 
@@ -380,5 +380,41 @@ unreachable:
 ; the parent of the other, but that we'd somehow lost track in the CFG
 ; of which was which along the way; generating each possibility lets
 ; whichever case was correct execute correctly.
-; CHECK-LABEL: @test9(
+; CHECK-LABEL: define void @test9(
 ; TODO: CHECKs
+
+define void @test10() personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+  invoke void @f()
+    to label %unreachable unwind label %inner
+inner:
+  %cleanup = cleanuppad []
+  ; make sure we don't overlook this cleanupret and try to process
+  ; successor %outer as a child of inner.
+  cleanupret %cleanup unwind label %outer
+outer:
+  %catch = catchpad [] to label %catch.body unwind label %endpad
+catch.body:
+  catchret %catch to label %exit
+endpad:
+  catchendpad unwind to caller
+exit:
+  ret void
+unreachable:
+  unreachable
+}
+; CHECK-LABEL: define void @test10(
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   invoke
+; CHECK-NEXT:     to label %unreachable unwind label %inner
+; CHECK:      inner:
+; CHECK-NEXT:   %cleanup = cleanuppad
+; CHECK-NEXT:   cleanupret %cleanup unwind label %outer
+; CHECK:      outer:
+; CHECK-NEXT:   %catch = catchpad [] to label %catch.body unwind label %endpad
+; CHECK:      catch.body:
+; CHECK-NEXT:   catchret %catch to label %exit
+; CHECK:      endpad:
+; CHECK-NEXT:   catchendpad unwind to caller
+; CHECK:      exit:
+; CHECK-NEXT:   ret void