From 883735e75af662c1878bf08537d2c621efa802a0 Mon Sep 17 00:00:00 2001 From: Joseph Tremoulet Date: Thu, 10 Sep 2015 16:51:25 +0000 Subject: [PATCH] [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 --- lib/CodeGen/WinEHPrepare.cpp | 17 ++++----- test/CodeGen/WinEH/wineh-cloning.ll | 54 ++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/lib/CodeGen/WinEHPrepare.cpp b/lib/CodeGen/WinEHPrepare.cpp index ea43cb2ffeb..3f666c944e2 100644 --- 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(Terminator) || - isa(Terminator) || - isa(Terminator)) { - // These block's successors have already been queued with the parent - // color. - continue; - } + } + + TerminatorInst *Terminator = Visiting->getTerminator(); + if (isa(Terminator) || + isa(Terminator) || + isa(Terminator)) { + // These blocks' successors have already been queued with the parent + // color. + continue; } for (BasicBlock *Succ : successors(Visiting)) { if (isa(Succ->getFirstNonPHI())) { diff --git a/test/CodeGen/WinEH/wineh-cloning.ll b/test/CodeGen/WinEH/wineh-cloning.ll index 39e5d40a48f..f7fe58f844f 100644 --- 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 -- 2.34.1