Index: include/clang/Analysis/Analyses/PostOrderCFGView.h =================================================================== --- include/clang/Analysis/Analyses/PostOrderCFGView.h (revision 215621) +++ include/clang/Analysis/Analyses/PostOrderCFGView.h (working copy) @@ -7,7 +7,7 @@ // //===----------------------------------------------------------------------===// // -// This file implements post order view of the blocks in a CFG. +// This file implements post order views of the blocks in a CFG. // //===----------------------------------------------------------------------===// @@ -68,8 +68,7 @@ } }; -private: - typedef llvm::po_iterator po_iterator; +protected: std::vector Blocks; typedef llvm::DenseMap BlockOrderTy; @@ -107,8 +106,17 @@ static const void *getTag(); static PostOrderCFGView *create(AnalysisDeclContext &analysisContext); + +protected: + PostOrderCFGView() {} }; +class ReversePostOrderCFGView : public PostOrderCFGView { +public: + ReversePostOrderCFGView(const CFG *cfg); + static ReversePostOrderCFGView *create(AnalysisDeclContext &analysisContext); +}; + } // end clang namespace #endif Index: include/clang/Analysis/Analyses/ThreadSafetyCommon.h =================================================================== --- include/clang/Analysis/Analyses/ThreadSafetyCommon.h (revision 215621) +++ include/clang/Analysis/Analyses/ThreadSafetyCommon.h (working copy) @@ -142,7 +142,7 @@ if (!dyn_cast_or_null(AC.getDecl())) return false; - SortedGraph = AC.getAnalysis(); + SortedGraph = AC.getAnalysis(); if (!SortedGraph) return false; Index: include/clang/Analysis/Analyses/DataflowWorklist.h =================================================================== --- include/clang/Analysis/Analyses/DataflowWorklist.h (revision 215621) +++ include/clang/Analysis/Analyses/DataflowWorklist.h (working copy) @@ -10,7 +10,7 @@ // DataflowWorklist keeps track of blocks for dataflow analysis. It maintains a // vector of blocks for priority processing, and falls back upon a reverse // post-order iterator. It supports both forward (used in UninitializedValues) -// and reverse (used in LiveVariables) analyses. +// and backward (used in LiveVariables) analyses. // //===----------------------------------------------------------------------===// @@ -21,40 +21,41 @@ namespace clang { -class DataflowWorklistBase { -protected: +class DataflowWorklist { PostOrderCFGView::iterator PO_I, PO_E; - PostOrderCFGView::BlockOrderCompare comparator; SmallVector worklist; llvm::BitVector enqueuedBlocks; - DataflowWorklistBase(const CFG &cfg, PostOrderCFGView &view) +protected: + DataflowWorklist(const CFG &cfg, PostOrderCFGView &view) : PO_I(view.begin()), PO_E(view.end()), - comparator(view.getComparator()), enqueuedBlocks(cfg.getNumBlockIDs(), true) { - // Treat the first block as already analyzed. - if (PO_I != PO_E) { - assert(*PO_I == &cfg.getEntry()); + // For forward analysis, treat the first block as already analyzed. + if ((PO_I != PO_E) && (*PO_I == &cfg.getEntry())) { enqueuedBlocks[(*PO_I)->getBlockID()] = false; ++PO_I; } } -}; -class DataflowWorklist : DataflowWorklistBase { - public: - DataflowWorklist(const CFG &cfg, AnalysisDeclContext &Ctx) - : DataflowWorklistBase(cfg, *Ctx.getAnalysis()) {} - void enqueueBlock(const CFGBlock *block); void enqueuePredecessors(const CFGBlock *block); void enqueueSuccessors(const CFGBlock *block); const CFGBlock *dequeue(); +}; - void sortWorklist(); +class BackwardDataflowWorklist : public DataflowWorklist { +public: + BackwardDataflowWorklist(const CFG &cfg, AnalysisDeclContext &Ctx) + : DataflowWorklist(cfg, *Ctx.getAnalysis()) {} }; +class ForwardDataflowWorklist : public DataflowWorklist { +public: + ForwardDataflowWorklist(const CFG &cfg, AnalysisDeclContext &Ctx) + : DataflowWorklist(cfg, *Ctx.getAnalysis()) {} +}; + } // end clang namespace #endif Index: lib/Analysis/PostOrderCFGView.cpp =================================================================== --- lib/Analysis/PostOrderCFGView.cpp (revision 215621) +++ lib/Analysis/PostOrderCFGView.cpp (working copy) @@ -7,7 +7,7 @@ // //===----------------------------------------------------------------------===// // -// This file implements post order view of the blocks in a CFG. +// This file implements post order views of the blocks in a CFG. // //===----------------------------------------------------------------------===// @@ -17,14 +17,16 @@ void PostOrderCFGView::anchor() { } -PostOrderCFGView::PostOrderCFGView(const CFG *cfg) { +ReversePostOrderCFGView::ReversePostOrderCFGView(const CFG *cfg) { Blocks.reserve(cfg->getNumBlockIDs()); CFGBlockSet BSet(cfg); - + + typedef llvm::po_iterator po_iterator; + for (po_iterator I = po_iterator::begin(cfg, BSet), E = po_iterator::end(cfg, BSet); I != E; ++I) { - BlockOrder[*I] = Blocks.size() + 1; Blocks.push_back(*I); + BlockOrder[*I] = Blocks.size(); } } @@ -47,3 +49,49 @@ return b1V > b2V; } +PostOrderCFGView::PostOrderCFGView(const CFG *cfg) { + unsigned size = cfg->getNumBlockIDs(); + Blocks.reserve(size); + CFGBlockSet BSet(cfg); + + typedef llvm::po_iterator > + > po_iterator; + + for (po_iterator I = po_iterator::begin(cfg, BSet), + E = po_iterator::end(cfg, BSet); I != E; ++I) { + Blocks.push_back(*I); + BlockOrder[*I] = Blocks.size(); + } + + // It may be that some blocks are inaccessible going from the CFG exit upwards + // (e.g. infinite loops); we still need to add them. + for (CFG::const_iterator I = cfg->begin(), E = cfg->end(); + (Blocks.size() < size) && (I != E); ++I) { + const CFGBlock* block = *I; + // Add a chain going upwards. + while (!BlockOrder.count(block)) { + Blocks.push_back(block); + BlockOrder[block] = Blocks.size(); + CFGBlock::const_pred_iterator PI = block->pred_begin(), + PE = block->pred_end(); + for (; PI != PE; ++PI) { + const CFGBlock* pred = *PI; + if (pred && !BlockOrder.count(pred)) { + block = pred; + break; + } + } + // Chain ends when we couldn't find an unmapped pred. + if (PI == PE) break; + } + } +} + +ReversePostOrderCFGView * +ReversePostOrderCFGView::create(AnalysisDeclContext &ctx) { + const CFG *cfg = ctx.getCFG(); + if (!cfg) + return nullptr; + return new ReversePostOrderCFGView(cfg); +} Index: lib/Analysis/DataflowWorklist.cpp =================================================================== --- lib/Analysis/DataflowWorklist.cpp (revision 215621) +++ lib/Analysis/DataflowWorklist.cpp (working copy) @@ -19,7 +19,7 @@ // but it doesn't control when the algorithm terminates. // Initially, enqueuedBlocks is set to true for all blocks; // that's not because everything is added initially to the worklist, -// but instead, to cause the forward analysis to follow the reverse post order +// but instead, to cause the analysis to follow the initial graph traversal // until we enqueue something on the worklist. void DataflowWorklist::enqueueBlock(const clang::CFGBlock *block) { if (block && !enqueuedBlocks[block->getBlockID()]) { @@ -28,10 +28,11 @@ } } -// The forward analysis alternates between essentially two worklists. +// The analysis alternates between essentially two worklists. // A prioritization worklist (SmallVector worklist) -// is consulted first, and if it's empty, we consult the reverse -// post-order traversal (PostOrderCFGView::iterator PO_I). +// is consulted first, and if it's empty, we consult +// PostOrderCFGView::iterator PO_I, which implements either post-order traversal +// for backward analysis, or reverse post-order traversal for forward analysis. // The prioritization worklist is used to prioritize analyzing from // the beginning, or to prioritize updates fed by back edges. // Typically, what gets enqueued on the worklist are back edges, which @@ -45,22 +46,11 @@ } } -// The reverse analysis uses a simple re-sorting of the worklist to -// reprioritize it. It's not as efficient as the two-worklists approach, -// but it isn't performance sensitive since it's used by the static analyzer, -// and the static analyzer does far more work that dwarfs the work done here. -// TODO: It would still be nice to use the same approach for both analyses. void DataflowWorklist::enqueuePredecessors(const clang::CFGBlock *block) { - const unsigned OldWorklistSize = worklist.size(); for (CFGBlock::const_pred_iterator I = block->pred_begin(), E = block->pred_end(); I != E; ++I) { enqueueBlock(*I); } - - if (OldWorklistSize == 0 || OldWorklistSize == worklist.size()) - return; - - sortWorklist(); } const CFGBlock *DataflowWorklist::dequeue() { @@ -71,8 +61,9 @@ if (!worklist.empty()) B = worklist.pop_back_val(); - // Next dequeue from the initial reverse post order. This is the - // theoretical ideal in the presence of no back edges. + // Next dequeue from the initial graph traversal (either post order or + // reverse post order). This is the theoretical ideal in the presence + // of no back edges. else if (PO_I != PO_E) { B = *PO_I; ++PO_I; @@ -86,7 +77,3 @@ return B; } -void DataflowWorklist::sortWorklist() { - std::sort(worklist.begin(), worklist.end(), comparator); -} - Index: lib/Analysis/Consumed.cpp =================================================================== --- lib/Analysis/Consumed.cpp (revision 215621) +++ lib/Analysis/Consumed.cpp (working copy) @@ -1360,7 +1360,7 @@ determineExpectedReturnState(AC, D); - PostOrderCFGView *SortedGraph = AC.getAnalysis(); + PostOrderCFGView *SortedGraph = AC.getAnalysis(); // AC.getCFG()->viewCFG(LangOptions()); BlockInfo = ConsumedBlockInfo(CFGraph->getNumBlockIDs(), SortedGraph); Index: lib/Analysis/LiveVariables.cpp =================================================================== --- lib/Analysis/LiveVariables.cpp (revision 215621) +++ lib/Analysis/LiveVariables.cpp (working copy) @@ -448,21 +448,18 @@ LiveVariablesImpl *LV = new LiveVariablesImpl(AC, killAtAssign); - // Construct the dataflow worklist. Enqueue the exit block as the - // start of the analysis. - DataflowWorklist worklist(*cfg, AC); + // Construct the backward dataflow worklist. + BackwardDataflowWorklist worklist(*cfg, AC); llvm::BitVector everAnalyzedBlock(cfg->getNumBlockIDs()); + llvm::BitVector scannedForAssignments(cfg->getNumBlockIDs()); - // FIXME: we should enqueue using post order. - for (CFG::const_iterator it = cfg->begin(), ei = cfg->end(); it != ei; ++it) { - const CFGBlock *block = *it; - worklist.enqueueBlock(block); + while (const CFGBlock *block = worklist.dequeue()) { // FIXME: Scan for DeclRefExprs using in the LHS of an assignment. // We need to do this because we lack context in the reverse analysis // to determine if a DeclRefExpr appears in such a context, and thus // doesn't constitute a "use". - if (killAtAssign) + if (killAtAssign && !scannedForAssignments[block->getBlockID()]) { for (CFGBlock::const_iterator bi = block->begin(), be = block->end(); bi != be; ++bi) { if (Optional cs = bi->getAs()) { @@ -477,11 +474,9 @@ } } } - } + scannedForAssignments[block->getBlockID()] = true; + } - worklist.sortWorklist(); - - while (const CFGBlock *block = worklist.dequeue()) { // Determine if the block's end value has changed. If not, we // have nothing left to do for this block. LivenessValues &prevVal = LV->blocksEndToLiveness[block]; Index: lib/Analysis/UninitializedValues.cpp =================================================================== --- lib/Analysis/UninitializedValues.cpp (revision 215621) +++ lib/Analysis/UninitializedValues.cpp (working copy) @@ -771,7 +771,7 @@ } // Proceed with the workist. - DataflowWorklist worklist(cfg, ac); + ForwardDataflowWorklist worklist(cfg, ac); llvm::BitVector previouslyVisited(cfg.getNumBlockIDs()); worklist.enqueueSuccessors(&cfg.getEntry()); llvm::BitVector wasAnalyzed(cfg.getNumBlockIDs(), false);