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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/urem-via-cmp-select.ll:5
+; https://alive2.llvm.org/ce/z/UNmz9j
+define noundef i64 @urem_assume(i64 noundef %x, i64 noundef %n) {
+; CHECK-LABEL: @urem_assume(
----------------
Should also drop noundef here and on the return values.


================
Comment at: llvm/test/Transforms/InstCombine/urem-via-cmp-select.ll:105
+start:
+  %cmp = icmp ult i64 %x, %n
+  %add = add i64 %x, 1
----------------
%cmp is a dead instruction here. The request wasn't to just drop the assume, but to establish the `%x < %n` precondition without using one. It's a bit tricky to do this without also triggering other folds, but maybe this would work?
```
define i64 @test(i64 %arg) {
  %x = shl nuw i64 1, %arg
  %n = shl nuw i64 3, %arg
  %add = add i64 %x, 1
  %out = urem i64 %add, %n
  ret i64 %out
}
```
Unfortunately this example still doesn't show the need for freeze (https://alive2.llvm.org/ce/z/FJCdt4) ... I'm pretty sure it's needed in the general case, but I'm not sure how to come up with an example.


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

https://reviews.llvm.org/D143883