<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 15, 2017 at 3:59 PM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Sorry, i normally only scan llvm-commits mail occasionally (gotta reod my filtering), i didn't notice this until joerg pointed it out to me.<div><div class="gmail_extra"><br><div class="gmail_quote"><span class="gmail-">On Wed, Feb 8, 2017 at 9:29 PM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Danny,<br>
<br>
I feel like there's something missing here.  In particular, I'm not sure I agree with your description of the problem.  See inline comments below.  Can you help me understand why this is needed?</blockquote></span><div>Sure.</div><div>Happy to also provide a testcase if it helps.</div><span class="gmail-"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span><br>
<br>
On 02/08/2017 07:22 AM, Daniel Berlin via llvm-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Author: dannyb<br>
Date: Wed Feb  8 09:22:52 2017<br>
New Revision: 294463<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=294463&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=294463&view=rev</a><br>
Log:<br>
LVI: Add a per-value worklist limit to LazyValueInfo.<br>
<br>
Summary:<br>
LVI is now depth first, which is optimal for iteration strategy in<br>
terms of work per call.  However, the way the results get cached means<br>
it can still go very badly N^2 or worse right now.  The overdefined<br>
cache is per-block, because LVI wants to try to get different results<br>
for the same name in different blocks (IE solve the problem<br>
PredicateInfo solves).  This means even if we discover a value is<br>
overdefined after going very deep, it doesn't cache this information,<br>
causing it to end up trying to rediscover it again and again.  The<br>
same is true for values along the way.<br>
</blockquote></span>
This doesn't parse for me.  We have the OverdefinedCache and do cache the result of a (Value, BB) pair being overdefined.  Other than the fact the cache is split - which admitted is confusing - where's the problem here?</blockquote></span><div>Given<br></div><div>BB 1</div><div>BB 2</div><div>BB 3</div><div>BB 4</div><div><br></div><div>Overdefined in one is not equal to overdefined in all.</div><div>So we cache in one bb</div><div>then ask about the same value in the next bb.</div><div>So it keeps rediscovering the overdefinedness of it.<br></div><div>16000 times per value.</div><div><br></div></div></div></div></div></blockquote><div><br></div><div>It's worse than this, btw.</div><div><br></div><div>Imagine i have the following horrible code:<br><br></div><div>entry:<br>tmp1 = overdefined</div><div>tmp2 = overdefined</div><div>.....</div><div>tmp16000 = overdefined</div><div><br></div><div>bb0:</div><div>nothing</div><div>br label bb1</div><div>....</div><div>bb16000:</div><div>use tmp1</div><div><div>use tmp2 </div></div><div>...</div><div>use tmp16000</div><div>br label 16001</div><div>bb16001:<br>nothing</div><div>br label 16002</div><div>....</div><div>bb32000:</div><div>use tmp1</div><div>use tmp2</div><div>...</div><div>use tmp16000</div><div><br></div><div>Let's look at the work between the two schemes:</div><div><br></div><div>In the per-block overdefined scheme, with no predicateinfo</div><div><br></div><div>For the use of tmp1 in bb16000, it will walk 16000 blocks back to the entry block, discover overdefined. Assume it properly caches all 16000 blocks (I actually believe it does not in some cases, but let's call that a bug and ignore it. it doesn't actually change our work done in this case).</div><div>We have spent 16k blocks discovering overdefinedness here.</div><div>For the use of tmp2..tmp16000 in bb16000, ditto</div><div>We do this to see if it has any conditions, etc, that give us info.</div><div><br></div><div>Let's go to bb32000</div><div>Again, let's assume it's all cached per-bb properly.</div><div><div>For the use of tmp1 in bb32000, it will walk 16000 blocks back to bb16000, discover overdefined. Assume it properly caches all 16000 new blocks </div><div>We have spent 16k blocks discovering overdefinedness here for each variable in bb32000.</div></div><div><br></div><div>In the predicateinfo scheme where overdefined is global, and uses are renamed where value could change:</div><div><br><div>For the use of tmp1 in bb16000, we see where the def is. It's in block bb1. Because we know the use would have been renamed if the value could have changed due to conditions, etc, we can skip all the blocks in the middle. They are irrelevant.   We look at bb1 (and use any predicateinfo to check conditions), see it's overdefined.</div><div>We have spent 1 block discovering overdefinedness here.</div><div>For the use of tmp2..tmp16000 in bb16000, ditto<br></div><div><br></div><div>Let's go to bb32000</div><div>Remember that overdefined is now global, because it would have been renamed if it was dominated by something that could change the value.</div><div>For the use of tmp1 in bb16000, it will walk 0 blocks. The value is overdefined for sure.</div></div><div>Ditto for tmp2..tmp16000</div><div><br></div><div>You can make the testcase worse for the current scheme by adding more splits.</div><div><br></div><div>IE imagine the above example, except two chains of 16000 blocks that get merged in bb 32000.</div><div><br></div><div>In the predicateinfo scheme, the only extra time we spend is what it takes to look at the predicateinfo, if any (if it isn'tlive in those blocks, and it isn't above, we don't create any) to see if it tells us anything.</div><div>In the current overdefined scheme, depending on pred ordering, even if we have cached one of the chains of 16k blocks to overdefined, we may look at the other chain first.</div><div><br></div><div>(again, we could hack a heuristic into the current one to follow a branch where it is dominated by some cached info, and we estimate the block distance between us and the cached info to be Y, but this gets complicated quickly).</div><div><br></div></div></div></div>