[PATCH] D144608: [InstCombine] Add transforms for `(icmp {u|s}ge/le (xor X, Y), X)`

[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
----------------
goldstein.w.n wrote:
> spatel wrote:
> > This is testing the commuted xor, so that's good  - but it would be better to commute the xor operands in the original test, so we are not relying on some other transform to get to canonical form.
> > This is testing the commuted xor, so that's good  - but it would be better to commute the xor operands in the original test, so we are not relying on some other transform to get to canonical form.
> 
> Sorry not sure I understand what you mean by "in the original test" or what you want changed.
> 
> Did update this patch (along with follow / or to swap op0/op1 so `op0` is always the xor.
I posted comments directly in D144607 to hopefully make it clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144608