[PATCH] D16240: [InstCombine] Simplify a known nonzero incoming value of PHI

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:933
@@ +932,3 @@
+    //      icmp eq, %p, 0
+    ICmpInst *CmpInst = dyn_cast<ICmpInst>(PHIUser);
+    // FIXME: To be simple, handle only integer type for now.
----------------
Please use `auto *` here.

================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:935
@@ +934,3 @@
+    // FIXME: To be simple, handle only integer type for now.
+    if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
+        (match(CmpInst->getOperand(1), PatternMatch::m_Zero()) ||
----------------
`CmpInst->isEquality()` will return `true` if the comparison is `ICMP_NE`.  Please add a testcase for this.

================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:936-937
@@ +935,4 @@
+    if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
+        (match(CmpInst->getOperand(1), PatternMatch::m_Zero()) ||
+         match(CmpInst->getOperand(0), PatternMatch::m_Zero()))) {
+      for (unsigned i = 0, e = PN.getNumIncomingValues(); i != e; ++i) {
----------------
Please add `using namespace llvm::PatternMatch;` to this file so that you don't need to qualify `m_Zero`.

================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:936-937
@@ +935,4 @@
+    if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
+        (match(CmpInst->getOperand(1), PatternMatch::m_Zero()) ||
+         match(CmpInst->getOperand(0), PatternMatch::m_Zero()))) {
+      for (unsigned i = 0, e = PN.getNumIncomingValues(); i != e; ++i) {
----------------
majnemer wrote:
> Please add `using namespace llvm::PatternMatch;` to this file so that you don't need to qualify `m_Zero`.
Instructions are canonicalized so that the RHS will have the constant.  Please remove the code regarding the LHS.


http://reviews.llvm.org/D16240