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

[Chris Lattner]

On Aug 14, 2012, at 10:35 PM, Daniel Berlin <dannyb at google.com> wrote:
>> 
>>> For example, current LLVM GVN notes critical edges blocking PRE, then
>>> splits them, then  *reiterates all of its PRE* just to deal with a
>>> critical edge.
>> 
>> Ok, well that's also insane. :-)
> 
> Fair, but IMHO, that's the typical solution that gets taken when folks
> take algorithms not meant to work on CFG's critical edges, and
> implement them in a situation where they aren't allowed to break
> critical edges.

Sure, but LLVM deviates from "common practice" in many ways when common practice is wrong ;-)

>>> Is there some mailing list discussion or performance data you can
>>> point me at that shows why it is such a problem?
>> 
>> Here is some intuition: splitting a critical edge in LLVM requires:
>> 
>> 1) a new BasicBlock
>> 2) a new unconditional branch
>> 3) updating any phi nodes in the successor
>> 4) various use-list manipulations

I also forgot that having split critical edges makes all other CFG analyses (e.g. dominators, loops) slower and use more memory... sometimes a LOT more.

> Sure.
> But if you do it in the middle of pass computations, you still get
> that same cost, but now add
> 
> 5. Updating whatever local datastructures exist (like availability and
> leaders) that are auxillary info.
> 6. Updating analysis passes (like memdep) that do basic block caching
> or paying for slow queries.
> 
> You hope is that you only do this for a few edges, instead of all, but
> that depends on how good your other cleanup passes are and what
> possibilities are still left.
> Otherwise, the cost of 5 + 6 alone usually greatly outweighs 1-4 in practice.

Right.  The principle here is that even great and general optimizations like GVN don't actually need to split every critical edge in practice.  It only needs to split an edge if it wants to place a computation there, and making the expense of an optimization proportional to the amount of goodness it delivers is a really nice thing. :)

> But we shouldn't simply say "we should never split all critical edges
> before pass because it's compile time expensive" when it could be less
> expensive than "split all critical edges before doing complicated
> analysis and caching that can't be updated in the middle".

I agree in principle, but I'm hoping that cleverness can be applied to all optimizations that are traditionally described in terms of split critical edges.  Reformulating things defined in terms of blocks to be defined in terms of edges is usually enough. 

>> bb1:
>>   ; preds = switchblock, switchblock, switchblock
>> 
>> 
>> Guess what - we've got three critical edges right there from the switch to bb1... and splitting these into three *separate* paths is both incredibly
>> inefficient (because many switches have a ton of cases) and brutally terrible to runtime performance if anything actually gets put into one of those > blocks.  This is the idea behind the "split critical edges 'nicely'" APIs that some passes use.
> 
> Currently, GVN's PRE is going to do this, but it's going to do it one
> edge at a time, and re-evaluate all the possibilities on each
> iteration.

But the point is that GVN (as opposed to the generic crit edge breaking pass) can be smart enough to know it doesn't need "textbook" edge spliting, it can use "nice" edge splitting.

-Chris