From: Nick Lewycky Date: Fri, 3 Jun 2011 06:27:15 +0000 (+0000) Subject: Bail on unswitching a switch statement for a case with a critical edge. We name X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=444f297a782047fbe6255a3543bc3ccd025c0691;p=oota-llvm.git Bail on unswitching a switch statement for a case with a critical edge. We name which edge to split by pred/succ pair, which means that we can end up splitting the wrong edge (by case value) in the switch statement entirely. Fixes PR10031! git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@132535 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Transforms/Scalar/LoopUnswitch.cpp b/lib/Transforms/Scalar/LoopUnswitch.cpp index 01a99af1439..e05f29c3e13 100644 --- a/lib/Transforms/Scalar/LoopUnswitch.cpp +++ b/lib/Transforms/Scalar/LoopUnswitch.cpp @@ -258,6 +258,7 @@ bool LoopUnswitch::processCurrentLoop() { if (LoopCond && SI->getNumCases() > 1) { // Find a value to unswitch on: // FIXME: this should chose the most expensive case! + // FIXME: scan for a case with a non-critical edge? Constant *UnswitchVal = SI->getCaseValue(1); // Do not process same value again and again. if (!UnswitchedVals.insert(UnswitchVal)) @@ -560,6 +561,8 @@ void LoopUnswitch::SplitExitEdges(Loop *L, BasicBlock *ExitBlock = ExitBlocks[i]; SmallVector Preds(pred_begin(ExitBlock), pred_end(ExitBlock)); + // Although SplitBlockPredecessors doesn't preserve loop-simplify in + // general, if we call it on all predecessors of all exits then it does. SplitBlockPredecessors(ExitBlock, Preds.data(), Preds.size(), ".us-lcssa", this); } @@ -915,10 +918,11 @@ void LoopUnswitch::RewriteLoopBodyWithConditionConstant(Loop *L, Value *LIC, // Found a dead case value. Don't remove PHI nodes in the // successor if they become single-entry, those PHI nodes may // be in the Users list. - + BasicBlock *Switch = SI->getParent(); BasicBlock *SISucc = SI->getSuccessor(DeadCase); BasicBlock *Latch = L->getLoopLatch(); + if (!SI->findCaseDest(SISucc)) continue; // Edge is critical. // If the DeadCase successor dominates the loop latch, then the // transformation isn't safe since it will delete the sole predecessor edge // to the latch. diff --git a/test/Transforms/LoopUnswitch/2011-06-02-CritSwitch.ll b/test/Transforms/LoopUnswitch/2011-06-02-CritSwitch.ll new file mode 100644 index 00000000000..61c54ddb156 --- /dev/null +++ b/test/Transforms/LoopUnswitch/2011-06-02-CritSwitch.ll @@ -0,0 +1,28 @@ +; RUN: opt -loop-unswitch -disable-output +; PR10031 + +define i32 @test(i32 %command) { +entry: + br label %tailrecurse + +tailrecurse: ; preds = %if.then14, %tailrecurse, %entry + br i1 undef, label %if.then, label %tailrecurse + +if.then: ; preds = %tailrecurse + switch i32 %command, label %sw.bb [ + i32 2, label %land.lhs.true + i32 0, label %land.lhs.true + ] + +land.lhs.true: ; preds = %if.then, %if.then + br i1 undef, label %sw.bb, label %if.then14 + +if.then14: ; preds = %land.lhs.true + switch i32 %command, label %tailrecurse [ + i32 0, label %sw.bb + i32 1, label %sw.bb + ] + +sw.bb: ; preds = %if.then14 + unreachable +}