[PATCH] D33584: Remove a quadratic behavior in assert-enabled builds

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

In https://reviews.llvm.org/D33584#765895, @anemet wrote:

> In https://reviews.llvm.org/D33584#765763, @dberlin wrote:
>
> > As I said last year, i believe, we should just remove this assert.
> >  It doesn't help anything. The callers literally  can't  handle it any better if they want real dominance answers.
>
>
> I thought GVN was a good counter example which bails early in the presence of duplicated edges: https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Scalar/GVN.cpp#L1733


This is not actually any more efficient than dominance would have answered it.
Both have to do precisely the same thing:
Look at the predecessors and see if any are the same :)
That has the same time bound no matter how you do it.
At least how LLVM has implemented "edge dominance", an edge does not dominate a block unless (basically) start dominates end and start, end is unique.
(i'm ignoring critical edges here for a moment).

The uniqueness test takes the same time no matter whether the caller does it or dominates does it.

>> There isn't anywhere else in llvm we assert because "a thing may become quadratic if you do dumb things", and the assert itself is quadratic.
> 
> Is it the problem that dominates is quadratic or that it returns the wrong answer in the presence of duplicated edges?

I believe it is now broken, but it was not before.
Before, it was only quadratic over multiple calls
IE i repeatedly query the same set.
Most callers only make one call.

It has always been quadratic in the critical edge case.

It is possible to make both edge dominance and critical edge dominance constant time by a variety of methods if we wanted.
A trivial one for edge dominance:

1. Have the dom tree maintain a set of non-singular edges that it builds at construction time.

Return false if it's in the set.

another not so trivial:

2. convert the multiple edges into the equivalent block form using virtual basic blocks.

Given LLVM defines edge dominance in a way that means non-unique edges never dominate their end, this is a waste of time.

For critical edges, there are also a number of ways by adding virtual links or blocks or ... to the dom tree.

Also note:
GVN deliberately does *not* compute dominance answers for the other single edge case.
(see propagateequality and isOnlyReachableViaThisEdge).

FWIW: Using edge dominance for all of this  is also, IMHO, not a great idea, but i'm ignoring that.


https://reviews.llvm.org/D33584