From: Artyom Skrobov Date: Tue, 23 Sep 2014 08:34:41 +0000 (+0000) Subject: Reverting r214064 and r215650 while investigating a pesky performance regression X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ef0ecd47b3152c3a77a6031edd5a7c198897b95b;p=clang Reverting r214064 and r215650 while investigating a pesky performance regression git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@218296 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/Analyses/DataflowWorklist.h b/include/clang/Analysis/Analyses/DataflowWorklist.h deleted file mode 100644 index c10bb5183e..0000000000 --- a/include/clang/Analysis/Analyses/DataflowWorklist.h +++ /dev/null @@ -1,61 +0,0 @@ -//===- DataflowWorklist.h - worklist for dataflow analysis --------*- C++ --*-// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// -// -// 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 backward (used in LiveVariables) analyses. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_DATAFLOWWORKLIST_H -#define LLVM_CLANG_ANALYSIS_ANALYSES_DATAFLOWWORKLIST_H - -#include "clang/Analysis/Analyses/PostOrderCFGView.h" - -namespace clang { - -class DataflowWorklist { - PostOrderCFGView::iterator PO_I, PO_E; - SmallVector worklist; - llvm::BitVector enqueuedBlocks; - -protected: - DataflowWorklist(const CFG &cfg, PostOrderCFGView &view) - : PO_I(view.begin()), PO_E(view.end()), - enqueuedBlocks(cfg.getNumBlockIDs(), true) { - // 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; - } - } - -public: - void enqueueBlock(const CFGBlock *block); - void enqueuePredecessors(const CFGBlock *block); - void enqueueSuccessors(const CFGBlock *block); - const CFGBlock *dequeue(); -}; - -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 diff --git a/include/clang/Analysis/Analyses/PostOrderCFGView.h b/include/clang/Analysis/Analyses/PostOrderCFGView.h index 7e895cde8a..68e42f225e 100644 --- a/include/clang/Analysis/Analyses/PostOrderCFGView.h +++ b/include/clang/Analysis/Analyses/PostOrderCFGView.h @@ -7,7 +7,7 @@ // //===----------------------------------------------------------------------===// // -// This file implements post order views of the blocks in a CFG. +// This file implements post order view of the blocks in a CFG. // //===----------------------------------------------------------------------===// @@ -68,7 +68,8 @@ public: } }; -protected: +private: + typedef llvm::po_iterator po_iterator; std::vector Blocks; typedef llvm::DenseMap BlockOrderTy; @@ -106,15 +107,6 @@ public: 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 diff --git a/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/include/clang/Analysis/Analyses/ThreadSafetyCommon.h index 01492685c7..76d83892dd 100644 --- a/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +++ b/include/clang/Analysis/Analyses/ThreadSafetyCommon.h @@ -142,7 +142,7 @@ public: if (!dyn_cast_or_null(AC.getDecl())) return false; - SortedGraph = AC.getAnalysis(); + SortedGraph = AC.getAnalysis(); if (!SortedGraph) return false; diff --git a/lib/Analysis/CMakeLists.txt b/lib/Analysis/CMakeLists.txt index d913f6668e..1df093d850 100644 --- a/lib/Analysis/CMakeLists.txt +++ b/lib/Analysis/CMakeLists.txt @@ -13,7 +13,6 @@ add_clang_library(clangAnalysis Consumed.cpp CodeInjector.cpp Dominators.cpp - DataflowWorklist.cpp FormatString.cpp LiveVariables.cpp ObjCNoReturn.cpp diff --git a/lib/Analysis/Consumed.cpp b/lib/Analysis/Consumed.cpp index 47239d098c..2b2da2c69a 100644 --- a/lib/Analysis/Consumed.cpp +++ b/lib/Analysis/Consumed.cpp @@ -1360,7 +1360,7 @@ void ConsumedAnalyzer::run(AnalysisDeclContext &AC) { determineExpectedReturnState(AC, D); - PostOrderCFGView *SortedGraph = AC.getAnalysis(); + PostOrderCFGView *SortedGraph = AC.getAnalysis(); // AC.getCFG()->viewCFG(LangOptions()); BlockInfo = ConsumedBlockInfo(CFGraph->getNumBlockIDs(), SortedGraph); diff --git a/lib/Analysis/DataflowWorklist.cpp b/lib/Analysis/DataflowWorklist.cpp deleted file mode 100644 index 9cc2a934e6..0000000000 --- a/lib/Analysis/DataflowWorklist.cpp +++ /dev/null @@ -1,79 +0,0 @@ -//===- DataflowWorklist.cpp - worklist for dataflow analysis ------*- C++ --*-// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// -// -// DataflowWorklist is used in LiveVariables and UninitializedValues analyses -// -//===----------------------------------------------------------------------===// - -#include "clang/Analysis/Analyses/DataflowWorklist.h" - -using namespace clang; - -// Marking a block as enqueued means that it cannot be re-added to the worklist, -// 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 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()]) { - enqueuedBlocks[block->getBlockID()] = true; - worklist.push_back(block); - } -} - -// The analysis alternates between essentially two worklists. -// A prioritization worklist (SmallVector worklist) -// 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 -// we want to prioritize analyzing first, because that causes dataflow facts -// to flow up the graph, which we then want to propagate forward. -// In practice this can cause the analysis to converge much faster. -void DataflowWorklist::enqueueSuccessors(const clang::CFGBlock *block) { - for (CFGBlock::const_succ_iterator I = block->succ_begin(), - E = block->succ_end(); I != E; ++I) { - enqueueBlock(*I); - } -} - -void DataflowWorklist::enqueuePredecessors(const clang::CFGBlock *block) { - for (CFGBlock::const_pred_iterator I = block->pred_begin(), - E = block->pred_end(); I != E; ++I) { - enqueueBlock(*I); - } -} - -const CFGBlock *DataflowWorklist::dequeue() { - const CFGBlock *B = nullptr; - - // First dequeue from the worklist. This can represent - // updates along backedges that we want propagated as quickly as possible. - if (!worklist.empty()) - B = worklist.pop_back_val(); - - // 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; - } - else { - return nullptr; - } - - assert(enqueuedBlocks[B->getBlockID()] == true); - enqueuedBlocks[B->getBlockID()] = false; - return B; -} - diff --git a/lib/Analysis/LiveVariables.cpp b/lib/Analysis/LiveVariables.cpp index 77f7dc386e..3d6fc039fd 100644 --- a/lib/Analysis/LiveVariables.cpp +++ b/lib/Analysis/LiveVariables.cpp @@ -14,16 +14,70 @@ #include "clang/Analysis/Analyses/LiveVariables.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" -#include "clang/Analysis/Analyses/DataflowWorklist.h" +#include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "clang/Analysis/AnalysisContext.h" #include "clang/Analysis/CFG.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/PostOrderIterator.h" #include "llvm/Support/raw_ostream.h" #include #include using namespace clang; +namespace { + +class DataflowWorklist { + SmallVector worklist; + llvm::BitVector enqueuedBlocks; + PostOrderCFGView *POV; +public: + DataflowWorklist(const CFG &cfg, AnalysisDeclContext &Ctx) + : enqueuedBlocks(cfg.getNumBlockIDs()), + POV(Ctx.getAnalysis()) {} + + void enqueueBlock(const CFGBlock *block); + void enqueuePredecessors(const CFGBlock *block); + + const CFGBlock *dequeue(); + + void sortWorklist(); +}; + +} + +void DataflowWorklist::enqueueBlock(const clang::CFGBlock *block) { + if (block && !enqueuedBlocks[block->getBlockID()]) { + enqueuedBlocks[block->getBlockID()] = true; + worklist.push_back(block); + } +} + +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(); +} + +void DataflowWorklist::sortWorklist() { + std::sort(worklist.begin(), worklist.end(), POV->getComparator()); +} + +const CFGBlock *DataflowWorklist::dequeue() { + if (worklist.empty()) + return nullptr; + const CFGBlock *b = worklist.pop_back_val(); + enqueuedBlocks[b->getBlockID()] = false; + return b; +} + namespace { class LiveVariablesImpl { public: @@ -448,18 +502,21 @@ LiveVariables::computeLiveness(AnalysisDeclContext &AC, LiveVariablesImpl *LV = new LiveVariablesImpl(AC, killAtAssign); - // Construct the backward dataflow worklist. - BackwardDataflowWorklist worklist(*cfg, AC); + // Construct the dataflow worklist. Enqueue the exit block as the + // start of the analysis. + DataflowWorklist worklist(*cfg, AC); llvm::BitVector everAnalyzedBlock(cfg->getNumBlockIDs()); - llvm::BitVector scannedForAssignments(cfg->getNumBlockIDs()); - while (const CFGBlock *block = worklist.dequeue()) { + // 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); // 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 && !scannedForAssignments[block->getBlockID()]) { + if (killAtAssign) for (CFGBlock::const_iterator bi = block->begin(), be = block->end(); bi != be; ++bi) { if (Optional cs = bi->getAs()) { @@ -474,9 +531,11 @@ LiveVariables::computeLiveness(AnalysisDeclContext &AC, } } } - 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]; diff --git a/lib/Analysis/PostOrderCFGView.cpp b/lib/Analysis/PostOrderCFGView.cpp index 714292b3f9..5a3c8182a1 100644 --- a/lib/Analysis/PostOrderCFGView.cpp +++ b/lib/Analysis/PostOrderCFGView.cpp @@ -7,7 +7,7 @@ // //===----------------------------------------------------------------------===// // -// This file implements post order views of the blocks in a CFG. +// This file implements post order view of the blocks in a CFG. // //===----------------------------------------------------------------------===// @@ -17,16 +17,14 @@ using namespace clang; void PostOrderCFGView::anchor() { } -ReversePostOrderCFGView::ReversePostOrderCFGView(const CFG *cfg) { +PostOrderCFGView::PostOrderCFGView(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(); } } @@ -49,49 +47,3 @@ bool PostOrderCFGView::BlockOrderCompare::operator()(const CFGBlock *b1, 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); -} diff --git a/lib/Analysis/UninitializedValues.cpp b/lib/Analysis/UninitializedValues.cpp index ef2cf36f3c..94f59f196e 100644 --- a/lib/Analysis/UninitializedValues.cpp +++ b/lib/Analysis/UninitializedValues.cpp @@ -15,7 +15,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/StmtVisitor.h" -#include "clang/Analysis/Analyses/DataflowWorklist.h" +#include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "clang/Analysis/Analyses/UninitializedValues.h" #include "clang/Analysis/AnalysisContext.h" #include "clang/Analysis/CFG.h" @@ -198,6 +198,66 @@ ValueVector::reference CFGBlockValues::operator[](const VarDecl *vd) { return scratch[idx.getValue()]; } +//------------------------------------------------------------------------====// +// Worklist: worklist for dataflow analysis. +//====------------------------------------------------------------------------// + +namespace { +class DataflowWorklist { + PostOrderCFGView::iterator PO_I, PO_E; + SmallVector worklist; + llvm::BitVector enqueuedBlocks; +public: + DataflowWorklist(const CFG &cfg, PostOrderCFGView &view) + : PO_I(view.begin()), PO_E(view.end()), + enqueuedBlocks(cfg.getNumBlockIDs(), true) { + // Treat the first block as already analyzed. + if (PO_I != PO_E) { + assert(*PO_I == &cfg.getEntry()); + enqueuedBlocks[(*PO_I)->getBlockID()] = false; + ++PO_I; + } + } + + void enqueueSuccessors(const CFGBlock *block); + const CFGBlock *dequeue(); +}; +} + +void DataflowWorklist::enqueueSuccessors(const clang::CFGBlock *block) { + for (CFGBlock::const_succ_iterator I = block->succ_begin(), + E = block->succ_end(); I != E; ++I) { + const CFGBlock *Successor = *I; + if (!Successor || enqueuedBlocks[Successor->getBlockID()]) + continue; + worklist.push_back(Successor); + enqueuedBlocks[Successor->getBlockID()] = true; + } +} + +const CFGBlock *DataflowWorklist::dequeue() { + const CFGBlock *B = nullptr; + + // First dequeue from the worklist. This can represent + // updates along backedges that we want propagated as quickly as possible. + 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. + else if (PO_I != PO_E) { + B = *PO_I; + ++PO_I; + } + else { + return nullptr; + } + + assert(enqueuedBlocks[B->getBlockID()] == true); + enqueuedBlocks[B->getBlockID()] = false; + return B; +} + //------------------------------------------------------------------------====// // Classification of DeclRefExprs as use or initialization. //====------------------------------------------------------------------------// @@ -796,7 +856,7 @@ void clang::runUninitializedVariablesAnalysis( } // Proceed with the workist. - ForwardDataflowWorklist worklist(cfg, ac); + DataflowWorklist worklist(cfg, *ac.getAnalysis()); llvm::BitVector previouslyVisited(cfg.getNumBlockIDs()); worklist.enqueueSuccessors(&cfg.getEntry()); llvm::BitVector wasAnalyzed(cfg.getNumBlockIDs(), false);