[PATCH] D12866: [InstCombine] fold zexts and constants into a phi (PR24766)

[Sanjay Patel via llvm-commits]

spatel marked 2 inline comments as done.

================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:437
@@ +436,3 @@
+  // infinite loop without this check.
+  if (NumConsts == 0 || NumZexts < 2)
+    return nullptr;
----------------
hfinkel wrote:
> Do we already have a regression test that covers these cases?
> 
There are other tests that cover the basics here: 
a. test6 ("Suck casts into phi")
b. test13 has a case with a constant in a phi

...but I think we should have tests for phis that include more than 2 operands, so I've added those to show that the existing functions are working as expected on the constructs that this new code doesn't cover.

================
Comment at: test/Transforms/InstCombine/phi.ll:662
@@ +661,3 @@
+; CHECK-LABEL: @PR24766(
+; CHECK: phi i1
+}
----------------
hfinkel wrote:
> Could you please actually check that the PHI itself is formed correctly, and add a test case that covers the handling of the truncated-constant incoming value?
> 
Yes - that was just lazy.
Let me know if the new test cases with 0/1 -> false/true cover the constant scenario you're thinking of.


http://reviews.llvm.org/D12866