r215650 - Use the proper post-order traversal in LiveVariables analysis,

Artyom Skrobov Artyom.Skrobov at arm.com
Thu Aug 14 09:04:47 PDT 2014


Author: askrobov
Date: Thu Aug 14 11:04:47 2014
New Revision: 215650

URL: http://llvm.org/viewvc/llvm-project?rev=215650&view=rev
Log:
Use the proper post-order traversal in LiveVariables analysis,
to recover the performance after r214064.

Also sorts out the naming for PostOrderCFGView, ReversePostOrderCFGView,
BackwardDataflowWorklist and ForwardDataflowWorklist, to match the accepted
terminology.

Also unifies BackwardDataflowWorklist and ForwardDataflowWorklist to share
the "worklist for prioritization, post-order traversal for fallback" logic,
and to avoid repetitive sorting.

Also cleans up comments in the affected area.



Modified:
    cfe/trunk/include/clang/Analysis/Analyses/DataflowWorklist.h
    cfe/trunk/include/clang/Analysis/Analyses/PostOrderCFGView.h
    cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
    cfe/trunk/lib/Analysis/Consumed.cpp
    cfe/trunk/lib/Analysis/DataflowWorklist.cpp
    cfe/trunk/lib/Analysis/LiveVariables.cpp
    cfe/trunk/lib/Analysis/PostOrderCFGView.cpp
    cfe/trunk/lib/Analysis/UninitializedValues.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/DataflowWorklist.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/DataflowWorklist.h?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/DataflowWorklist.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/DataflowWorklist.h Thu Aug 14 11:04:47 2014
@@ -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,38 +21,39 @@
 
 namespace clang {
 
-class DataflowWorklistBase {
-protected:
+class DataflowWorklist {
   PostOrderCFGView::iterator PO_I, PO_E;
-  PostOrderCFGView::BlockOrderCompare comparator;
   SmallVector<const CFGBlock *, 20> 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<PostOrderCFGView>()) {}
-
   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<PostOrderCFGView>()) {}
+};
+
+class ForwardDataflowWorklist : public DataflowWorklist {
+public:
+  ForwardDataflowWorklist(const CFG &cfg, AnalysisDeclContext &Ctx)
+    : DataflowWorklist(cfg, *Ctx.getAnalysis<ReversePostOrderCFGView>()) {}
 };
 
 } // end clang namespace

Modified: cfe/trunk/include/clang/Analysis/Analyses/PostOrderCFGView.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PostOrderCFGView.h?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PostOrderCFGView.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/PostOrderCFGView.h Thu Aug 14 11:04:47 2014
@@ -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 @@ public:
     }
   };
 
-private:
-  typedef llvm::po_iterator<const CFG*, CFGBlockSet, true>  po_iterator;
+protected:
   std::vector<const CFGBlock*> Blocks;
 
   typedef llvm::DenseMap<const CFGBlock *, unsigned> BlockOrderTy;
@@ -107,6 +106,15 @@ 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

Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h Thu Aug 14 11:04:47 2014
@@ -142,7 +142,7 @@ public:
     if (!dyn_cast_or_null<NamedDecl>(AC.getDecl()))
       return false;
 
-    SortedGraph = AC.getAnalysis<PostOrderCFGView>();
+    SortedGraph = AC.getAnalysis<ReversePostOrderCFGView>();
     if (!SortedGraph)
       return false;
 

Modified: cfe/trunk/lib/Analysis/Consumed.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Consumed.cpp?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/Consumed.cpp (original)
+++ cfe/trunk/lib/Analysis/Consumed.cpp Thu Aug 14 11:04:47 2014
@@ -1360,7 +1360,7 @@ void ConsumedAnalyzer::run(AnalysisDeclC
 
   determineExpectedReturnState(AC, D);
 
-  PostOrderCFGView *SortedGraph = AC.getAnalysis<PostOrderCFGView>();
+  PostOrderCFGView *SortedGraph = AC.getAnalysis<ReversePostOrderCFGView>();
   // AC.getCFG()->viewCFG(LangOptions());
   
   BlockInfo = ConsumedBlockInfo(CFGraph->getNumBlockIDs(), SortedGraph);

Modified: cfe/trunk/lib/Analysis/DataflowWorklist.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/DataflowWorklist.cpp?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/DataflowWorklist.cpp (original)
+++ cfe/trunk/lib/Analysis/DataflowWorklist.cpp Thu Aug 14 11:04:47 2014
@@ -19,7 +19,7 @@ using namespace clang;
 // 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 @@ void DataflowWorklist::enqueueBlock(cons
   }
 }
 
-// The forward analysis alternates between essentially two worklists.
+// The analysis alternates between essentially two worklists.
 // A prioritization worklist (SmallVector<const CFGBlock *> 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 @@ void DataflowWorklist::enqueueSuccessors
   }
 }
 
-// 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 @@ const CFGBlock *DataflowWorklist::dequeu
   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 @@ const CFGBlock *DataflowWorklist::dequeu
   return B;
 }
 
-void DataflowWorklist::sortWorklist() {
-  std::sort(worklist.begin(), worklist.end(), comparator);
-}
-

Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
+++ cfe/trunk/lib/Analysis/LiveVariables.cpp Thu Aug 14 11:04:47 2014
@@ -448,21 +448,18 @@ LiveVariables::computeLiveness(AnalysisD
 
   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<CFGStmt> cs = bi->getAs<CFGStmt>()) {
@@ -477,11 +474,9 @@ LiveVariables::computeLiveness(AnalysisD
           }
         }
       }
-  }
-  
-  worklist.sortWorklist();
+      scannedForAssignments[block->getBlockID()] = true;
+    }
   
-  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];

Modified: cfe/trunk/lib/Analysis/PostOrderCFGView.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PostOrderCFGView.cpp?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/PostOrderCFGView.cpp (original)
+++ cfe/trunk/lib/Analysis/PostOrderCFGView.cpp Thu Aug 14 11:04:47 2014
@@ -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 @@ using namespace clang;
 
 void PostOrderCFGView::anchor() { }
 
-PostOrderCFGView::PostOrderCFGView(const CFG *cfg) {
+ReversePostOrderCFGView::ReversePostOrderCFGView(const CFG *cfg) {
   Blocks.reserve(cfg->getNumBlockIDs());
   CFGBlockSet BSet(cfg);
-    
+
+  typedef llvm::po_iterator<const CFG*, CFGBlockSet, true> 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 @@ bool PostOrderCFGView::BlockOrderCompare
   return b1V > b2V;
 }
 
+PostOrderCFGView::PostOrderCFGView(const CFG *cfg) {
+  unsigned size = cfg->getNumBlockIDs();
+  Blocks.reserve(size);
+  CFGBlockSet BSet(cfg);
+
+  typedef llvm::po_iterator<const CFG*, CFGBlockSet, true,
+                            llvm::GraphTraits<llvm::Inverse<const CFG*> >
+                           > 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);
+}

Modified: cfe/trunk/lib/Analysis/UninitializedValues.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/UninitializedValues.cpp?rev=215650&r1=215649&r2=215650&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/UninitializedValues.cpp (original)
+++ cfe/trunk/lib/Analysis/UninitializedValues.cpp Thu Aug 14 11:04:47 2014
@@ -771,7 +771,7 @@ void clang::runUninitializedVariablesAna
   }
 
   // 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);





More information about the cfe-commits mailing list