[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#766174, @sanjoy wrote:

> In https://reviews.llvm.org/D33584#766167, @dberlin wrote:
>
> > > 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.
> >
> > Today, any caller that asks if edge bb0->bb1 dominates some use of cond, it will assert :)
>
>
> I see what you mean -- since we're only changing behavior in the case we'd have failed an assert before (i.e. before this change), the behavior change is correct by definition.
>
> > Now, you are right that there are situation we *could* return true, but we wouldn't :)
>
> We wouldn't return true today (i.e. without this patch), but we would return true once this patch is applied.  I was trying to argue that it makes more sense to return `false` for non-unique edges, since it preserves the "if the `bb0` -> `bb1` edge dominates some use of `%cond` then said use can be replaced with `i1 true`" reasoning.  The reasoning holds today on all of the cases where the antecedent, "if the `bb0` -> `bb1` edge dominates some use of `%cond`", is valid (i.e. does not assert).  With this change, we will make the antecedent valid in some cases where that implication won't hold, which is what I'm suggesting we avoid.


Oh yeah, i think we are all in violent agreement about the latter. We probably need to turn it back into a check that returns false, one way or the other.


https://reviews.llvm.org/D33584