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

[Daniel Berlin]

On Wed, Aug 15, 2012 at 8:42 AM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
>> I do have numbers to show that in at least two compilers, the
>> pre-splitting approach was faster than the iteration approach, and the
>> pseudo-cfg approach.
>
> Sorry, lets back out a bit. The testcases I have found from working on
> pr13307 can be fixed by splitting critical edges *or* by having the
> notion of an edge dominating a use available to GVN.

Great!

>
> While the logic for edge dominance is not trivial, it is not very
> different from what we have for handling indirectbr.
>
> I might take a look at Chris suggestion of updating the GVN evaluation
> logic to use edge dominance. Specially since it might offset the cost
> of reasoning about edges when propagating equality.

That would be nice.
>
> Daniel, the paper you are basing the new implementation on mentions
> edge dominance. You choose to eagerly split edges because of previous
> performance experience or there is some testcase where it is needed?

Performance experience.
My recollection is it starts to fall down very quickly on code
generated code that has a lot of switch statement
fallthroughs/critical edges.  I should hopefully still be able to find
them (I believe the worst were from some scheme-to-c compiler)

This often causes PRE that edge-splits on demand to iterate.
Iteration in PRE, copying sets around, and recalculation of other
stuff, always took more time than pre-splitting edges.

When we did do on-demand edge splitting were some bugs here and there
around needing to do recursive edge splits and updating stuff in that
presence, but those were just bugs.

>
> I left a benchmark of 4 different implementations running during the
> night (attached). The implementations are
>
> * master: trunk r161923.
> * patch1: My previous patch that reasons about edge dominance.
> * patch2: My patch that eagerly splits critical edges.
> * patch1-dont-check: A variation of patch 1 that handles multiple
> edges in GVN itself instead of doing it in Dominance.cpp (attached)
>
> The benchmark is running "opt -O2" on the bitcode of gcc as a single
> file produced with "clang -c -emit-llvm -O2 -Xclang
> -disable-llvm-optzns".

>
> The results show that, at least on the current implementation, eager
> edge splitting is the most expensive option.

Sure. The current implementation does a fairly local form of PRE, and
isn't going to find that many opportunities right now anyway.
I can come up with a real  testcase where it's the most expensive, if
you like :)

>
> I would like the attached patch a lot more if I was able to assert in
> Dominance.cpp that we are not given a multiple edge, but I have to
> refactor the verifier a bit before it can be done.
>
> So I would like to propose
>
> * We go with the attached patch for now.
> * I will try to change the verifier so it doesn't try to check
> dominance on an invoke it already knows is broken. With that we should
> be able to assert in Dominance.cpp that we are not passed multiple
> edges.
> * I will try to look at cases where GVN split egdes to see if can be avoided.

You could copy the leader sets around
>
> On the other hand, if the new algorithm Daniel is implementing works
> better with edge splitting and the overall result is better, I think
> that is fine too. A agree with him we should not just assume it.
>
>> -Eli
>
> Cheers,
> Rafael