[PATCH] D40146: [JumpThreading] Preservation of DT and LVI across the pass

[Brian Rzycki via Phabricator via llvm-commits]

brzycki added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp:2610
+  // the above calls to update Dominators.
+  DDT->applyUpdates(
+      {// Guarded block split.
----------------
fhahn wrote:
> I think DuplicateInstructionsInSplitBetween can easily update the DT itself: D44629. Is it better to do the update incrementally here or make DuplicateInstructionsInSplitBetween take care of it (via splitBlock)? Letting functions update the DT themselves seems to be what similar functions do.
> 
> 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 :)
Hi @fhahn , when I initially did the work to preserve `DT` across JumpThreading I tried to do two things:

  # Minimize the changes to external calls.
  # Update the `DT` real-time.

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 [[ http://llvm.org/OpenProjects.html#llvm_domtree_updater | 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...

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. :)


Repository:
  rL LLVM

https://reviews.llvm.org/D40146