[PATCH] D117110: [InstCombine] try to fold binop with phi operands

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1297
+  if (!Phi0 || !Phi1 || !Phi0->hasOneUse() || !Phi1->hasOneUse() ||
+      Phi0->getNumOperands() != 2 || Phi1->getNumOperands() != 2)
+    return nullptr;
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > Technically, this is wrong.
> > 1. for switch predecessor, we could have more than a single incoming edges 
> > 2. This check is backwards. It's not really that it must have exactly two predecessors,
> >    but more that there must be at most a single predecessor with non-constant input value,
> >    otherwise we'd increase instruction count. And for constant incoming values we'll constant-fold.
> Can you please at least add a FIXME for these issues?
Sorry - I got distracted from this patch. I agree that this should be more like foldOpIntoPhi().
But yes, I can add a FIXME to this and follow-up. We should still get the original motivating case even with this artificial limitation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117110/new/

https://reviews.llvm.org/D117110