r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses

[Alexander Kornienko]

Any news here? This keeps biting us.

Maybe you can apply this patch first and work on a better solution after
that?

On Tue, Aug 5, 2014 at 6:23 PM, Artyom Skrobov <Artyom.Skrobov at arm.com>
wrote:

> 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/20140808/95b3724f/attachment.html>