<div dir="ltr">+1. Real time updating in the presence of how  JT operates right now is too costly.<div>The lazy analysis it uses require DT be updated at all times, forever (instead of say, once an iteration).</div><div>There is no way to update the domtree 100000 times a pass and make it fast :)</div><div>What we should be targeting is O(10)-O(100) updates.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 19, 2018 at 9:09 AM, Brian Rzycki via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">brzycki added inline comments.<br>
<span class=""><br>
<br>
================<br>
Comment at: llvm/trunk/lib/Transforms/<wbr>Scalar/JumpThreading.cpp:2610<br>
+  // the above calls to update Dominators.<br>
+  DDT->applyUpdates(<br>
+      {// Guarded block split.<br>
----------------<br>
</span><span class="">fhahn wrote:<br>
> I think DuplicateInstructionsInSplitBe<wbr>tween can easily update the DT itself: D44629. Is it better to do the update incrementally here or make DuplicateInstructionsInSplitBe<wbr>tween take care of it (via splitBlock)? Letting functions update the DT themselves seems to be what similar functions do.<br>
><br>
> We do not have access to the plain DT here though, which makes that slightly trickier and that's why I want to check what's better before doing any changes here :)<br>
</span>Hi @fhahn , when I initially did the work to preserve `DT` across JumpThreading I tried to do two things:<br>
<br>
  # Minimize the changes to external calls.<br>
  # Update the `DT` real-time.<br>
<br>
Compile time testing showed that updating the `DT` real-time was just too slow. This not-so-great compromise is the deferred class we have today. Any time we have to call `DDT-flush()` incurs a compile-time penalty which is why you can find external calls that take `DT` as a parameter but I didn't use it because it'd require me to flush first. I'd also like to avoid having multiple functions that take both a `DT` and a `DDT` parameter. It complexifies function signatures, positional `nullPtr` placement, and more branchy checks inside these routines.  I know that @kuhar is leading a [[ <a href="http://llvm.org/OpenProjects.html#llvm_domtree_updater" rel="noreferrer" target="_blank">http://llvm.org/OpenProjects.<wbr>html#llvm_domtree_updater</a> | GSoC topic to unify the DT/PDT/DDT interface ]] and it may be worth following that before spending any of your time trying to hack on this mess...<br>
<br>
You are more than welcome to see if you can make changes here, just make sure test-suite ctmark isn't adversely impacted when you do. :)<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D40146" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D40146</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div>