[PATCH] D143766: [InstCombine][WIP] Fix InstCombinerImpl::foldICmpMulConstant for nsw and nuw mul with unsigned compare.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2082
           MulTy, APIntOps::RoundingSDiv(C, *MulC, APInt::Rounding::DOWN));
-  } else {
-    assert(Mul->hasNoUnsignedWrap() && "Expected mul nuw");
+  } else if (Mul->hasNoUnsignedWrap()) {
     if (Pred == ICmpInst::ICMP_ULT || Pred == ICmpInst::ICMP_UGE)
----------------
goldstein.w.n wrote:
> Does this need an `ICmpInst::isUnsigned(Pred)` check aswell?
I didn't add `isUnsigned` because the body of else only checks unsigned predicates and leaves NewC null if it doesn't find an unsigned predicate. NewC being null is sufficient to make the function return null.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-mul.ll:949
 ; CHECK-LABEL: @splat_mul_unknown_lz(
-; CHECK-NEXT:    [[Z:%.*]] = zext i32 [[X:%.*]] to i128
-; CHECK-NEXT:    [[M:%.*]] = mul nuw nsw i128 [[Z]], 18446744078004518913
-; CHECK-NEXT:    [[R:%.*]] = icmp ult i128 [[M]], 39614081257132168796771975168
+; CHECK-NEXT:    [[R:%.*]] = icmp sgt i32 [[X:%.*]], -1
 ; CHECK-NEXT:    ret i1 [[R]]
----------------
goldstein.w.n wrote:
> imo a few more tests (at least one with unsigned pred).
Yes I'm going to find the tests for the unsigned code and see if I can copy them and add an nsw flag to expose the bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143766