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

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 26 15:09:34 PDT 2017


sanjoy added a comment.

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.


https://reviews.llvm.org/D33584





More information about the llvm-commits mailing list