r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses

[Ted kremenek]

I think we should just apply this patch now as we iterate over the proper solution.  We can then revert it.

> On Aug 7, 2014, at 3:35 PM, Alexander Kornienko <alexfh at google.com> wrote:
> 
> 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.
>> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140808/ae90d188/attachment.html>