[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#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.

The original version you replaced limited this in a way that it would not be because of the single use restriction
I'm pretty positive yours, combined with instcombine's iteration, can be shown to be exponential worst case


https://reviews.llvm.org/D36172