[PATCH] D145846: [InstCombine] enhance icmp with sub folds

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4096
+  if (match(&I, m_c_ICmp(Pred, m_OneUse(m_Neg(m_Value(X))), m_Deferred(X))) &&
+      (Pred == ICmpInst::ICMP_NE || Pred == ICmpInst::ICMP_EQ)) {
+    Type *Ty = X->getType();
----------------
Use `ICmpInst::isEquality()`.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4098
+    Type *Ty = X->getType();
+    uint32_t BitWidth = cast<IntegerType>(Ty)->getBitWidth();
+    Constant *MaxSignedVal =
----------------
Please add a vector test, this is probably going to crash. (Can use getScalarSizeInBits instead.)


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4103
+    Constant *Zero = Constant::getNullValue(Ty);
+    return CmpInst::Create(Instruction::ICmp, I.getPredicate(), And, Zero);
+  }
----------------
`I.getPrediate()` -> `Pred`.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-sub.ll:590
   ret i1 %cmp
 }
----------------
Missing commuted test case (negation on RHS), multi-use test case and negative tests (e.g. non-equality predicate).


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

https://reviews.llvm.org/D145846