[PATCH] D44147: [InstCombine] Fold add/sub over phi node

[Daniel Berlin via llvm-commits]

Yeah, this is a generally tricky case to get.
It requires a value based PRE that is going to do a phi of ops transform,
because this testcase is really:

if (b) phival1 = v + 1
else phival2 = callee(v)
phi = (phival1, phival2)
return phi -1

So it has to understand this is equivalent to:

if (b) phival1 = v+ 1, tmpval1 = v
else phival2 = callee(v), tmpval2 = 1 + callee
phi = (phival1, phival2)
return phi-1

Then it can see that v is already available, so tmpval1 is available, so
this is PREable.

None of our PRE's do.
NewGVN does detect this,  but it's not a full redundancy, and full
NewGVN-PRE is not done.
IE you will get:
Processing instruction   %10 = sub nsw i64 %.0, 1
Simplified   <badref> = sub nsw i64 %6, 1 to  variable i64 %0
Found phi of ops operand i64 %0 in %5
Cannot find phi of ops operand for   <badref> = sub nsw i64 %8, 1 in block
%7

You could trivially handle these kinds of case though by making
makePossiblePHIOfOps do PRE insertion depending on safety and availability
in preds. It computes availability already, and knows this is a PRE case.
It computes some parts of safety but not the parts you'd need to insert
memory instructions.
It also  would not be "lifetime optimal" PRE.


It would subsume what we do for GVN's scalar PRE, but it would also require
multiple iterations of NewGVN (just as we require multiple iterations of
GVN's PRE) to get all cases instead of being able to do it in one step.


If you change it to a full redundancy, you'll see it will already perform
the no-cost transform:

c = v+2
printf("%d\n", c); // make c used
if (b) phival1 = v + 1
else phival2 =v + 3
phi = (phival1, phival2)
return phi -1
->
c = v+2
printf("%d\n", c); // make c used
if (b) phival1 = v + 1
else phival2 =v + 3
phi = (phival1, phival2)
phiofops = (v, c)
return phiofops


(I didn't add global reassociation, so how far you get here depends on how
good a just InstSimplify does at reassociating for you. It's actually very
trivial to add though if we wanted)


Bryant Wong is working on a NewGVN PRE that could perform this in a
lifetime optimal way if you add the phi of ops transform we do in the
appropriate places.





On Wed, Mar 7, 2018 at 1:32 AM, Hiroshi Inoue via Phabricator <
reviews at reviews.llvm.org> wrote:

> inouehrs added a comment.
>
> Thanks for the comments!
> After disabling optimization for loop induction variables, optimization
> still happens more than 10k during the bootstrap. However, I cannot see
> visible change in code size and performance with benchmarks.
> So, I will revisit this when I find a realistic case for which it matters.
>
> As a simplified toy example, the following C code results in obviously
> redundant generated code:
>
>   unsigned long func(unsigned long v, bool b) {
>     unsigned long rc;
>     if (b) rc = v + 1;
>     else rc = callee(v);
>     return rc - 1;
>   }
>
> generates a code sequence like
>
>         addi 3, 3, 1
>         addi 3, 3, -1
>         blr
>
>
>
>
> https://reviews.llvm.org/D44147
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180307/f15254b2/attachment.html>