[PATCH] D147597: [InstCombine] Fold icmp(bin(X, Y) + Positive) --> icmp(bin(X, Y))

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1349
+      match(Op1, m_APInt(RHS))) {
+    if (LHS->isStrictlyPositive() && RHS->isNonNegative() && LHS->sgt(*RHS)) {
+      auto Relax = [&](ICmpInst::Predicate Pred, Value *X) {
----------------
I don't think this matches the alive2 link you have. You only verify for `RHS == 0`, but here you seem to take any positive value.

Can you update the alive2 link (or the code). Also the alive2 link seems to have spurious `nsw` flags on the `mul`. Verifies w.o the `nsw` flags so nbd.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1353
+            Pred, X, ConstantInt::getNullValue(X->getType()));
+        return replaceInstUsesWith(Cmp, NewCmp);
+      };
----------------
Can this just be `return new ICmpInst(Pred, X, ConstantInt::getNullValue(X->getType())`?


================
Comment at: llvm/test/Transforms/InstCombine/icmp.ll:4637
 ;
+
   %mul1 = mul nsw i8 %a, %b
----------------
Remove?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147597