[PATCH] D43276: [InstCombine] Don't fold select(C, Z, binop(select(C, X, Y), W)) -> select(C, Z, binop(Y, W)) if the binop is rem or div.

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

This is conservatively correct, so LGTM.

We could do better by using isSafeToSpeculativelyExecute(), but I think you'd have to use that after you've setOperand() and then switch it back if that fails. Or duplicate that logic with the proposed canMergeSelectThroughBinop().

The difference would show up in an example like this where we have a constant divisor, so we know there's no div-by-zero possibility.

define i32 @foo(i1 %a, i32 %b, i32 %c) {

  %sel1 = select i1 %a, i32 42, i32 %b
  %div = udiv i32 %c, %sel1
  %sel2 = select i1 %a, i32 %div, i32 0
  ret i32 %sel2

}



================
Comment at: test/Transforms/InstCombine/pr36362.ll:9
+; We shouldn't remove the select before the srem
+define i32 @foo() {
+; CHECK-LABEL: @foo(
----------------
Can minimize this to something like:


```
define i32 @foo(i1 %a, i32 %b, i32 %c) {
  %sel1 = select i1 %a, i32 %b, i32 -1
  %rem = srem i32 %c, %sel1
  %sel2 = select i1 %a, i32 %rem, i32 0
  ret i32 %sel2
}

```


Repository:
  rL LLVM

https://reviews.llvm.org/D43276