[PATCH] D36172: [InstCombine] Improve profitability check for folding PHI args

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

In https://reviews.llvm.org/D36172#829195, @dberlin wrote:

>




> Let be very clear: I'm actually talking about *this specific transformation*, not instcombine.
> 
> The general transformations between
>  a = x op y
>  b = x op y
>  result = phi(a, b)
> and
>  1 = phi(x, y)
>  2 = phi(x, y)
>  result = 1 op 2
> 
> is exponential when applied repeatedly.

Well, yes, because that would introduce two phis where there originally was just one.  But InstCombine doesn't do that, either with or without my patch.   Before the patch, InstCombine used to transform:

a = x op c
b = y op c
result = phi(a, b)
to
tmp = phi(x, y)
result = tmp op c

Now, it will in addition also transform:

a = x op c
b = y op c
result1 = phi(a, b)
result2 = phi(a, b)
to
tmp1 = phi(x, y)
result1 = tmp1 op c
tmp2 = phi(x, y)
result2 = tmp2 op c

So with my patch, it may introduce more copies of the "op" (but still not more that we had before the transformation).  But either with or without my patch, there will never be more *phis*.  This is not because of the single-use check, but because we only move unary ops, or binary ops where the other operand is invariant, over the phi.


https://reviews.llvm.org/D36172