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

[Chris Lattner]

On Aug 13, 2012, at 11:38 PM, Daniel Berlin <dannyb at google.com> wrote:
>>> 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.
> 
> In what exact way?

Eli is right.  While the pass is convenient, it is really bad for compile time performance.

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

> Additionally, to my recollection, in the history of GCC, critical edge
> splitting, even on huge functions, has never shown up as any real
> performance hit (IE > 0.1% of the total profile).
> 
> 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

This isn't *hugely* expensive, but it isn't free either.  This is particularly bad because we also have SimplifyCFG, whose job it is to return the CFG back to canonical form, thus *undoing* this.  The worst case then is that you have a pass that requires critical edges to be broken, but that rarely actually kicks in to do something.  Then you effectively get:

BreakCriticalEdges
YourPass
SimplifyCFG

and since YourPass "usually" doesn't actually do anything (maybe it is something like loop unswitch, which only kicks in on specific CFG structures), we end up burning compile time breaking *all* critical edges, and then throwing that work away by reunifying them away.

There is another *major* problem with critical edge splitting, at least in the textbook definition that BreakCriticalEdges implements: consider a switch statement where multiple destinations go to the same block.  This is very common:

switch (c) {
case 1:
case 24:
case 37:
   goto bb1
case ...

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.


The upshot of this is that it is *far* better to have the passes that want critical edges broken to do it themselves, when there is actually a reason to do it.  In the case of GVN, the "analysis" part of the algorithm should be updated to work on a pseudo-CFG that looks as-if the edges were broken, and then actually do it when there is a reason to do so.

-Chris