[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#829223, @uweigand wrote:

> 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:


That's the space issue, but the time issue is the actual evaluation .
You have definitely solved the space issue, i will not disagree there.

I was also just giving an example of the generalization of this transform.

Anyway, i've said my say, so i'm going to leave this for someone else to review and decide on :)


https://reviews.llvm.org/D36172