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

[Daniel Berlin via llvm-commits]

+1. Real time updating in the presence of how  JT operates right now is too
costly.
The lazy analysis it uses require DT be updated at all times, forever
(instead of say, once an iteration).
There is no way to update the domtree 100000 times a pass and make it fast
:)
What we should be targeting is O(10)-O(100) updates.


On Mon, Mar 19, 2018 at 9:09 AM, Brian Rzycki via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180319/1facb28e/attachment.html>