Index: llvm/trunk/lib/Transforms/Scalar/ADCE.cpp =================================================================== --- llvm/trunk/lib/Transforms/Scalar/ADCE.cpp +++ llvm/trunk/lib/Transforms/Scalar/ADCE.cpp @@ -17,6 +17,7 @@ #include "llvm/Transforms/Scalar/ADCE.h" #include "llvm/ADT/DepthFirstIterator.h" +#include "llvm/ADT/PostOrderIterator.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" @@ -26,6 +27,7 @@ #include "llvm/IR/BasicBlock.h" #include "llvm/IR/CFG.h" #include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/IRBuilder.h" #include "llvm/IR/InstIterator.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicInst.h" @@ -37,11 +39,17 @@ #define DEBUG_TYPE "adce" STATISTIC(NumRemoved, "Number of instructions removed"); +STATISTIC(NumBranchesRemoved, "Number of branch instructions removed"); // This is a tempoary option until we change the interface // to this pass based on optimization level. static cl::opt RemoveControlFlowFlag("adce-remove-control-flow", - cl::init(false), cl::Hidden); + cl::init(true), cl::Hidden); + +// This option enables removing of may-be-infinite loops which have no other +// effect. +static cl::opt RemoveLoops("adce-remove-loops", cl::init(false), + cl::Hidden); namespace { /// Information about Instructions @@ -72,8 +80,11 @@ /// Corresponding BasicBlock. BasicBlock *BB = nullptr; - /// Cache of BB->getTerminator() + /// Cache of BB->getTerminator(). TerminatorInst *Terminator = nullptr; + + /// Post-order numbering of reverse control flow graph. + unsigned PostOrder; }; class AggressiveDeadCodeElimination { @@ -97,8 +108,9 @@ /// Set of blocks with not known to have live terminators. SmallPtrSet BlocksWithDeadTerminators; - /// The set of blocks which we have determined are live in the - /// most recent iteration of propagating liveness. + /// The set of blocks which we have determined whose control + /// dependence sources must be live and which have not had + /// those dependences analyized. SmallPtrSet NewLiveBlocks; /// Set up auxiliary data structures for Instructions and BasicBlocks and @@ -113,7 +125,10 @@ void markLiveInstructions(); /// Mark an instruction as live. void markLive(Instruction *I); - + /// Mark a block as live. + void markLive(BlockInfoType &BB); + void markLive(BasicBlock *BB) { markLive(BlockInfo[BB]); } + /// Mark terminators of control predecessors of a PHI node live. void markPhiLive(PHINode *PN); @@ -130,6 +145,17 @@ /// was removed. bool removeDeadInstructions(); + /// Identify connected sections of the control flow grap which have + /// dead terminators and rewrite the control flow graph to remove them. + void updateDeadRegions(); + + /// Set the BlockInfo::PostOrder field based on a post-order + /// numbering of the reverse control flow graph. + void computeReversePostOrder(); + + /// Make the terminator of this block an unconditional branch to \p Target. + void makeUnconditional(BasicBlock *BB, BasicBlock *Target); + public: AggressiveDeadCodeElimination(Function &F, PostDominatorTree &PDT) : F(F), PDT(PDT) {} @@ -144,7 +170,7 @@ } static bool isUnconditionalBranch(TerminatorInst *Term) { - auto BR = dyn_cast(Term); + auto *BR = dyn_cast(Term); return BR && BR->isUnconditional(); } @@ -186,23 +212,65 @@ if (!RemoveControlFlowFlag) return; - // This is temporary: will update with post order traveral to - // find loop bottoms - SmallPtrSet Seen; - for (auto &BB : F) { - Seen.insert(&BB); - TerminatorInst *Term = BB.getTerminator(); - if (isLive(Term)) - continue; + if (!RemoveLoops) { + // This stores state for the depth-first iterator. In addition + // to recording which nodes have been visited we also record whether + // a node is currently on the "stack" of active ancestors of the current + // node. + typedef DenseMap StatusMap ; + class DFState : public StatusMap { + public: + std::pair insert(BasicBlock *BB) { + return StatusMap::insert(std::make_pair(BB, true)); + } - for (auto Succ : successors(&BB)) - if (Seen.count(Succ)) { - // back edge.... - markLive(Term); + // Invoked after we have visited all children of a node. + void completed(BasicBlock *BB) { (*this)[BB] = false; } + + // Return true if \p BB is currently on the active stack + // of ancestors. + bool onStack(BasicBlock *BB) { + auto Iter = find(BB); + return Iter != end() && Iter->second; + } + } State; + + State.reserve(F.size()); + // Iterate over blocks in depth-first pre-order and + // treat all edges to a block already seen as loop back edges + // and mark the branch live it if there is a back edge. + for (auto *BB: depth_first_ext(&F.getEntryBlock(), State)) { + TerminatorInst *Term = BB->getTerminator(); + if (isLive(Term)) + continue; + + for (auto *Succ : successors(BB)) + if (State.onStack(Succ)) { + // back edge.... + markLive(Term); + break; + } + } + } + + // Mark blocks live if there is no path from the block to the + // return of the function or a successor for which this is true. + // This protects IDFCalculator which cannot handle such blocks. + for (auto &BBInfoPair : BlockInfo) { + auto &BBInfo = BBInfoPair.second; + if (BBInfo.terminatorIsLive()) + continue; + auto *BB = BBInfo.BB; + if (!PDT.getNode(BB)) { + markLive(BBInfo.Terminator); + continue; + } + for (auto *Succ : successors(BB)) + if (!PDT.getNode(Succ)) { + markLive(BBInfo.Terminator); break; } } - // End temporary handling of loops. // Mark blocks live if there is no path from the block to the // return of the function or a successor for which this is true. @@ -218,7 +286,7 @@ markLive(BBInfo.Terminator); continue; } - for (auto Succ : successors(BB)) + for (auto *Succ : successors(BB)) if (!PDT.getNode(Succ)) { DEBUG(dbgs() << "Successor not post-dominated by return: " << BB->getName() << '\n';); @@ -278,32 +346,19 @@ Instruction *LiveInst = Worklist.pop_back_val(); DEBUG(dbgs() << "work live: "; LiveInst->dump();); - // Collect the live debug info scopes attached to this instruction. - if (const DILocation *DL = LiveInst->getDebugLoc()) - collectLiveScopes(*DL); - for (Use &OI : LiveInst->operands()) if (Instruction *Inst = dyn_cast(OI)) markLive(Inst); - + if (auto *PN = dyn_cast(LiveInst)) markPhiLive(PN); } + + // After data flow liveness has been identified, examine which branch + // decisions are required to determine live instructions are executed. markLiveBranchesFromControlDependences(); - if (Worklist.empty()) { - // Temporary until we can actually delete branches. - SmallVector DeadTerminators; - for (auto *BB : BlocksWithDeadTerminators) - DeadTerminators.push_back(BB->getTerminator()); - for (auto *I : DeadTerminators) - markLive(I); - assert(BlocksWithDeadTerminators.empty()); - // End temporary. - } } while (!Worklist.empty()); - - assert(BlocksWithDeadTerminators.empty()); } void AggressiveDeadCodeElimination::markLive(Instruction *I) { @@ -316,13 +371,26 @@ Info.Live = true; Worklist.push_back(I); + // Collect the live debug info scopes attached to this instruction. + if (const DILocation *DL = I->getDebugLoc()) + collectLiveScopes(*DL); + // Mark the containing block live auto &BBInfo = *Info.Block; - if (BBInfo.Terminator == I) + if (BBInfo.Terminator == I) { BlocksWithDeadTerminators.erase(BBInfo.BB); + // For live terminators, mark destination blocks + // live to preserve this control flow edges. + if (!BBInfo.UnconditionalBranch) + for (auto *BB : successors(I->getParent())) + markLive(BB); + } + markLive(BBInfo); +} + +void AggressiveDeadCodeElimination::markLive(BlockInfoType &BBInfo) { if (BBInfo.Live) return; - DEBUG(dbgs() << "mark block live: " << BBInfo.BB->getName() << '\n'); BBInfo.Live = true; if (!BBInfo.CFLive) { @@ -332,7 +400,7 @@ // Mark unconditional branches at the end of live // blocks as live since there is no work to do for them later - if (BBInfo.UnconditionalBranch && I != BBInfo.Terminator) + if (BBInfo.UnconditionalBranch) markLive(BBInfo.Terminator); } @@ -408,7 +476,7 @@ NewLiveBlocks.clear(); // Dead terminators which control live blocks are now marked live. - for (auto BB : IDFBlocks) { + for (auto *BB : IDFBlocks) { DEBUG(dbgs() << "live control in: " << BB->getName() << '\n'); markLive(BB->getTerminator()); } @@ -421,8 +489,33 @@ //===----------------------------------------------------------------------===// bool AggressiveDeadCodeElimination::removeDeadInstructions() { + // Updates control and dataflow around dead blocks + updateDeadRegions(); + + DEBUG({ + for (Instruction &I : instructions(F)) { + // Check if the instruction is alive. + if (isLive(&I)) + continue; + + if (auto *DII = dyn_cast(&I)) { + // Check if the scope of this variable location is alive. + if (AliveScopes.count(DII->getDebugLoc()->getScope())) + continue; + + // If intrinsic is pointing at a live SSA value, there may be an + // earlier optimization bug: if we know the location of the variable, + // why isn't the scope of the location alive? + if (Value *V = DII->getVariableLocation()) + if (Instruction *II = dyn_cast(V)) + if (isLive(II)) + dbgs() << "Dropping debug info for " << *DII << "\n"; + } + } + }); + // The inverse of the live set is the dead set. These are those instructions - // which have no side effects and do not influence the control flow or return + // that have no side effects and do not influence the control flow or return // value of the function, and may therefore be deleted safely. // NOTE: We reuse the Worklist vector here for memory efficiency. for (Instruction &I : instructions(F)) { @@ -430,23 +523,12 @@ if (isLive(&I)) continue; - assert(!I.isTerminator() && "NYI: Removing Control Flow"); - if (auto *DII = dyn_cast(&I)) { // Check if the scope of this variable location is alive. if (AliveScopes.count(DII->getDebugLoc()->getScope())) continue; // Fallthrough and drop the intrinsic. - DEBUG({ - // If intrinsic is pointing at a live SSA value, there may be an - // earlier optimization bug: if we know the location of the variable, - // why isn't the scope of the location alive? - if (Value *V = DII->getVariableLocation()) - if (Instruction *II = dyn_cast(V)) - if (isLive(II)) - dbgs() << "Dropping debug info for " << *DII << "\n"; - }); } // Prepare to delete. @@ -462,6 +544,96 @@ return !Worklist.empty(); } +// A dead region is the set of dead blocks with a common live post-dominator. +void AggressiveDeadCodeElimination::updateDeadRegions() { + + DEBUG({ + dbgs() << "final dead terminator blocks: " << '\n'; + for (auto *BB : BlocksWithDeadTerminators) + dbgs() << '\t' << BB->getName() + << (BlockInfo[BB].Live ? " LIVE\n" : "\n"); + }); + + // Don't compute the post ordering unless we needed it. + bool HavePostOrder = false; + + for (auto *BB : BlocksWithDeadTerminators) { + auto &Info = BlockInfo[BB]; + if (Info.UnconditionalBranch) { + InstInfo[Info.Terminator].Live = true; + continue; + } + + if (!HavePostOrder) { + computeReversePostOrder(); + HavePostOrder = true; + } + + // Add an unconditional branch to the successor closest to the + // end of the function which insures a path to the exit for each + // live edge. + BlockInfoType *PreferredSucc = nullptr; + for (auto *Succ : successors(BB)) { + auto *Info = &BlockInfo[Succ]; + if (!PreferredSucc || PreferredSucc->PostOrder < Info->PostOrder) + PreferredSucc = Info; + } + assert((PreferredSucc && PreferredSucc->PostOrder > 0) && + "Failed to find safe successor for dead branc"); + bool First = true; + for (auto *Succ : successors(BB)) { + if (!First || Succ != PreferredSucc->BB) + Succ->removePredecessor(BB); + else + First = false; + } + makeUnconditional(BB, PreferredSucc->BB); + NumBranchesRemoved += 1; + } +} + +// reverse top-sort order +void AggressiveDeadCodeElimination::computeReversePostOrder() { + + // This provides a post-order numbering of the reverse conrtol flow graph + // Note that it is incomplete in the presence of infinite loops but we don't + // need numbers blocks which don't reach the end of the functions since + // all branches in those blocks are forced live. + + // For each block without successors, extend the DFS from the bloack + // backward through the graph + SmallPtrSet Visited; + unsigned PostOrder = 0; + for (auto &BB : F) { + if (succ_begin(&BB) != succ_end(&BB)) + continue; + for (BasicBlock *Block : inverse_post_order_ext(&BB,Visited)) + BlockInfo[Block].PostOrder = PostOrder++; + } +} + +void AggressiveDeadCodeElimination::makeUnconditional(BasicBlock *BB, + BasicBlock *Target) { + TerminatorInst *PredTerm = BB->getTerminator(); + // Collect the live debug info scopes attached to this instruction. + if (const DILocation *DL = PredTerm->getDebugLoc()) + collectLiveScopes(*DL); + + // Just mark live an existing unconditional branch + if (isUnconditionalBranch(PredTerm)) { + PredTerm->setSuccessor(0, Target); + InstInfo[PredTerm].Live = true; + return; + } + DEBUG(dbgs() << "making unconditional " << BB->getName() << '\n'); + NumBranchesRemoved += 1; + IRBuilder<> Builder(PredTerm); + auto *NewTerm = Builder.CreateBr(Target); + InstInfo[NewTerm].Live = true; + if (const DILocation *DL = PredTerm->getDebugLoc()) + NewTerm->setDebugLoc(DL); +} + //===----------------------------------------------------------------------===// // // Pass Manager integration code @@ -494,7 +666,8 @@ void getAnalysisUsage(AnalysisUsage &AU) const override { AU.addRequired(); - AU.setPreservesCFG(); // TODO -- will remove when we start removing branches + if (!RemoveControlFlowFlag) + AU.setPreservesCFG(); AU.addPreserved(); } }; Index: llvm/trunk/test/Transforms/ADCE/2002-05-23-ZeroArgPHITest.ll =================================================================== --- llvm/trunk/test/Transforms/ADCE/2002-05-23-ZeroArgPHITest.ll +++ llvm/trunk/test/Transforms/ADCE/2002-05-23-ZeroArgPHITest.ll @@ -4,7 +4,8 @@ ; removed even though there were uses still around. Now the uses are filled ; in with a dummy value before the PHI is deleted. ; -; RUN: opt < %s -adce +; RUN: opt < %s -S -adce | grep bb1 +; RUN: opt < %s -S -adce -adce-remove-loops | FileCheck %s %node_t = type { double*, %node_t*, %node_t**, double**, double*, i32, i32 } @@ -17,6 +18,7 @@ bb1: ; preds = %bb0 %reg107 = load %node_t*, %node_t** %nodelist.upgrd.1 ; <%node_t*> [#uses=2] %cond211 = icmp eq %node_t* %reg107, null ; [#uses=1] +; CHECK: br label %bb3 br i1 %cond211, label %bb3, label %bb2 bb2: ; preds = %bb2, %bb1 @@ -24,6 +26,7 @@ %reg212 = getelementptr %node_t, %node_t* %reg109, i64 0, i32 1 ; <%node_t**> [#uses=1] %reg110 = load %node_t*, %node_t** %reg212 ; <%node_t*> [#uses=2] %cond213 = icmp ne %node_t* %reg110, null ; [#uses=1] +; CHECK: br label %bb3 br i1 %cond213, label %bb2, label %bb3 bb3: ; preds = %bb2, %bb1 Index: llvm/trunk/test/Transforms/ADCE/2002-05-28-Crash-distilled.ll =================================================================== --- llvm/trunk/test/Transforms/ADCE/2002-05-28-Crash-distilled.ll +++ llvm/trunk/test/Transforms/ADCE/2002-05-28-Crash-distilled.ll @@ -1,6 +1,7 @@ ; This testcase is a distilled form of: 2002-05-28-Crash.ll ; RUN: opt < %s -adce +; RUN: opt < %s -adce -adce-remove-loops -S | FileCheck %s define float @test(i32 %i) { %F = sitofp i32 %i to float ; [#uses=1] @@ -9,6 +10,7 @@ Loop: ; preds = %Loop, %0 %B = icmp ne i32 %I, 0 ; [#uses=1] +; CHECK: br label %Out br i1 %B, label %Out, label %Loop Out: ; preds = %Loop Index: llvm/trunk/test/Transforms/ADCE/2002-05-28-Crash.ll =================================================================== --- llvm/trunk/test/Transforms/ADCE/2002-05-28-Crash.ll +++ llvm/trunk/test/Transforms/ADCE/2002-05-28-Crash.ll @@ -12,6 +12,7 @@ ;} ; ; RUN: opt < %s -adce +; RUN: opt < %s -adce -adce-remove-loops -S | FileCheck %s define i32 @rx_bitset_empty(i32 %size, i32* %set) { bb1: @@ -28,6 +29,7 @@ %reg124 = getelementptr i32, i32* %set, i64 %reg114-idxcast-offset.upgrd.1 ; [#uses=1] %reg125 = load i32, i32* %reg124 ; [#uses=1] %cond232 = icmp ne i32 %reg125, 0 ; [#uses=1] +; CHECK: br label %bb3 br i1 %cond232, label %bb3, label %bb2 bb2: ; preds = %bb2, %bb1 Index: llvm/trunk/test/Transforms/ADCE/2002-07-17-AssertionFailure.ll =================================================================== --- llvm/trunk/test/Transforms/ADCE/2002-07-17-AssertionFailure.ll +++ llvm/trunk/test/Transforms/ADCE/2002-07-17-AssertionFailure.ll @@ -3,11 +3,12 @@ ; block in this function, it would work fine, but that would be the part we ; have to fix now, wouldn't it.... ; -; RUN: opt < %s -adce +; RUN: opt < %s -adce -S | FileCheck %s define void @foo(i8* %reg5481) { %cast611 = bitcast i8* %reg5481 to i8** ; [#uses=1] %reg162 = load i8*, i8** %cast611 ; [#uses=1] +; CHECK-NOT: ptrtoint ptrtoint i8* %reg162 to i32 ; :1 [#uses=0] ret void } Index: llvm/trunk/test/Transforms/ADCE/2002-07-17-PHIAssertion.ll =================================================================== --- llvm/trunk/test/Transforms/ADCE/2002-07-17-PHIAssertion.ll +++ llvm/trunk/test/Transforms/ADCE/2002-07-17-PHIAssertion.ll @@ -1,6 +1,6 @@ ; This testcase was extracted from the gzip SPEC benchmark ; -; RUN: opt < %s -adce +; RUN: opt < %s -adce | FileCheck %s @bk = external global i32 ; [#uses=2] @hufts = external global i32 ; [#uses=1] @@ -16,6 +16,8 @@ bb3: ; preds = %bb2 br label %UnifiedExitNode +; CHECK-NOT: bb4: +; CHECK-NOT: bb5: bb4: ; preds = %bb2 %reg117 = load i32, i32* @hufts ; [#uses=2] %cond241 = icmp ule i32 %reg117, %reg128 ; [#uses=1] Index: llvm/trunk/test/Transforms/ADCE/2002-07-29-Segfault.ll =================================================================== --- llvm/trunk/test/Transforms/ADCE/2002-07-29-Segfault.ll +++ llvm/trunk/test/Transforms/ADCE/2002-07-29-Segfault.ll @@ -1,4 +1,5 @@ ; RUN: opt < %s -adce -disable-output +; RUN: opt < %s -adce -disable-output -adce-remove-loops define void @test() { br label %BB3 Index: llvm/trunk/test/Transforms/ADCE/2003-01-22-PredecessorProblem.ll =================================================================== --- llvm/trunk/test/Transforms/ADCE/2003-01-22-PredecessorProblem.ll +++ llvm/trunk/test/Transforms/ADCE/2003-01-22-PredecessorProblem.ll @@ -1,5 +1,6 @@ ; Testcase reduced from 197.parser by bugpoint ; RUN: opt < %s -adce +; RUN: opt < %s -adce -adce-remove-loops -S | FileCheck %s define void @conjunction_prune() { ;