r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses

[Artyom Skrobov]

So, the simplest way to restore the original behaviour seems to be

 

Index: lib/Analysis/LiveVariables.cpp

===================================================================

--- lib/Analysis/LiveVariables.cpp   (revision 214183)

+++ lib/Analysis/LiveVariables.cpp   (working copy)

@@ -451,6 +451,7 @@

   // Construct the dataflow worklist.  Enqueue the exit block as the

   // start of the analysis.

   DataflowWorklist worklist(*cfg, AC);

+  worklist.quick_hack();

   llvm::BitVector everAnalyzedBlock(cfg->getNumBlockIDs());

   // FIXME: we should enqueue using post order.

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

===================================================================

--- include/clang/Analysis/Analyses/DataflowWorklist.h     (revision 214183)

+++ include/clang/Analysis/Analyses/DataflowWorklist.h     (working copy)

@@ -53,6 +53,7 @@

   const CFGBlock *dequeue();

   void sortWorklist();

+  void quick_hack() { enqueuedBlocks.reset(); PO_I = PO_E; }

};

 } // end clang namespace

 

This has the effect of disabling the fallback post-order iteration.

 

The reason for the performance regression, however, is related to the iteration order over the CFG: UninitializedValues expects to use the default (i.e. reverse) PostOrderCFGView iteration, whereas LiveVariables expects the opposite order, corresponding to using llvm::GraphTraits<llvm::Inverse<const CFG*> > > as the last parameter in typedef po_iterator.

 

I’m currently thinking of a proper solution which would allow using PostOrderCFGView for iteration in either direction.

 

 

 

From: Alexander Kornienko [mailto:alexfh at google.com] 
Sent: 05 August 2014 12:30
To: Artyom Skrobov
Cc: cfe-commits at cs.uiuc.edu Commits
Subject: Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses

 

On another file from the same project the difference in my setup is 2 minutes before the patch vs. 50+ hours (I terminated the analyzer before it finished) after the patch. So it's a rather bad performance regression.

 

On Tue, Aug 5, 2014 at 10:17 AM, Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:

Indeed, for the other version of options.c, the analysis (on my machine) takes 3 sec before the patch, and 6 min after the patch.

It doesn’t hang indefinitely though, and the output is the same in both cases; so what we have is a performance regression.

 

I’m now trying a fix.

 

 

From: Alexander Kornienko [mailto:alexfh at google.com] 
Sent: 04 August 2014 19:26
To: Artyom Skrobov


Subject: Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses

 

On Mon, Aug 4, 2014 at 4:36 PM, Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:

Are you sure this issue has anything to do with my patch?

I’ve just run “clang -cc1 -analyze -analyzer-checker=deadcode.DeadStores options.i” using a build of Clang pre-dating my patch by a couple of weeks; and the analysis hangs nevertheless.

 

You're right. This specific file causes hangs regardless of your patch. Sorry for not verifying this.

 

However, I've got another version of this file, which only makes analyzer hang with your patch. Please try it.

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140805/6fa711f8/attachment.html>