From f62c44a38dae790a4ba9491e1199a8e443fed63b Mon Sep 17 00:00:00 2001 From: Owen Anderson Date: Mon, 25 Jun 2007 05:41:12 +0000 Subject: [PATCH] 1) Fix an issue with non-deterministic iteration order in phi_translate 2) Remove some maximal-set computing code that is no longer used. 3) Use a post-order CFG traversal to compute ANTIC_IN instead of a postdom traversal. This causes the ANTIC_IN calculation to converge much faster. Thanks to Daniel Berlin for suggesting this. With this patch, the time to optimize 403.gcc decreased from 17.5s to 7.5s, and Anton's huge testcase decreased from 62 minutes to 38 seconds. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@37714 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/GVNPRE.cpp | 197 ++++++++++++++----------------- 1 file changed, 86 insertions(+), 111 deletions(-) diff --git a/lib/Transforms/Scalar/GVNPRE.cpp b/lib/Transforms/Scalar/GVNPRE.cpp index 21e118a5c5d..e1f4af072f3 100644 --- a/lib/Transforms/Scalar/GVNPRE.cpp +++ b/lib/Transforms/Scalar/GVNPRE.cpp @@ -24,7 +24,6 @@ #include "llvm/Instructions.h" #include "llvm/Function.h" #include "llvm/Analysis/Dominators.h" -#include "llvm/Analysis/PostDominators.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DepthFirstIterator.h" @@ -86,9 +85,6 @@ namespace { DenseMap valueNumbering; std::map expressionNumbering; - std::set maximalExpressions; - SmallPtrSet maximalValues; - uint32_t nextValueNumber; Expression::ExpressionOpcode getOpcode(BinaryOperator* BO); @@ -101,11 +97,6 @@ namespace { uint32_t lookup(Value* V); void add(Value* V, uint32_t num); void clear(); - std::set& getMaximalExpressions() { - return maximalExpressions; - - } - SmallPtrSet& getMaximalValues() { return maximalValues; } void erase(Value* v); unsigned size(); }; @@ -230,8 +221,6 @@ ValueTable::Expression ValueTable::create_expression(BinaryOperator* BO) { e.rightVN = lookup_or_add(BO->getOperand(1)); e.opcode = getOpcode(BO); - maximalExpressions.insert(e); - return e; } @@ -242,8 +231,6 @@ ValueTable::Expression ValueTable::create_expression(CmpInst* C) { e.rightVN = lookup_or_add(C->getOperand(1)); e.opcode = getOpcode(C); - maximalExpressions.insert(e); - return e; } @@ -254,8 +241,6 @@ ValueTable::Expression ValueTable::create_expression(CmpInst* C) { /// lookup_or_add - Returns the value number for the specified value, assigning /// it a new number if it did not have one before. uint32_t ValueTable::lookup_or_add(Value* V) { - maximalValues.insert(V); - DenseMap::iterator VI = valueNumbering.find(V); if (VI != valueNumbering.end()) return VI->second; @@ -314,23 +299,16 @@ void ValueTable::add(Value* V, uint32_t num) { valueNumbering.insert(std::make_pair(V, num)); } -/// clear - Remove all entries from the ValueTable and the maximal sets +/// clear - Remove all entries from the ValueTable void ValueTable::clear() { valueNumbering.clear(); expressionNumbering.clear(); - maximalExpressions.clear(); - maximalValues.clear(); nextValueNumber = 1; } -/// erase - Remove a value from the value numbering and maximal sets +/// erase - Remove a value from the value numbering void ValueTable::erase(Value* V) { - maximalValues.erase(V); valueNumbering.erase(V); - if (BinaryOperator* BO = dyn_cast(V)) - maximalExpressions.erase(create_expression(BO)); - else if (CmpInst* C = dyn_cast(V)) - maximalExpressions.erase(create_expression(C)); } /// size - Return the number of assigned value numbers @@ -362,7 +340,6 @@ namespace { virtual void getAnalysisUsage(AnalysisUsage &AU) const { AU.setPreservesCFG(); AU.addRequired(); - AU.addRequired(); } // Helper fuctions @@ -467,33 +444,38 @@ Value* GVNPRE::phi_translate(Value* V, BasicBlock* pred, BasicBlock* succ) { if (V == 0) return 0; - if (BinaryOperator* BO = dyn_cast(V)) { + if (isa(V) || isa(V)) { + User* U = cast(V); + Value* newOp1 = 0; - if (isa(BO->getOperand(0))) - newOp1 = phi_translate(find_leader(anticipatedIn[succ], - VN.lookup(BO->getOperand(0))), - pred, succ); + if (isa(U->getOperand(0))) + newOp1 = phi_translate(U->getOperand(0), pred, succ); else - newOp1 = BO->getOperand(0); + newOp1 = U->getOperand(0); if (newOp1 == 0) return 0; Value* newOp2 = 0; - if (isa(BO->getOperand(1))) - newOp2 = phi_translate(find_leader(anticipatedIn[succ], - VN.lookup(BO->getOperand(1))), - pred, succ); + if (isa(U->getOperand(1))) + newOp2 = phi_translate(U->getOperand(1), pred, succ); else - newOp2 = BO->getOperand(1); + newOp2 = U->getOperand(1); if (newOp2 == 0) return 0; - if (newOp1 != BO->getOperand(0) || newOp2 != BO->getOperand(1)) { - Instruction* newVal = BinaryOperator::create(BO->getOpcode(), - newOp1, newOp2, - BO->getName()+".expr"); + if (newOp1 != U->getOperand(0) || newOp2 != U->getOperand(1)) { + Instruction* newVal = 0; + if (BinaryOperator* BO = dyn_cast(U)) + newVal = BinaryOperator::create(BO->getOpcode(), + newOp1, newOp2, + BO->getName()+".expr"); + else if (CmpInst* C = dyn_cast(U)) + newVal = CmpInst::create(C->getOpcode(), + C->getPredicate(), + newOp1, newOp2, + C->getName()+".expr"); uint32_t v = VN.lookup_or_add(newVal); @@ -510,47 +492,6 @@ Value* GVNPRE::phi_translate(Value* V, BasicBlock* pred, BasicBlock* succ) { } else if (PHINode* P = dyn_cast(V)) { if (P->getParent() == succ) return P->getIncomingValueForBlock(pred); - } else if (CmpInst* C = dyn_cast(V)) { - Value* newOp1 = 0; - if (isa(C->getOperand(0))) - newOp1 = phi_translate(find_leader(anticipatedIn[succ], - VN.lookup(C->getOperand(0))), - pred, succ); - else - newOp1 = C->getOperand(0); - - if (newOp1 == 0) - return 0; - - Value* newOp2 = 0; - if (isa(C->getOperand(1))) - newOp2 = phi_translate(find_leader(anticipatedIn[succ], - VN.lookup(C->getOperand(1))), - pred, succ); - else - newOp2 = C->getOperand(1); - - if (newOp2 == 0) - return 0; - - if (newOp1 != C->getOperand(0) || newOp2 != C->getOperand(1)) { - Instruction* newVal = CmpInst::create(C->getOpcode(), - C->getPredicate(), - newOp1, newOp2, - C->getName()+".expr"); - - uint32_t v = VN.lookup_or_add(newVal); - - Value* leader = find_leader(availableOut[pred], v); - if (leader == 0) { - createdExpressions.push_back(newVal); - return newVal; - } else { - VN.erase(newVal); - delete newVal; - return leader; - } - } } return V; @@ -728,9 +669,9 @@ bool GVNPRE::elimination() { E = df_end(DT.getRootNode()); DI != E; ++DI) { BasicBlock* BB = DI->getBlock(); - DOUT << "Block: " << BB->getName() << "\n"; - dump(availableOut[BB]); - DOUT << "\n\n"; + //DOUT << "Block: " << BB->getName() << "\n"; + //dump(availableOut[BB]); + //DOUT << "\n\n"; for (BasicBlock::iterator BI = BB->begin(), BE = BB->end(); BI != BE; ++BI) { @@ -866,11 +807,15 @@ bool GVNPRE::buildsets_anticout(BasicBlock* BB, SmallPtrSet& anticOut, std::set& visited) { if (BB->getTerminator()->getNumSuccessors() == 1) { - if (visited.count(BB->getTerminator()->getSuccessor(0)) == 0) + if (BB->getTerminator()->getSuccessor(0) != BB && + visited.count(BB->getTerminator()->getSuccessor(0)) == 0) { + DOUT << "DEFER: " << BB->getName() << "\n"; return true; - else + } + else { phi_translate_set(anticipatedIn[BB->getTerminator()->getSuccessor(0)], BB, BB->getTerminator()->getSuccessor(0), anticOut); + } } else if (BB->getTerminator()->getNumSuccessors() > 1) { BasicBlock* first = BB->getTerminator()->getSuccessor(0); anticOut.insert(anticipatedIn[first].begin(), anticipatedIn[first].end()); @@ -910,13 +855,17 @@ unsigned GVNPRE::buildsets_anticin(BasicBlock* BB, if (defer) return 0; + + anticIn.clear(); BitVector numbers(VN.size()); for (SmallPtrSet::iterator I = anticOut.begin(), E = anticOut.end(); I != E; ++I) { anticIn.insert(*I); - numbers.set(VN.lookup_or_add(*I)); + unsigned num = VN.lookup_or_add(*I); + numbers.resize(VN.size()); + numbers.set(num); } for (SmallPtrSet::iterator I = currExps.begin(), E = currExps.end(); I != E; ++I) { @@ -931,11 +880,15 @@ unsigned GVNPRE::buildsets_anticin(BasicBlock* BB, anticIn.erase(*I); clean(anticIn); - anticOut.clear(); - if (old != anticIn.size()) + if (old != anticIn.size()) { + DOUT << "OLD: " << old << "\n"; + DOUT << "NEW: " << anticIn.size() << "\n"; + DOUT << "ANTIC_OUT: " << anticOut.size() << "\n"; + anticOut.clear(); return 2; - else + } else + anticOut.clear(); return 1; } @@ -979,21 +932,41 @@ unsigned GVNPRE::buildsets(Function& F) { currTemps, availNumbers, expNumbers); } + + // Phase 1, Part 2: calculate ANTIC_IN - // If function has no exit blocks, only perform GVN - PostDominatorTree &PDT = getAnalysis(); - if (PDT[&F.getEntryBlock()] == 0) { - bool changed_function = elimination(); - cleanup(); + DOUT << "Calculating walk\n"; + // Calculate a postorder CFG walk + std::vector walk; + std::vector walkStack; + SmallPtrSet walkVisited; + walkStack.push_back(&F.getEntryBlock()); + walkVisited.insert(&F.getEntryBlock()); + + while (!walkStack.empty()) { + BasicBlock* BB = walkStack.back(); + walkVisited.insert(BB); - if (changed_function) - return 2; // Bailed early, made changes - else - return 1; // Bailed early, no changes + bool inserted = false; + for (unsigned i = 0; i < BB->getTerminator()->getNumSuccessors(); ++i) { + BasicBlock* succ = BB->getTerminator()->getSuccessor(i); + if (walkVisited.count(succ) == 0) { + walkStack.push_back(succ); + inserted = true; + } + } + + if (inserted) + continue; + else { + walk.push_back(BB); + walkStack.pop_back(); + } } + DOUT << "Finished calculating walk\n"; - // Phase 1, Part 2: calculate ANTIC_IN + // Perform the ANTIC_IN calculation std::set visited; @@ -1004,23 +977,23 @@ unsigned GVNPRE::buildsets(Function& F) { SmallPtrSet anticOut; // Top-down walk of the postdominator tree - for (df_iterator PDI = - df_begin(PDT.getRootNode()), E = df_end(PDT.getRootNode()); - PDI != E; ++PDI) { - BasicBlock* BB = PDI->getBlock(); + for (std::vector::iterator BBI = walk.begin(), BBE = walk.end(); + BBI != BBE; ++BBI) { + BasicBlock* BB = *BBI; if (BB == 0) continue; - - unsigned ret = buildsets_anticin(BB, anticOut,generatedExpressions[BB], generatedTemporaries[BB], visited); if (ret == 0) { changed = true; - break; + continue; } else { visited.insert(BB); + if (ret == 2) { + DOUT << "CHANGED: " << BB->getName() << "\n"; + } changed |= (ret == 2); } } @@ -1028,6 +1001,8 @@ unsigned GVNPRE::buildsets(Function& F) { iterations++; } + DOUT << "ITERATIONS: " << iterations << "\n"; + return 0; // No bail, no changes } @@ -1197,10 +1172,10 @@ bool GVNPRE::insertion(Function& F) { workList.reserve(anticIn.size()); topo_sort(anticIn, workList); - DOUT << "Merge Block: " << BB->getName() << "\n"; - DOUT << "ANTIC_IN: "; - dump(anticIn); - DOUT << "\n"; + //DOUT << "Merge Block: " << BB->getName() << "\n"; + //DOUT << "ANTIC_IN: "; + //dump(anticIn); + //DOUT << "\n"; unsigned result = insertion_mergepoint(workList, DI, new_set); if (result & 1)