[llvm-commits] [patch] Use the new edge dominance functions in GVN

[Eli Friedman]

On Mon, Aug 13, 2012 at 8:05 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
>> In any case,  an updated patch is attached. I made it slightly less
>> aggressive but faster by keeping isOnlyReachableViaThisEdge as a fast
>> dominance approximation. I understand that it is taking a technical
>> debt, but I think I should be able to repay it :-)
>
> Looking at the code of the new GVN pass I see that it implements a
> similar feature by doing
>
>       AU.addRequiredID(BreakCriticalEdgesID)
>
> While this is a possibility, applying this to the current GVN
> implementation causes some problems. First, we have to mark
> BreakCriticalEdges as preserving AliasAnalysis and
> MemoryDependenceAnalysis, which I guess is fine.
>
> The addition of extra basic blocks seems to prevent some optimizations
> and causes two tests to fail:
>
>     LLVM :: CodeGen/Thumb2/2009-12-01-LoopIVUsers.ll
>     LLVM :: Transforms/GVN/2011-06-01-NonLocalMemdepMiscompile.l
>
> I have not debugged exactly why. Finally, it seems that when looking
> at the time of a full opt -O2 run, at least with the current
> algorithm, the impact of splitting the critical edge is larger than
> the impact of directly reasoning about a use being dependent on an
> edge.
>
> In any case. I have attached both the previous patch and a variation
> that uses critical edge splitting instead. If there is a preference
> for the edge splitting one I can try to figure out the two remaining
> test failures.

We don't want to add additional users of the BreakCriticalEdges pass;
it's bad for performance.  If you need to break a specific critical
edge, it should be done on demand, where necessary.

-Eli