Index: llvm/lib/Transforms/Utils/Local.cpp =================================================================== --- llvm/lib/Transforms/Utils/Local.cpp +++ llvm/lib/Transforms/Utils/Local.cpp @@ -847,17 +847,17 @@ /// branch to Succ, into Succ. /// /// Assumption: Succ is the single successor for BB. -static bool CanPropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) { +static bool +CanPropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ, + const SmallPtrSetImpl &BBPreds) { assert(*succ_begin(BB) == Succ && "Succ is not successor of BB!"); LLVM_DEBUG(dbgs() << "Looking to fold " << BB->getName() << " into " << Succ->getName() << "\n"); // Shortcut, if there is only a single predecessor it must be BB and merging // is always safe - if (Succ->getSinglePredecessor()) return true; - - // Make a list of the predecessors of BB - SmallPtrSet BBPreds(pred_begin(BB), pred_end(BB)); + if (Succ->getSinglePredecessor()) + return true; // Look at all the phi nodes in Succ, to see if they present a conflict when // merging these blocks @@ -883,7 +883,7 @@ } } } else { - Value* Val = PN->getIncomingValueForBlock(BB); + Value *Val = PN->getIncomingValueForBlock(BB); for (unsigned PI = 0, PE = PN->getNumIncomingValues(); PI != PE; ++PI) { // See if the incoming value for the common predecessor is equal to the // one for BB, in which case this phi node will not prevent the merging @@ -997,6 +997,66 @@ } } +static bool +CanMergeEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ, + const SmallPtrSetImpl &BBPreds) { + // Check to see if merging these blocks would cause conflicts for any of the + // phi nodes in BB or Succ. If not, we can safely merge. + if (!CanPropagatePredecessorsForPHIs(BB, Succ, BBPreds)) + return false; + + // Check to see if merging these blocks would cause conflicts for any of + // the phi nodes in BB or Succ. If not, we can safely merge. + + // Check for cases where Succ has multiple predecessors and a PHI node in BB + // has uses which will not disappear when the PHI nodes are merged. It is + // possible to handle such cases, but difficult: it requires checking whether + // BB dominates Succ, which is non-trivial to calculate in the case where + // Succ has multiple predecessors. Also, it requires checking whether + // constructing the necessary self-referential PHI node doesn't introduce any + // conflicts; this isn't too difficult, but the previous code for doing this + // was incorrect. + // + // Note that if this check finds a live use, BB dominates Succ, so BB is + // something like a loop pre-header (or rarely, a part of an irreducible CFG); + // folding the branch isn't profitable in that case anyway. + + if (!Succ->getSinglePredecessor()) { + BasicBlock::iterator BBI = BB->begin(); + while (isa(*BBI)) { + for (Use &U : BBI->uses()) { + if (PHINode *PN = dyn_cast(U.getUser())) { + if (PN->getIncomingBlock(U) != BB) + return false; + } else { + return false; + } + } + ++BBI; + } + } + + return true; +} + +// Find the single common predecessor of BB and its single successor Succ. +// If there are more than 1 common predecessors, set MoreThanOnePred to be true. +static BasicBlock * +getSingleCommonPred(const SmallPtrSetImpl &BBPreds, + const SmallPtrSetImpl &SuccPreds) { + BasicBlock *Ret = nullptr; + for (BasicBlock *SuccPred : SuccPreds) { + if (BBPreds.count(SuccPred)) { + if (Ret) + return nullptr; + + Ret = SuccPred; + } + } + + return Ret; +} + /// Replace a value flowing from a block to a phi with /// potentially multiple instances of that value flowing from the /// block's predecessors to the phi. @@ -1004,9 +1064,11 @@ /// \param BB The block with the value flowing into the phi. /// \param BBPreds The predecessors of BB. /// \param PN The phi that we are updating. +/// \param CommonPred The common predecessor of BB and PN's BasicBlock static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB, const PredBlockVector &BBPreds, - PHINode *PN) { + PHINode *PN, + BasicBlock *CommonPred) { Value *OldVal = PN->removeIncomingValue(BB, false); assert(OldVal && "No entry in PHI for Pred BB!"); @@ -1034,26 +1096,39 @@ // will trigger asserts if we try to clean it up now, without also // simplifying the corresponding conditional branch). BasicBlock *PredBB = OldValPN->getIncomingBlock(i); + + if (PredBB == CommonPred) + continue; + Value *PredVal = OldValPN->getIncomingValue(i); - Value *Selected = selectIncomingValueForBlock(PredVal, PredBB, - IncomingValues); + Value *Selected = + selectIncomingValueForBlock(PredVal, PredBB, IncomingValues); // And add a new incoming value for this predecessor for the // newly retargeted branch. PN->addIncoming(Selected, PredBB); } + if (CommonPred) + PN->addIncoming(OldValPN->getIncomingValueForBlock(CommonPred), BB); + } else { for (unsigned i = 0, e = BBPreds.size(); i != e; ++i) { // Update existing incoming values in PN for this // predecessor of BB. BasicBlock *PredBB = BBPreds[i]; - Value *Selected = selectIncomingValueForBlock(OldVal, PredBB, - IncomingValues); + + if (PredBB == CommonPred) + continue; + + Value *Selected = + selectIncomingValueForBlock(OldVal, PredBB, IncomingValues); // And add a new incoming value for this predecessor for the // newly retargeted branch. PN->addIncoming(Selected, PredBB); } + if (CommonPred) + PN->addIncoming(OldVal, BB); } replaceUndefValuesInPhi(PN, IncomingValues); @@ -1064,40 +1139,32 @@ assert(BB != &BB->getParent()->getEntryBlock() && "TryToSimplifyUncondBranchFromEmptyBlock called on entry block!"); - // We can't eliminate infinite loops. + // We can't simplify infinite loops. BasicBlock *Succ = cast(BB->getTerminator())->getSuccessor(0); - if (BB == Succ) return false; + if (BB == Succ) + return false; - // Check to see if merging these blocks would cause conflicts for any of the - // phi nodes in BB or Succ. If not, we can safely merge. - if (!CanPropagatePredecessorsForPHIs(BB, Succ)) return false; + SmallPtrSet BBPreds(pred_begin(BB), pred_end(BB)); + SmallPtrSet SuccPreds(pred_begin(Succ), pred_end(Succ)); - // Check for cases where Succ has multiple predecessors and a PHI node in BB - // has uses which will not disappear when the PHI nodes are merged. It is - // possible to handle such cases, but difficult: it requires checking whether - // BB dominates Succ, which is non-trivial to calculate in the case where - // Succ has multiple predecessors. Also, it requires checking whether - // constructing the necessary self-referential PHI node doesn't introduce any - // conflicts; this isn't too difficult, but the previous code for doing this - // was incorrect. - // - // Note that if this check finds a live use, BB dominates Succ, so BB is - // something like a loop pre-header (or rarely, a part of an irreducible CFG); - // folding the branch isn't profitable in that case anyway. - if (!Succ->getSinglePredecessor()) { - BasicBlock::iterator BBI = BB->begin(); - while (isa(*BBI)) { - for (Use &U : BBI->uses()) { - if (PHINode* PN = dyn_cast(U.getUser())) { - if (PN->getIncomingBlock(U) != BB) - return false; - } else { - return false; - } - } - ++BBI; - } - } + // Find the single common predecessor of BB and Succ + BasicBlock *CommonPred = nullptr; + + bool BBKillable = CanMergeEmptyBBToSucc(BB, Succ, BBPreds); + + if (!BBKillable) + CommonPred = getSingleCommonPred(BBPreds, SuccPreds); + + bool BBPhisMergeable = + CommonPred && !BB->phis().empty() && !Succ->phis().empty(); + + if (!BBKillable && !BBPhisMergeable) + return false; + + if (BBPhisMergeable) + LLVM_DEBUG(dbgs() << "Found Common Predecessor between: " << BB->getName() + << " and " << Succ->getName() << " : " + << CommonPred->getName() << "\n"); // 'BB' and 'BB->Pred' are loop latches, bail out to presrve inner loop // metadata. @@ -1171,25 +1238,29 @@ if (PredTI->hasMetadata(LLVMContext::MD_loop)) return false; - LLVM_DEBUG(dbgs() << "Killing Trivial BB: \n" << *BB); + if (BBKillable) + LLVM_DEBUG(dbgs() << "Killing Trivial BB: \n" << *BB); + else if (BBPhisMergeable) + LLVM_DEBUG(dbgs() << "Merge Phis in Trivial BB: \n" << *BB); SmallVector Updates; + if (DTU) { // To avoid processing the same predecessor more than once. - SmallPtrSet SeenPreds; - // All predecessors of BB will be moved to Succ. - SmallPtrSet PredsOfSucc(pred_begin(Succ), pred_end(Succ)); + // All predecessors of BB (except the common predecessor) will be moved to + // Succ. Updates.reserve(Updates.size() + 2 * pred_size(BB) + 1); - for (auto *PredOfBB : predecessors(BB)) + + for (auto *PredOfBB : BBPreds) + Updates.push_back({DominatorTree::Delete, PredOfBB, BB}); + + for (auto *PredOfBB : BBPreds) // This predecessor of BB may already have Succ as a successor. - if (!PredsOfSucc.contains(PredOfBB)) - if (SeenPreds.insert(PredOfBB).second) - Updates.push_back({DominatorTree::Insert, PredOfBB, Succ}); - SeenPreds.clear(); - for (auto *PredOfBB : predecessors(BB)) - if (SeenPreds.insert(PredOfBB).second) - Updates.push_back({DominatorTree::Delete, PredOfBB, BB}); - Updates.push_back({DominatorTree::Delete, BB, Succ}); + if (!SuccPreds.contains(PredOfBB)) + Updates.push_back({DominatorTree::Insert, PredOfBB, Succ}); + + if (BBKillable) + Updates.push_back({DominatorTree::Delete, BB, Succ}); } if (isa(Succ->begin())) { @@ -1202,7 +1273,7 @@ for (BasicBlock::iterator I = Succ->begin(); isa(I); ++I) { PHINode *PN = cast(I); - redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN); + redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN, CommonPred); } } @@ -1215,7 +1286,7 @@ Succ->splice(Succ->getFirstNonPHI()->getIterator(), BB); } else { while (PHINode *PN = dyn_cast(&BB->front())) { - // We explicitly check for such uses in CanPropagatePredecessorsForPHIs. + // We explicitly check for such uses for merging phis. assert(PN->use_empty() && "There shouldn't be any uses here!"); PN->eraseFromParent(); } @@ -1228,21 +1299,35 @@ for (BasicBlock *Pred : predecessors(BB)) Pred->getTerminator()->setMetadata(LLVMContext::MD_loop, LoopMD); - // Everything that jumped to BB now goes to Succ. - BB->replaceAllUsesWith(Succ); - if (!Succ->hasName()) Succ->takeName(BB); - - // Clear the successor list of BB to match updates applying to DTU later. - if (BB->getTerminator()) - BB->back().eraseFromParent(); - new UnreachableInst(BB->getContext(), BB); - assert(succ_empty(BB) && "The successor list of BB isn't empty before " - "applying corresponding DTU updates."); + if (BBKillable) { + // Everything that jumped to BB now goes to Succ. + BB->replaceAllUsesWith(Succ); + + if (!Succ->hasName()) + Succ->takeName(BB); + + // Clear the successor list of BB to match updates applying to DTU later. + if (BB->getTerminator()) + BB->back().eraseFromParent(); + + new UnreachableInst(BB->getContext(), BB); + assert(succ_empty(BB) && "The successor list of BB isn't empty before " + "applying corresponding DTU updates."); + } else if (BBPhisMergeable) { + // Everything except CommonPred that jumped to BB now goes to Succ. + BB->replaceUsesWithIf(Succ, [BBPreds, CommonPred](Use &use) -> bool { + if (Instruction *UseInst = dyn_cast(use.getUser())) + return UseInst->getParent() != CommonPred && + BBPreds.contains(UseInst->getParent()); + return false; + }); + } if (DTU) DTU->applyUpdates(Updates); - DeleteDeadBlock(BB, DTU); + if (BBKillable) + DeleteDeadBlock(BB, DTU); return true; } Index: llvm/test/CodeGen/ARM/jump-table-islands.ll =================================================================== --- llvm/test/CodeGen/ARM/jump-table-islands.ll +++ llvm/test/CodeGen/ARM/jump-table-islands.ll @@ -1,7 +1,9 @@ -; RUN: llc -mtriple=armv7-apple-ios8.0 -o - %s | FileCheck %s +; RUN: llc -arm-atomic-cfg-tidy=0 -mtriple=armv7-apple-ios8.0 -o - %s | FileCheck %s %BigInt = type i8500 +; FIXME: SimplifyCFG optimizes the CFG here, resulting different asm. +; Update is needed. define %BigInt @test_moved_jumptable(i1 %tst, i32 %sw, %BigInt %l) { ; CHECK-LABEL: test_moved_jumptable: Index: llvm/test/Transforms/SimplifyCFG/merge-phis-in-switch.ll =================================================================== --- llvm/test/Transforms/SimplifyCFG/merge-phis-in-switch.ll +++ llvm/test/Transforms/SimplifyCFG/merge-phis-in-switch.ll @@ -17,12 +17,11 @@ ; CHECK: unreachable: ; CHECK-NEXT: unreachable ; CHECK: case1: -; CHECK-NEXT: br label [[CASE01]] +; CHECK-NEXT: br label [[END]] ; CHECK: case01: -; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ 2, [[CASE1]] ], [ 1, [[START:%.*]] ] ; CHECK-NEXT: br label [[END]] ; CHECK: end: -; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ [[PHI1]], [[CASE01]] ], [ 3, [[START]] ] +; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ 3, [[START:%.*]] ], [ 2, [[CASE1]] ], [ 1, [[CASE01]] ] ; CHECK-NEXT: ret i8 [[PHI2]] ; start: @@ -58,14 +57,13 @@ ; CHECK: unreachable: ; CHECK-NEXT: unreachable ; CHECK: case1: -; CHECK-NEXT: br label [[CASE012]] +; CHECK-NEXT: br label [[END]] ; CHECK: case2: -; CHECK-NEXT: br label [[CASE012]] +; CHECK-NEXT: br label [[END]] ; CHECK: case012: -; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ 3, [[CASE2]] ], [ 2, [[CASE1]] ], [ 1, [[START:%.*]] ] ; CHECK-NEXT: br label [[END]] ; CHECK: end: -; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ [[PHI1]], [[CASE012]] ], [ 4, [[START]] ] +; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ 4, [[START:%.*]] ], [ 3, [[CASE2]] ], [ 2, [[CASE1]] ], [ 1, [[CASE012]] ] ; CHECK-NEXT: ret i8 [[PHI2]] ; start: @@ -105,17 +103,15 @@ ; CHECK: unreachable: ; CHECK-NEXT: unreachable ; CHECK: case1: -; CHECK-NEXT: br label [[CASE012]] +; CHECK-NEXT: br label [[END]] ; CHECK: case2: -; CHECK-NEXT: br label [[CASE012]] +; CHECK-NEXT: br label [[END]] ; CHECK: case012: -; CHECK-NEXT: [[PHI1_1:%.*]] = phi i8 [ 3, [[CASE2]] ], [ 2, [[CASE1]] ], [ 1, [[START:%.*]] ] -; CHECK-NEXT: [[PHI1_2:%.*]] = phi i8 [ 6, [[CASE2]] ], [ 5, [[CASE1]] ], [ 4, [[START]] ] ; CHECK-NEXT: br label [[END]] ; CHECK: end: -; CHECK-NEXT: [[PHI2_1:%.*]] = phi i8 [ [[PHI1_1]], [[CASE012]] ], [ 4, [[START]] ] -; CHECK-NEXT: [[PHI2_2:%.*]] = phi i8 [ [[PHI1_1]], [[CASE012]] ], [ 5, [[START]] ] -; CHECK-NEXT: [[PHI2_3:%.*]] = phi i8 [ [[PHI1_2]], [[CASE012]] ], [ 3, [[START]] ] +; CHECK-NEXT: [[PHI2_1:%.*]] = phi i8 [ 4, [[START:%.*]] ], [ 3, [[CASE2]] ], [ 2, [[CASE1]] ], [ 1, [[CASE012]] ] +; CHECK-NEXT: [[PHI2_2:%.*]] = phi i8 [ 5, [[START]] ], [ 3, [[CASE2]] ], [ 2, [[CASE1]] ], [ 1, [[CASE012]] ] +; CHECK-NEXT: [[PHI2_3:%.*]] = phi i8 [ 3, [[START]] ], [ 6, [[CASE2]] ], [ 5, [[CASE1]] ], [ 4, [[CASE012]] ] ; CHECK-NEXT: call void @use(i8 [[PHI2_1]]) ; CHECK-NEXT: call void @use(i8 [[PHI2_2]]) ; CHECK-NEXT: call void @use(i8 [[PHI2_3]]) @@ -166,19 +162,17 @@ ; CHECK: unreachable: ; CHECK-NEXT: unreachable ; CHECK: case0: -; CHECK-NEXT: br label [[CASE0123]] +; CHECK-NEXT: br label [[END]] ; CHECK: case1: -; CHECK-NEXT: br label [[CASE0123]] +; CHECK-NEXT: br label [[END]] ; CHECK: case2: -; CHECK-NEXT: br label [[CASE0123]] +; CHECK-NEXT: br label [[END]] ; CHECK: case0123: -; CHECK-NEXT: [[PHI1:%.*]] = phi i8 [ 4, [[START:%.*]] ], [ 3, [[CASE2]] ], [ 2, [[CASE1]] ], [ 1, [[CASE0]] ] -; CHECK-NEXT: br label [[CASE01234]] +; CHECK-NEXT: br label [[END]] ; CHECK: case01234: -; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ [[PHI1]], [[CASE0123]] ], [ 5, [[START]] ] ; CHECK-NEXT: br label [[END]] ; CHECK: end: -; CHECK-NEXT: [[PHI3:%.*]] = phi i8 [ [[PHI2]], [[CASE01234]] ], [ 6, [[START]] ] +; CHECK-NEXT: [[PHI3:%.*]] = phi i8 [ 6, [[START:%.*]] ], [ 3, [[CASE2]] ], [ 2, [[CASE1]] ], [ 1, [[CASE0]] ], [ 4, [[CASE0123]] ], [ 5, [[CASE01234]] ] ; CHECK-NEXT: ret i8 [[PHI3]] ; start: @@ -231,21 +225,19 @@ ; CHECK: unreachable: ; CHECK-NEXT: unreachable ; CHECK: case0: -; CHECK-NEXT: br label [[CASE012]] +; CHECK-NEXT: br label [[CASE0123456]] ; CHECK: case1: -; CHECK-NEXT: br label [[CASE012]] +; CHECK-NEXT: br label [[CASE0123456]] ; CHECK: case012: -; CHECK-NEXT: [[PHI123:%.*]] = phi i8 [ 3, [[START:%.*]] ], [ 2, [[CASE1]] ], [ 1, [[CASE0]] ] ; CHECK-NEXT: br label [[CASE0123456]] ; CHECK: case3: -; CHECK-NEXT: br label [[CASE345]] +; CHECK-NEXT: br label [[CASE0123456]] ; CHECK: case4: -; CHECK-NEXT: br label [[CASE345]] +; CHECK-NEXT: br label [[CASE0123456]] ; CHECK: case345: -; CHECK-NEXT: [[PHI456:%.*]] = phi i8 [ 6, [[START]] ], [ 5, [[CASE4]] ], [ 4, [[CASE3]] ] ; CHECK-NEXT: br label [[CASE0123456]] ; CHECK: case0123456: -; CHECK-NEXT: [[PHI1234567:%.*]] = phi i8 [ 7, [[START]] ], [ [[PHI456]], [[CASE345]] ], [ [[PHI123]], [[CASE012]] ] +; CHECK-NEXT: [[PHI1234567:%.*]] = phi i8 [ 7, [[START:%.*]] ], [ 2, [[CASE1]] ], [ 1, [[CASE0]] ], [ 3, [[CASE012]] ], [ 5, [[CASE4]] ], [ 4, [[CASE3]] ], [ 6, [[CASE345]] ] ; CHECK-NEXT: ret i8 [[PHI1234567]] ; start: Index: llvm/test/Transforms/SimplifyCFG/switch-simplify-crash2.ll =================================================================== --- llvm/test/Transforms/SimplifyCFG/switch-simplify-crash2.ll +++ llvm/test/Transforms/SimplifyCFG/switch-simplify-crash2.ll @@ -6,6 +6,9 @@ ; CHECK-NEXT: entry: ; CHECK-NEXT: br label [[LOOP2:%.*]] ; CHECK: loop2: +; CHECK-NEXT: [[PHI2:%.*]] = phi i8 [ 0, [[ENTRY:%.*]] ], [ [[SPEC_SELECT:%.*]], [[LOOP2]] ] +; CHECK-NEXT: [[COND:%.*]] = icmp eq i8 [[PHI2]], 0 +; CHECK-NEXT: [[SPEC_SELECT]] = select i1 [[COND]], i8 0, i8 [[PHI2]] ; CHECK-NEXT: br label [[LOOP2]] ; entry: