[llvm] r291968 - NewGVN: Move leaders around properly to ensure we have a canonical dominating leader. Fixes PR 31613.

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 13 17:22:07 PST 2017


On Fri, Jan 13, 2017 at 2:40 PM, Daniel Berlin via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: dannyb
> Date: Fri Jan 13 16:40:01 2017
> New Revision: 291968
>
> URL: http://llvm.org/viewvc/llvm-project?rev=291968&view=rev
> Log:
> NewGVN: Move leaders around properly to ensure we have a canonical dominating leader. Fixes PR 31613.
>
> Summary:
> This is a testcase where phi node cycling happens, and because we do
> not order the leaders by domination or anything similar, the leader
> keeps changing.
>
> Using std::set for the members is too expensive, and we actually don't
> need them sorted all the time, only at leader changes.
>
> We could keep both a set and a vector, and keep them mostly sorted and
> resort as necessary, or use a set and a fibheap, but all of this seems
> premature.
>
> After running some statistics, we are able to avoid the vast majority
> of sorting by keeping a "next leader" field.  Most congruence classes only have
> leader changes once or twice during GVN.
>
> Reviewers: davide
>

I think this can be merged to stable, as it fixes the last known issue
I was able to find on the codebases I tried (of course, not the last
issue in general). I have the hope of making this available for
broader testing internally (my customers) and given we branch on top
of every x.0 release, that would save me a bit of pain keeping local
patches in our downstream tree. Also it's good for people who want to
try this but don't keep track of trunk (just trying to elaborate a bit
in case I ask to merge other NewGVN related patches, hope it makes
sense).


More information about the llvm-commits mailing list