Reverting r214064 and r215650 while investigating a pesky performance regression

llvm-svn: 218296

clang/include/clang/Analysis/Analyses/DataflowWorklist.h

@@ -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<const CFGBlock *, 20> 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<PostOrderCFGView>()) {}
-};
-
-class ForwardDataflowWorklist : public DataflowWorklist {
-public:
-  ForwardDataflowWorklist(const CFG &cfg, AnalysisDeclContext &Ctx)
-    : DataflowWorklist(cfg, *Ctx.getAnalysis<ReversePostOrderCFGView>()) {}
-};
-
-} // end clang namespace
-
-#endif

clang/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<const CFG*, CFGBlockSet, true>  po_iterator;
   std::vector<const CFGBlock*> Blocks;
 
   typedef llvm::DenseMap<const CFGBlock *, unsigned> 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

clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h

@@ -142,7 +142,7 @@ public:
     if (!dyn_cast_or_null<NamedDecl>(AC.getDecl()))
       return false;
 
-    SortedGraph = AC.getAnalysis<ReversePostOrderCFGView>();
+    SortedGraph = AC.getAnalysis<PostOrderCFGView>();
     if (!SortedGraph)
       return false;
 

clang/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

clang/lib/Analysis/Consumed.cpp

@@ -1360,7 +1360,7 @@ void ConsumedAnalyzer::run(AnalysisDeclContext &AC) {
 
   determineExpectedReturnState(AC, D);
 
-  PostOrderCFGView *SortedGraph = AC.getAnalysis<ReversePostOrderCFGView>();
+  PostOrderCFGView *SortedGraph = AC.getAnalysis<PostOrderCFGView>();
   // AC.getCFG()->viewCFG(LangOptions());
   
   BlockInfo = ConsumedBlockInfo(CFGraph->getNumBlockIDs(), SortedGraph);

clang/lib/Analysis/DataflowWorklist.cpp

@@ -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<const CFGBlock *> 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;
-}
-

clang/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 <algorithm>
 #include <vector>
 
 using namespace clang;
 
+namespace {
+
+class DataflowWorklist {
+  SmallVector<const CFGBlock *, 20> worklist;
+  llvm::BitVector enqueuedBlocks;
+  PostOrderCFGView *POV;
+public:
+  DataflowWorklist(const CFG &cfg, AnalysisDeclContext &Ctx)
+    : enqueuedBlocks(cfg.getNumBlockIDs()),
+      POV(Ctx.getAnalysis<PostOrderCFGView>()) {}
+  
+  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<CFGStmt> cs = bi->getAs<CFGStmt>()) {
@@ -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];

clang/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<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();
   }
 }
 
@@ -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<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);
-}

clang/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<const CFGBlock *, 20> 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<PostOrderCFGView>());
   llvm::BitVector previouslyVisited(cfg.getNumBlockIDs());
   worklist.enqueueSuccessors(&cfg.getEntry());
   llvm::BitVector wasAnalyzed(cfg.getNumBlockIDs(), false);