[PATCH] D144607: [InstCombine] Add tests for transforming `(icmp (xor X, Y), X)`; NFC

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll:41
+  %y = or <2 x i8> %yy, <i8 9, i8 8>
+  %xor = xor <2 x i8> %x, %y
+  %r = icmp ule <2 x i8> %xor, %x
----------------
Commute x and y on this instruction, so the test is not relying on another transform to get to canonical form.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll:71
+  %y = or i8 %yy, 128
+  %xor = xor i8 %x, %y
+  %r = icmp sge i8 %xor, %x
----------------
Commute x and y on this instruction, so the test is not relying on another transform to get to canonical form.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-of-xor-x.ll:72
+  %xor = xor i8 %x, %y
+  %r = icmp sge i8 %xor, %x
+  ret i1 %r
----------------
This test is covering the same pattern (although with different types and predicate) as "xor_ule_2".
We're missing a test to provide coverage for a pattern like this:
x <= (y ^ x)

We can add an instruction to make sure the operands don't get commuted, and the test provides the coverage we want:

```

define i1 @xor_sge(i8 %xx, i8 %yy) {
  %x = mul i8 %xx, %xx      ; thwart complexity-based canonicalization
  %y = or i8 %yy, 128
  %xor = xor i8 %y, %x      ; both ops are binops, so they won't commute
  %r = icmp sge i8 %x, %xor ; both ops are binops, so they won't commute
  ret i1 %r
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144607