[PATCH] D109151: [InstCombine] Convert xor (ashr X, BW-1), C -> select(X >=s 0, C, ~C)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:3436
+        *CA == X->getType()->getScalarSizeInBits() - 1 &&
+        !C1->isAllOnesValue() && !C1->isZeroValue()) {
+      Value *ICmp = Builder.CreateICmpSGE(X, ConstantInt::get(X->getType(), 0));
----------------
"xor X, 0"  - seems like that should be an assert because it means this instruction escaped from instsimplify.

"xor X, -1" - I think we should handle this too for consistency. The transform holds for any value unless I'm missing something:
https://alive2.llvm.org/ce/z/R7qEeT
(If that's right, please add a test so we know we are not or will not conflict with some other transform.)


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:3437
+        !C1->isAllOnesValue() && !C1->isZeroValue()) {
+      Value *ICmp = Builder.CreateICmpSGE(X, ConstantInt::get(X->getType(), 0));
+      return SelectInst::Create(ICmp, Op1, Builder.CreateNot(Op1));
----------------
Might as well create this as "SGT X, -1" to save a step since the "SGE" will get canonicalized to that form.


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

https://reviews.llvm.org/D109151