<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=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 20, 2014, at 11:10 AM, Hans Wennborg <<a href="mailto:hans@chromium.org" class="">hans@chromium.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">Sorry for coming late to this thread.<br class=""><br class="">On Fri, Oct 10, 2014 at 6:31 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>> wrote:<br class=""><blockquote type="cite" class="">----- Original Message -----<br class=""><blockquote type="cite" class="">From: "Jiangning Liu" <<a href="mailto:liujiangning1@gmail.com" class="">liujiangning1@gmail.com</a>><br class="">To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>><br class="">Cc: "<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a> for LLVM" <<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a>>, "Nick Lewycky" <<a href="mailto:nicholas@mxc.ca" class="">nicholas@mxc.ca</a>>, "Daniel Berlin"<br class=""><<a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a>><br class="">Sent: Friday, October 10, 2014 3:29:25 AM<br class="">Subject: Re: [llvm] r215343 - In LVI(Lazy Value Info), originally value on a BB can only be caculated once,<br class="">If you have any other empirical evidence, could you please share it?<br class=""></blockquote><br class="">No, but I think it became clear that there were serious algorithmic concerns, justified by the fact that we needed a cut-off in the first place. The fact remains that, initially, we did not think that this commit would change the asymptotic complexity of the algorithm. I remember thinking that it would only cause additional processing to happen for blocks we had already decided to visit, not cause any additional visits, and so I expected, at most, a constant-factor increase. This turned out to be wrong, and I think that Danny and Nick have done a good job explaining the error in my reasoning.<br class=""><br class="">So at this point we're having a design discussion about a proper solution, and that's great. Nevertheless, the reasoning behind approving the current implementation was that it did not change the complexity, which was wrong, and based on Danny's and Nick's notes, I'm also not convinced that the cut-offs we introduced were proper (meaning minimal). As a matter of general policy, we don't keep commits in tree for which there are serious algorithmic concerns, and so reverting was appropriate.<br class=""><br class="">Please do continue to work on this!<br class=""></blockquote><br class="">Is there any update on this patch? When it got reverted, we saw a 110<br class="">k binary size increase in Chromium.<br class=""></div></blockquote>I came across this independently a few days ago. My example was similar. I had</div><div><br class=""></div><div>if (x == 0) {</div><div> if (…) { … } else { … }</div><div> ++x</div><div>}</div><div><br class=""></div><div>The problem here is that as we walk from ++x up, we visit the BBs on the inner if statement. Their predecessors haven’t been processed yet so we return out from solveBlockValueNonLocal which leaves them in the ‘overdefined’ state. We eventually get to ‘x == 0’ and start the walk back down the CFG. But on the walk back down we do the following check and immediately bail. So intermediate BBs get set to overdefined because of unvisited predecessors and never leave that state.</div><div><br class=""></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">if</span> (!BBLV.<span style="font-variant-ligatures: no-common-ligatures; color: #31595d" class="">isUndefined</span>()) {</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #78492a" class="">DEBUG</span>(<span style="font-variant-ligatures: no-common-ligatures; color: #3d1d81" class="">dbgs</span>() << <span style="font-variant-ligatures: no-common-ligatures; color: #d12f1b" class="">" reuse BB '"</span> << BB-><span style="font-variant-ligatures: no-common-ligatures; color: #3d1d81" class="">getName</span>() << <span style="font-variant-ligatures: no-common-ligatures; color: #d12f1b" class="">"' val="</span> << BBLV <<<span style="font-variant-ligatures: no-common-ligatures; color: #272ad8" class="">'\n'</span>);</div><p style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""> <br class="webkit-block-placeholder"></p><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// Since we're reusing a cached value here, we don't need to update the </div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// OverDefinedCahce. The cache will have been properly updated </div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// whenever the cached value was inserted.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> ODCacheUpdater.<span style="font-variant-ligatures: no-common-ligatures; color: #31595d" class="">markResult</span>(<span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">false</span>);</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(187, 44, 162);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>return<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>true<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">;</span></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> }</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px;" class="">So, here’s the patch I have to fix the problem. I believe it has no algorithmic complexity issues as we do the same walk up the CFG from before, but as we walk back down we will retry calling solveBlockValueNonLocal now that all predecessors have a state. I don’t have an IR test case this minute, but I can get one tonight if needed.</div><div style="margin: 0px;" class=""><br class=""></div><div style="margin: 0px;" class=""><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><div style="margin: 0px;" class=""><span style="color: rgb(52, 187, 199);" class="">@@ -672,8 +672,16 @@</span> bool LazyValueInfoCache::solveBlockValueNonLocal(LVILatticeVal &BBLV,</div><div style="margin: 0px;" class=""> return true;</div><div style="margin: 0px;" class=""> }</div><div style="margin: 0px;" class=""> }</div><div style="margin: 0px; color: rgb(195, 55, 32);" class="">- if (EdgesMissing)</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ if (EdgesMissing) {</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ // If we haven't yet processed all the predecessors, then put this</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ // value back to undefined so that we'll revisit it again. Otherwise it</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ // will be 'overdefined' once we return from here and we'll think that</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ // this is its final state, while in fact we don't know the state of this</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ // value in this BB yet.</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ LVILatticeVal Result;</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ BBLV = Result;</div><div style="margin: 0px;" class=""> return false;</div><div style="margin: 0px; color: rgb(52, 189, 38);" class="">+ }</div></div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(52, 189, 38);" class=""><br class=""></div><div style="margin: 0px;" class="">Thanks,</div><div style="margin: 0px;" class="">Pete</div></div></div><div><blockquote type="cite" class=""><div class=""><br class=""> - Hans<br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></body></html>