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

[Rafael Espíndola]

> He's also trying to reconcile the fact that i get about 1 hour to work
> on it a week at this point, due to dealing with chandler-made-stuff,
> so he's politely trying to find a way to get it done without
> mentioning that it's 1+ month late :)

Since pr13307 is part of one of the performance regressions we are
seeing, I might be able  to put an hour or so in helping with the gvn
rewrite :-)

>> I will take a look, but I would also like to fix these small issues on
>> the current one. If nothing else it provides testcases for the new one
>> :-)
>
> Don't take this the wrong way, but the "small issues' have been piling
> up with the old one for a long time.  It's pretty crufty at this
> point.  For example, the edge dominance stuff you are doing to solve
> 13307 would be better taken care of by implementing the predicate
> support + value inference from the new algorithm, and making it fast
> (I had implemented it, decided i wouldn't have time to make it fast
> for the first commit, and ripped it back out).

Do you have a pointer to a description of this algorithm?

> It takes care of issues like this in a nice, clean way, without having
> to try to guess locally substituting which equalities will turn out
> better later on.

Yes that is a problem with the current one. The patch I am working
right now just exposes more opportunities. To actually fix 13307 I
need some heuristic on how to use the equality. In that test replacing
null with %0 is the right thing to do as it simplifies the phi.

> The current algorithm relies *heavily* on catching everything in a
> single iteration so the next iteration gets the advantage, rather than
> having a clean separation between analysis and removal, and making the
> analysis work well, etc.  If you take your example from 13307, and
> through another variable in it that gets copies of x, etc, you will
> see it starts to take more iterations to accomplish the same thing.
>
> This is actually a fairly bad and slow way to accomplish things in the
> long run. It works pretty well for small cases, but, for example, as I
> showed nick, it means that it turns local load dependency checking
> into slow non-local load dependency checking as it removes loads on
> each iteration, etc.

I see. So the main point of your rewrite is to replace the current
iterative replacement with one "complete" analysis step followed by a
single replacement step, is that it?

>
> Anyhoo, the github branch is https://github.com/dberlin/llvm-gvn-rewrite
>
> It's likely slightly broken at this exact moment, i was in the midst
> of fixing up a small issue, and needed to leave for chicago, so i
> pushed what i had before i left.
>
> I'll push a fixed version when i get back to my laptop on monday.
>
> (The version there will be faster in most cases, but slower in some
> degenerate cases. If you will profile it, you will see all of those
> cases spend 90+% of their time in getNonLocalPointerDependency, which
> I was going to fix as a followup).

Thanks. If I am able to put some time on it, what do you think would
be the best thing for me to work on? Implementing the "predicate
support + value inference" so that it passes the tests I included in
my patch?

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

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: application/octet-stream
Size: 13096 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120811/7b91dbbc/attachment.obj>