<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 13, 2017 at 9:41 AM, Brian M. Rzycki <span dir="ltr"><<a href="mailto:brzycki@gmail.com" target="_blank">brzycki@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Daniel, thanks for coming back to the conversation.<span class=""><div><br></div><div><br></div><div><span style="font-size:12.8px">> </span><span style="font-size:12.8px">In that sense, 4% is really really good.  You are saving probably</span></div><div><span style="font-size:12.8px">> 100's or 1000's of dominator tree rebuilds to get the equivalent preservation.</span></div></span><div><span style="font-size:12.8px">One of my first attempts at preserving DT across JT was to simply use recalculate(). It was terribly slow but helped me to identify the areas where DT needed to be properly preserved with the new API.</span></div><span class=""><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">> This is being done presumably to enable some better optimization.</span><span style="font-size:12.8px"><br></span></div></span><div><span style="font-size:12.8px">Correct. The JT pass in LLVM misses opportunities GCC catches. Two of the areas Sebastian and I would like to explore are threaded jumps across loop boundaries and 2 (or more) blocks threaded in one pass. DT is essential to adding LI analysis preservation to detect loops for jumping.</span></div><span class=""><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">> </span><span style="font-size:12.8px">The only other option is to batch more (which i believe should just work.</span></div><div><span style="font-size:12.8px">> You should be able to keep a more global update vector, and apply less,</span></div><div><span style="font-size:12.8px">> and it should just work)</span></div></span><div><span style="font-size:12.8px">The jumpthreading pass makes heavy use of functions in Local.cpp and a few other calls that update DT like the ones in BasicBlockUtils. It would take coordination between these different call sites to use the global data structure. </span></div></div></blockquote><div><br></div><div>Or an updater object :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><span style="font-size:12.8px">If there was some way to "defer" updates in the DT interface that may help mitigate some of this overhead. Something like:</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">DT->deleteEdgeDeferred(BB, BBFoo);</span></div><div><span style="font-size:12.8px">...</span></div><div><span style="font-size:12.8px">DT->deferredUpdates(<wbr>UpdateVector);</span></div><div><span style="font-size:12.8px">...</span></div><div><span style="font-size:12.8px">DT->applyDeferredUpdates();</span></div></div></blockquote><div><br></div><div>This should already work.</div><div>The batch updater should be able to apply the updates whenever you want.  It should not require they be applied in lockstep with the CFG.</div><div><br></div><div>The only requirement, i believe, is that the CFG nodes themselves (not the edges) don't go completely away because it literally needs to be able to query them without use after free (so it can, for example, get the DT node for a CFG node).</div><div>This seems fixable to me.</div><div>If you have an updater object, you use WeakVH for the CFG nodes so they go away without use after free, and Updater.add stores a pointer to the DT node so we can call deleteunreachable on it (by definition, any completely erased cfg node must be unreachable, or else you are going to have a bad time anyway).</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">It would also have the benefit if people use the older API then DT will know to implicitly applyDeferredUpdates before executing the older functions.</span></div><div><span style="font-size:12.8px"><br></span></div></div></blockquote><div><br></div><div>I suggested this interface at one point to Kuba, but it turns out there are things that directly access the bits of DT that make this likely to fail as an interface, unfortunately.</div><div><br></div></div></div></div>