[cfe-dev] [cfe-commits] r134697 - /cfe/trunk/lib/Analysis/UninitializedValues.cpp

David Blaikie dblaikie at gmail.com
Fri Jul 8 08:28:02 PDT 2011


Is there a bug for why this scenario happened at all, given that the
variables in the switch were locally scoped to each case, or is that by
design for some reason? From: Chandler Carruth
Sent: Friday, July 08, 2011 4:20 AM
To: cfe-commits at cs.uiuc.edu
Subject: [cfe-commits] r134697
- /cfe/trunk/lib/Analysis/UninitializedValues.cpp
Author: chandlerc
Date: Fri Jul  8 06:19:06 2011
New Revision: 134697

URL: http://llvm.org/viewvc/llvm-project?rev=134697&view=rev
Log:
Make the worklist in the uninitialized values checker actually a queue.
Previously, despite the names 'enqueue' and 'dequeue', it behaved as
a stack and visited blocks in a LIFO fashion. This interacts badly with
extremely broad CFGs *inside* of a loop (such as a large switch inside
a state machine) where every block updates a different variable.

When encountering such a CFG, the checker visited blocks in essentially
a "depth first" order due to the stack-like behavior of the work list.
Combined with each block updating a different variable, the saturation
logic of the checker caused it to re-traverse blocks [1,N-1] of the
broad CFG inside the loop after traversing block N. These re-traversals
were to propagate the variable values derived from block N. Assuming
approximately the same number of variables as inner blocks exist, the
end result is O(N^2) updates. By making this a queue, we also make the
traversal essentially "breadth-first" across each of the N inner blocks
of the loop. Then all of this state is propagated around to all N inner
blocks of the loop. The result is O(N) updates.

The truth is in the numbers:
Before, gcc.c:   96409 block visits  (max: 61546,   avg: 591)
After,  gcc.c:   69958 block visits  (max: 33090,   avg: 429)
Before, PR10183: 2540494 block vists (max: 2536495, avg: 37360)
After,  PR10183: 137803 block visits (max: 134406,  avg: 2026)

The nearly 20x reduction in work for PR10183 corresponds to a roughly
100x speedup in compile time.

I've tested it on all the code I can get my hands on, and I've seen no
slowdowns due to this change. Where I've collected stats, the ammount of
work done is on average less. I'll also commit shortly some synthetic
test cases useful in analyzing the performance of CFG-based warnings.

Submitting this based on Doug's feedback that post-commit review should
be good. Ted, please review! Hopefully this helps compile times until
then.

Modified:
    cfe/trunk/lib/Analysis/UninitializedValues.cpp

Modified: cfe/trunk/lib/Analysis/UninitializedValues.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/UninitializedValues.cpp?rev=134697&r1=134696&r2=134697&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/UninitializedValues.cpp (original)
+++ cfe/trunk/lib/Analysis/UninitializedValues.cpp Fri Jul  8 06:19:06 2011
@@ -288,28 +288,28 @@
 public:
   DataflowWorklist(const CFG &cfg) : enqueuedBlocks(cfg.getNumBlockIDs()) {}

-  void enqueue(const CFGBlock *block);
   void enqueueSuccessors(const CFGBlock *block);
   const CFGBlock *dequeue();
-
 };
 }

-void DataflowWorklist::enqueue(const CFGBlock *block) {
-  if (!block)
-    return;
-  unsigned idx = block->getBlockID();
-  if (enqueuedBlocks[idx])
-    return;
-  worklist.push_back(block);
-  enqueuedBlocks[idx] = true;
-}
-
 void DataflowWorklist::enqueueSuccessors(const clang::CFGBlock *block) {
+  unsigned OldWorklistSize = worklist.size();
   for (CFGBlock::const_succ_iterator I = block->succ_begin(),
        E = block->succ_end(); I != E; ++I) {
-    enqueue(*I);
+    const CFGBlock *Successor = *I;
+    if (!Successor || enqueuedBlocks[Successor->getBlockID()])
+      continue;
+    worklist.push_back(Successor);
+    enqueuedBlocks[Successor->getBlockID()] = true;
   }
+  if (OldWorklistSize == 0 || OldWorklistSize == worklist.size())
+    return;
+
+  // Rotate the newly added blocks to the start of the worklist so
that it forms
+  // a proper queue when we pop off the end of the worklist.
+  std::rotate(worklist.begin(), worklist.begin() + OldWorklistSize,
+              worklist.end());
 }

 const CFGBlock *DataflowWorklist::dequeue() {


_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-dev mailing list