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

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 26 14:13:03 PDT 2017


dberlin added a comment.

Note: i haven't thought out whether  the pred test it does in most places will actually give the right answer.
At one point, it tested whether the edge was unique and returned false, this got turned into the current assert.
You may have to add that back.

If that is too slow, we could add the non-singular edge set and invalidate/recompute it.  It should suffice to invalidate it anywhere we invalidate the dfs numbers.
Then, like dominates calls updateDFSNumber if they are invalid and it hits 32 queries, these function could do something similar and call updateNonSingleEdges if they are queried too much.

It's worth noting that the (edge, phi use) case is theoretically wrong after your xhange, right now, but it may not matter.

The above code will now claim that dominates(edge, phi use) is "true" for *any* use from the same block, when there are multiple incoming edges to the phi.
that *would* definitely be wrong if we allowed something like
bb1:
switch x {
case 1 : goto bb2;
case 2 : goto bb2;
}
bb2:
phi([1, bb1], [2, bb1]).

Because right now it will claim the second edge dominates the first use, etc.
However, we only allow multiple same-block edges to a phi if the values are the same:
bb1:
switch x {
case 1 : goto bb2;
case 2 : goto bb2;
}
bb2:
phi([x, bb1], [x, bb1]).

So i'm not sure returning true will break anything.
It may :)


https://reviews.llvm.org/D33584





More information about the llvm-commits mailing list