[PATCH] D44626: [InstCombine] Fold (A OR B) AND B code sequence over Phi node

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D44626#1082044, @inouehrs wrote:

> In https://reviews.llvm.org/D44626#1081987, @spatel wrote:
>
> > I think what this patch really wants to ask/do is: "Does this binop simplify with the incoming value of the phi to 1 of the binop operands? If yes, substitute that value into the phi."
>
>
> Note that this patch intend to optimize only a simple but important case on `std::pair` to help jump threading. Other optimizers are already able to do more generic optimization for this type of code sequence, but it's too late to help jump threading.


I understand that we get many other similar cases (for better or worse, here in instcombine). But if we're going to try this, then we might as well generalize it, so it triggers more often and leads to less lumpy optimization. Ie, you should get this case too:

  define i64 @phi_with_idempotent_binop(i1 %f, i64 %a) {
  entry:
    br i1 %f, label %BB1, label %BB2
  
  BB1:                               
    %and = and i64 %a, 1
    br label %BB2
  
  BB2:                                 
    %phi = phi i64 [ %a, %entry ], [ %and, %BB1 ]
    %or = or i64 %phi, 1
    ret i64 %or
  }

If you use a more general matcher (and I think it will be cheaper for the tests you're showing), we can get the motivating cases you've seen and the ones you haven't seen yet. :)

I think the matcher is quite simple. It would look something like this:

  if (BinOp.isIdempotent()) { // handle both 'and' and 'or'
    Value *V = SimplifyBinOp(BinOp->getOpcode(), Phi->getIncomingVal(Idx), BinOp->getOperand(1)) // no limits on what's on the other side of the phi
    if (V && V == BinOp->getOperand(1))
      Phi->setIncomingValue(Idx, V);
  }  

(add the appropriate checks for uses, commutes, etc)


https://reviews.llvm.org/D44626