[PATCH] D143883: [InstCombine] canonicalize urem as cmp+select

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1793-1794
+        simplifyICmpInst(ICmpInst::ICMP_ULT, X, Op1, SQ.getWithInstruction(&I));
+    if (auto *CVal = dyn_cast_or_null<Constant>(Val))
+      if (CVal->isOneValue()) {
+        Value *FrozenX = Builder.CreateFreeze(Op0, Op0->getName() + ".frozen");
----------------
Use `match(Val, m_One())` instead?


================
Comment at: llvm/test/Transforms/InstCombine/urem-via-cmp-select.ll:24
+; https://alive2.llvm.org/ce/z/uo7HMz
+define noundef i64 @urem_assume_without_nuw(i64 noundef %x, i64 noundef %n) {
+; CHECK-LABEL: @urem_assume_without_nuw(
----------------
Drop noundef attributes


================
Comment at: llvm/test/Transforms/InstCombine/urem-via-cmp-select.ll:77
+
+declare void @llvm.assume(i1 noundef)
----------------
Missing tests where add constant is not 1.

It would be good to also have a test that is not based on assume, where `%x < %n` is known through some other way instead.


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

https://reviews.llvm.org/D143883