<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi Artyom,</div><div class=""><br class=""></div><div class="">I was thinking about this some more, and I'm wondering if enqueuing the unreachable blocks is the wrong approach. The reality is that since these blocks are unreachable we don't even need to bother computing their dataflow values. They will simply never be used.</div><div class=""><br class=""></div><div class="">I'm also wondering about the following code in your patch:</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div class="">class PostOrderInverseCFGView : public PostOrderCFGView {</div><div class=""> typedef llvm::po_iterator<const CFG*, CFGBlockSet, true,</div><div class=""> llvm::GraphTraits<llvm::Inverse<const CFG*> ></div><div class=""> > po_iterator;</div></blockquote><br class=""></div><div class="">It is true that this inverts the order, but I don't think this definition of po_iterator is used anywhere except the constructor for PostOrderInverseCFGView. There is no virtual definition of the 'dequeue' method, so 'dequeue' (which is declared in DataflowWorklistBase) uses the po_iterator defined in DataflowWorklistBase, which is the wrong definition for reverse analysis. For reverse analysis, we want to escalate values *down* back edges so that they update the dataflow values deeper in the CFG, which then bubble up in the reverse analysis.</div><div class=""><br class=""></div><div class="">I'm wondering if we need PostOrderInverseCFGView at all. Instead, I maybe its just a matter of "virtualizing" the following code in DataflowWorklist::dequeue():</div><div class=""><br class=""></div><div class=""><div class=""> // Next dequeue from the initial reverse post order. This is the</div><div class=""> // theoretical ideal in the presence of no back edges.</div><div class=""> else if (PO_I != PO_E) {</div><div class=""> B = *PO_I;</div><div class=""> ++PO_I;</div><div class=""> }</div></div><div class=""><br class=""></div><div class="">Here you are using an iterator interface. That's fine, but it seems like we can just add one layer of abstraction. Instead, we could have some kind of object that wraps the two kind of iterators, and uses dynamic dispatch to provide the indirection. For example:</div><div class=""><br class=""></div><div class="">class DataflowBlockIterator {</div><div class="">public:</div><div class=""> virtual bool isDone() = 0;</div><div class=""> virtual void advance();</div><div class="">}</div><div class=""><br class=""></div><div class="">// something like this</div><div class="">class DataflowForwardBlockIterator : DataflowBlockIterator {</div><div class=""><div class=""> typedef llvm::po_iterator<const CFG*, CFGBlockSet, true> po_iterator;</div></div><div class=""> po_iterator PO_I, PO_E;</div><div class="">public:</div><div class=""> virtual bool isDone() { PO_I == PO_E; }</div><div class=""> virtual CFGBlock *advance() { CFGBlock *B = *PO_I; ++PO_I; return B; }</div><div class="">}</div><div class=""><br class=""></div><div class="">// something like this</div><div class=""><div class="">class DataflowReverseBlockIterator : DataflowBlockIterator {</div><div class=""><div class=""> typedef llvm::po_iterator<const CFG*, CFGBlockSet, true,</div><div class=""> llvm::GraphTraits<llvm::Inverse<const CFG*> ></div><div class=""> > po_iterator;</div></div><div class=""> po_iterator PO_I, PO_E;</div><div class="">public:</div><div class=""> virtual bool isDone() { PO_I == PO_E; }</div><div class=""> virtual CFGBlock *advance() { CFGBlock *B = *PO_I; ++PO_I; return B; }</div><div class="">}</div></div><div class=""><br class=""></div><div class="">and then do the following in dequeue():</div><div class=""><br class=""></div><div class=""><div class=""> // Next dequeue from the initial [reverse] post order. This is the</div><div class=""> // theoretical ideal in the presence of no back edges.</div><div class=""> // Note: blockIterator is of type DataflowBlockIterator*<span class="Apple-tab-span" style="white-space: pre;"> </span></div><div class=""> else if (!blockIterator->isDone()) {</div><div class=""> B = blockIterator->advance();</div><div class=""> }</div></div><div class=""><br class=""></div><div class="">Maybe that's all you need.</div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 8, 2014, at 6:32 PM, Ted Kremenek <<a href="mailto:kremenek@apple.com" class="">kremenek@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Hi Artyom,</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">I have a question about this code:</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" class=""><div class=""> // It may be that some blocks are inaccessible going from the CFG exit upwards</div><div class=""> // (e.g. infinite loops); we still need to add them.</div><div class=""> while (Blocks.size() < size) {</div><div class=""> // Find a block that we skipped</div><div class=""> CFG::const_iterator I = cfg->begin(), E = cfg->end();</div><div class=""> for (; I != E; ++I)</div><div class=""> if (!BlockOrder.count(*I))</div><div class=""> break;</div><div class=""> assert(I != E);</div><div class=""> const CFGBlock* block = *I;</div><div class=""> // Add a chain going upwards</div><div class=""> while (!BlockOrder.count(block)) {</div><div class=""> BlockOrder[block] = Blocks.size() + 1;</div><div class=""> Blocks.push_back(block);</div><div class=""> CFGBlock::const_pred_iterator PI = block->pred_begin(),</div><div class=""> PE = block->pred_end();</div><div class=""> for (; PI != PE; ++PI) {</div><div class=""> const CFGBlock* pred = *PI;</div><div class=""> if (pred && !BlockOrder.count(pred)) {</div><div class=""> block = pred;</div><div class=""> break;</div><div class=""> }</div><div class=""> }</div><div class=""> // Chain ends when we couldn't find an unmapped pred</div><div class=""> if (PI == PE) break;</div><div class=""> }</div><div class=""> }</div></blockquote><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">It looks like we will do a complete iteration of the CFG blocks many times, each time we mark a skipped block. Can we invert this loop?</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> for (CFG::const_iterator I = cfg->begin(), E = cfg->end(); I != E; ++I) {</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> if (Blocks.size() == size)</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> break;</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">That seems far more efficient. We have an invariant that once we visit a block we don't need to visit it again. Inverting the loop makes this an O(n) operation, instead of possibly O(n^2).</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Also minor, but I'd prefer this:</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> BlockOrder[block] = Blocks.size() + 1;<br class=""> Blocks.push_back(block);</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">to be written as:</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> Blocks.push_back(block);</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"> BlockOrder[block] = Blocks.size();</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">That's more consistent (I think) with how we do these things elsewhere.</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Note that even this iteration of skipped blocks isn't completely precise; we may hit a block first that dominates other blocks that are all dead.</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Perhaps a better approach is to include all unreachable blocks by querying the CFG for unreachable edges. We now try and build the CFG so that all unreachable edges aren't hard pruned, but that you can access the pruned edge via an API call:</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""> /// Get the potentially unreachable block.</div><div class=""> CFGBlock *getPossiblyUnreachableBlock() const {</div><div class=""> return UnreachableBlock.getPointer();</div><div class=""> }</div></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">That said, we may not have converted the entire CFG to record these "ghost edges". However, if you include the possibly unreachable blocks when you do your initial construction of Blocks, there will be less blocks that are skipped in the first go, and everything will be in the proper order in the worklist.</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Also minor, please make all comments be sentences that end with a '.'</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">For expediency, besides the comments being slightly tweaked, I'm fine with the patch going in as is. That would hopefully unblock Alex's current performance problem. I do think we should invert the above loop though in a subsequent patch.</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Ted</div><div class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><br class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><div class="">On Aug 8, 2014, at 8:48 AM, Artyom Skrobov <<a href="mailto:Artyom.Skrobov@arm.com" class="">Artyom.Skrobov@arm.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Thank you for the patience – indeed it took me some time to prepare the proper patch, which is now ready for review.<o:p class=""></o:p></span></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">It comes without a regression test, as the only observable change is performance improvement of the analysis.<o:p class=""></o:p></span></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div class="" style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0cm 0cm;"><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b class=""><span lang="EN-US" class="" style="font-size: 10pt; font-family: Tahoma, sans-serif;">From:</span></b><span lang="EN-US" class="" style="font-size: 10pt; font-family: Tahoma, sans-serif;"><span class="Apple-converted-space"> </span>Alexander Kornienko [<a href="mailto:alexfh@google.com" class="" style="color: purple; text-decoration: underline;">mailto:alexfh@google.com</a>]<span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>07 August 2014 23:35<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Artyom Skrobov<br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:cfe-commits@cs.uiuc.edu" class="" style="color: purple; text-decoration: underline;">cfe-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>Commits<br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses<o:p class=""></o:p></span></div></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p class=""> </o:p></div><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">Any news here? This keeps biting us.<o:p class=""></o:p></div><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p class=""> </o:p></div></div><div class=""><p class="MsoNormal" style="margin: 0cm 0cm 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;">Maybe you can apply this patch first and work on a better solution after that?<o:p class=""></o:p></p><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">On Tue, Aug 5, 2014 at 6:23 PM, Artyom Skrobov <<a href="mailto:Artyom.Skrobov@arm.com" target="_blank" class="" style="color: purple; text-decoration: underline;">Artyom.Skrobov@arm.com</a>> wrote:<o:p class=""></o:p></div><div class=""><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">So, the simplest way to restore the original behaviour seems to be</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">Index: lib/Analysis/LiveVariables.cpp</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">===================================================================</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">--- lib/Analysis/LiveVariables.cpp (revision 214183)</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">+++ lib/Analysis/LiveVariables.cpp (working copy)</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">@@ -451,6 +451,7 @@</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> // Construct the dataflow worklist. Enqueue the exit block as the</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> // start of the analysis.</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> DataflowWorklist worklist(*cfg, AC);</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">+ worklist.quick_hack();</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> llvm::BitVector everAnalyzedBlock(cfg->getNumBlockIDs());</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> // FIXME: we should enqueue using post order.</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">Index: include/clang/Analysis/Analyses/DataflowWorklist.h</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">===================================================================</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">--- include/clang/Analysis/Analyses/DataflowWorklist.h (revision 214183)</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">+++ include/clang/Analysis/Analyses/DataflowWorklist.h (working copy)</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">@@ -53,6 +53,7 @@</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> const CFGBlock *dequeue();</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> void sortWorklist();</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">+ void quick_hack() { enqueuedBlocks.reset(); PO_I = PO_E; }</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);">};</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);"> } // end clang namespace</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">This has the effect of disabling the fallback post-order iteration.</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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.</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">I’m currently thinking of a proper solution which would allow using PostOrderCFGView for iteration in either direction.</span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p class=""></o:p></div><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span class="" style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p class=""></o:p></div><div class="" style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0cm 0cm;"><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b class=""><span lang="EN-US" class="" style="font-size: 10pt; font-family: Tahoma, sans-serif;">From:</span></b><span lang="EN-US" class="" style="font-size: 10pt; font-family: Tahoma, sans-serif;"><span class="Apple-converted-space"> </span>Alexander Kornienko [mailto:<a href="mailto:alexfh@google.com" target="_blank" class="" style="color: purple; text-decoration: underline;">alexfh@google.com</a>]<span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>05 August 2014 12:30</span><o:p class=""></o:p></div><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Artyom Skrobov<br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" class="" style="color: purple; text-decoration: underline;">cfe-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>Commits<o:p class=""></o:p></div></div><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: r214064 - Factoring DataflowWorklist out of LiveVariables and UninitializedValues analyses<o:p class=""></o:p></div></div></div><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p class=""></o:p></div><div class=""><div class="" style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">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.<o:p class=""></o:p></div><div class=""><p class="MsoNormal" style="margin: 0cm 0cm 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"> <o:p class=""></o:p></p></div></div></div></div></div></div></div></div></div><span id="cid:FEE35756-4F3F-4FB6-BF31-1BDF04F590F7@apple.com" class=""><DataflowWorklistInverse.patch></span><span class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;">_______________________________________________</span><br class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;">cfe-commits mailing list</span><br class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><a href="mailto:cfe-commits@cs.uiuc.edu" class="" style="color: purple; text-decoration: underline; font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">cfe-commits@cs.uiuc.edu</a><br class="" style="font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" class="" style="color: purple; text-decoration: underline; font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></div></blockquote></div></div></blockquote></div><br class=""></body></html>