Index: llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h =================================================================== --- llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h +++ llvm/trunk/include/llvm/Transforms/Scalar/Reassociate.h @@ -65,7 +65,7 @@ PreservedAnalyses run(Function &F, FunctionAnalysisManager &); private: - void BuildRankMap(Function &F); + void BuildRankMap(Function &F, ReversePostOrderTraversal &RPOT); unsigned getRank(Value *V); void canonicalizeOperands(Instruction *I); void ReassociateExpression(BinaryOperator *I); Index: llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp =================================================================== --- llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp +++ llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp @@ -145,7 +145,8 @@ return nullptr; } -void ReassociatePass::BuildRankMap(Function &F) { +void ReassociatePass::BuildRankMap(Function &F, + ReversePostOrderTraversal &RPOT) { unsigned i = 2; // Assign distinct ranks to function arguments. @@ -154,7 +155,7 @@ DEBUG(dbgs() << "Calculated Rank[" << I->getName() << "] = " << i << "\n"); } - ReversePostOrderTraversal RPOT(&F); + // Traverse basic blocks in ReversePostOrder for (BasicBlock *BB : RPOT) { unsigned BBRank = RankMap[BB] = ++i << 16; @@ -2174,11 +2175,19 @@ } PreservedAnalyses ReassociatePass::run(Function &F, FunctionAnalysisManager &) { + // Get the functions basic blocks in Reverse Post Order. This order is used by + // BuildRankMap to pre calculate ranks correctly. It also excludes dead basic + // blocks (it has been seen that the analysis in this pass could hang when + // analysing dead basic blocks). + ReversePostOrderTraversal RPOT(&F); + // Calculate the rank map for F. - BuildRankMap(F); + BuildRankMap(F, RPOT); MadeChange = false; - for (Function::iterator BI = F.begin(), BE = F.end(); BI != BE; ++BI) { + // Traverse the same blocks that was analysed by BuildRankMap. + for (BasicBlock *BI : RPOT) { + assert(RankMap.count(&*BI) && "BB should be ranked."); // Optimize every instruction in the basic block. for (BasicBlock::iterator II = BI->begin(), IE = BI->end(); II != IE;) if (isInstructionTriviallyDead(&*II)) { Index: llvm/trunk/test/Transforms/Reassociate/deadcode.ll =================================================================== --- llvm/trunk/test/Transforms/Reassociate/deadcode.ll +++ llvm/trunk/test/Transforms/Reassociate/deadcode.ll @@ -0,0 +1,37 @@ +; RUN: opt < %s -reassociate -disable-output + +; It has been detected that dead loops like the one in this test case can be +; created by -jump-threading (it was detected by a csmith generated program). +; +; According to -verify this is valid input (even if it could be discussed if +; the dead loop really satisfies SSA form). +; +; The problem found was that the -reassociate pass ends up in an infinite loop +; when analysing the 'deadloop1' basic block. See "Bugzilla - Bug 30818". +define void @deadloop1() { + br label %endlabel + +deadloop1: + %1 = xor i32 %2, 7 + %2 = xor i32 %1, 8 + br label %deadloop1 + +endlabel: + ret void +} + + +; Another example showing that dead code could result in infinite loops in +; reassociate pass. See "Bugzilla - Bug 30818". +define void @deadloop2() { + br label %endlabel + +deadloop2: + %1 = and i32 %2, 7 + %2 = and i32 %3, 8 + %3 = and i32 %1, 6 + br label %deadloop2 + +endlabel: + ret void +}