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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

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

> Removed the asserts.  As Danny put it:
>
> "Given LLVM defines edge dominance in a way that means non-unique edges never dominate their end, this is a waste of time."
>
> In other words, there is no need for an assert here since it's not the case
>  that the answer would be wrong or would take more time to compute than for
>  unique edges.  It's simply that the answer would always be non-dominance by
>  how domininance of edges is defined.


I'm not sure I agree with this.  To be clear, say we have:

  bb0:
    br i1 undef, label %bb1, label %bb1
  
  bb1;
    ...

are you suggesting that `dominates([bb0->bb1], bb1)` will be false anyway, and there is no specific need to check `isSingleEdge()` at all?  That does not seem to be the case -- I think `DominatorTree::dominates([bb0->bb1], bb1)` will return true.  I think you need to change the loop in `dominates` over `preds(End)` to return false if the `if (BB == Start)` condition is taken more than once.

The external property this affects is cases like

  bb0:
    br i1 %cond, label %bb1, label ...
  
  bb1;
    ...

Today if the `bb0` -> `bb1` edge dominates some use of `%cond` then said use can be replaced with `i1 true`, but with your change that will no longer hold.


https://reviews.llvm.org/D33584