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

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

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

> In https://reviews.llvm.org/D36172#829179, @uweigand wrote:
>
> > The testcase is extracted from a real-word program.  On that program, the transformation (moving some of those operations out of a hot loop) is a significant overall win (about 10% improved performance of the whole program).  I agree that this application is a quite special case -- this patch doesn't make much difference to the overall performance of the platform in general.
>
>
> I'd be against doing it for this reason, but i'll leave it to others.
>
> > Agreed that the overall InstCombine algorithm can exhibit performance issues, and should probably be replaced in the long term.  But I don't think this particular patch makes those issues significantly worse :-)
>
> 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.


and NewGVN is only able to avoid such behavior by

1. memoizing the results properly, since it's a value numberer
2. Only attempting the transform when it sees something that could possibly be redundant as a result.

(but even then, the transform is still polynomial rather than linear)

Neither of these limitations apply here.
Given the right program, i believe you will happily go off into exponential land :)


https://reviews.llvm.org/D36172