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

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 24 08:38:49 PDT 2015

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.


More information about the llvm-commits mailing list