<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 13, 2017 at 11:05 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"><span class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">That may actually be fast enough already if you do that, which i'm hoping (but you know better than I) is not a lot more work than you have here.</span></blockquote></span><div>There are a couple of complications with this method as I think about it. The first is the existence of DT used outside of the Jumpthreading pass: Local.cpp and other helper functions like the ones in BasicBlockUtils.cpp. I'll have to flush before calling these sites, or rework the interfaces to them to have an optional vector passed around. The hand-off between these separate functions will require me figuring out the most efficient way of updating the DT.</div></div></blockquote><div><br></div><div>Right. This is what i said previously, it's a bit sucky.</div><div>IMHO, the right thing to do is to make a simple updater class and pass it around.</div><div>For starters, it can just wrap a vector of updates and have the simple API i gave.</div><div>Add eraseBlock. and internally make it call applyUpdates.</div><div>(Like i said, we can probably remove this limitation later, but we will have to do some bookkeeping anyway, so having eraseBlock there would  work)<br></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><br></div><div>The second is I haven't done the analysis of exactly where I need the DT analysis preserved within the JT pass. Part of the reason we did the work this way was so we didn't have to worry about the state of DT, we could just use it at any point. This proposal changes it to be a much more explicit sync-then-analysis event.</div></div></blockquote><div><br></div><div>Yes. But at the current cost, i can't see how you can make this work.</div><div>We can make batching more efficient.</div><div>There are no real known algorithms to make individual single-update-at-a-time updating significantly more efficient than we are doing (there are a couple optimizations, but profiling this change, they don't cover enough of the cases).</div><div>For example, it's possible to make 'nodes that are eventually deleted in your sequence of batch updates' nearly constant time in batch updating.</div><div>Ditto for nodes that become leaves, etc.</div><div>None of this is possible in single update-at-a-time.</div><div><br></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><br></div><div>I suspect both of these will cause more frequent than "block is deleted so flush DT" style code. It might be more efficient to batch the updates, I'm not sure.</div><div><br></div><div>Finally, I'd need a way to detect if the root block is to be removed from the function: in that case I have to recalculate the DT.</div></div></blockquote><div><br></div><div>We could fix this in general by virtual rooting both trees, instead of just postdom.</div><div><br></div><div>But there is another thing we could do:<br>You can only ever replace the entry block with a different block as an entry, you can't just remove it.</div><div>That new block should have no predecessors</div><div><br></div><div>As such, it should be a trivial update of reattaching the disconnected block as the new root.</div><div>In fact, i bet we could do this as something like this:</div><div>implement swap for DT nodes</div><div>swap the old entry and the new entry node data</div><div>remove the old entry node</div><div><br></div><div>and even the following may work </div><div>Update dt.roots</div><div>temp = DT.nodemapping[oldentry]</div><div>DT.nodemapping.erase(oldentry)</div><div>DT.nodemapping.insert(newentry, temp)</div><div><br></div><div>But one thing at a time.</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><br></div><div>If this is the recommendation going forward I can start on this work.</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br></div></div></div></blockquote><div>IMHO, i can't see another way to do this.  There are no known algorithms to make what we are doing faster, and while it's a fertile area of research, i don' think we will be the ones to come up with a breakthrough here :)</div><div><br></div></div></div></div>