[PATCH] D42739: [InstCombine] allow multi-use values in canEvaluate* if all uses are in 1 inst

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp:189
+    Value *LHS, *RHS;
+    if (I->getOperand(0) == I->getOperand(1)) {
+      // Don't create an unnecessary value if the operands are repeated.
----------------
spatel wrote:
> aaboud wrote:
> > Sorry, I did not notice this before.
> > But should not we do the same check also for the "Select" and "PHI" below?
> > Could that be related to the infinite loop caused by this patch?
> Yes, certainly for the select. I'm not sure about PHI though. A PHI with identical operands would be an invalid instruction?
> 'PHINode should have one entry for each predecessor of its parent basic block!'
> 
I limited the multi-use evaluation to binops with rL324252 . That should disallow select and PHI, so hopefully closes the infinite looping potential. Still trying to come up with a test case.


Repository:
  rL LLVM

https://reviews.llvm.org/D42739