[PATCH] D29729: [InstCombine] don't lose nsw/nuw from add by converting to xor

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: test/Transforms/InstCombine/add.ll:252
 ; CHECK-LABEL: @test20(
-; CHECK-NEXT:    ret i32 %x
+; CHECK-NEXT:    [[Z:%.*]] = or i32 %x, -2147483648
+; CHECK-NEXT:    ret i32 [[Z]]
----------------
sanjoy wrote:
> Can we add a simple peephole to not regress this case?  I.e.  `(xor SIGN_BIT (add nsw X SIGN_BIT))` to `X`?
> 
> (I'm fine doing that in an immediately following change and not in this change specifically).
Sure - I didn't know if combining those 2 steps into 1 patch would be welcome, but it makes sense to not introduce a known regression.


================
Comment at: test/Transforms/InstCombine/icmp-add.ll:226
 
-; FIXME: InstCombine should not lose wrapping information by changing the add to xor.
+; InstCombine should not thwart this opportunity to simplify completely.
 
----------------
sanjoy wrote:
> Why not just delete the comment?
I'd prefer to leave it here to supply the motivation. I often see tests like this that would seem to belong in InstSimplify rather than InstCombine, and I can't tell if that's intentional or just an artifact of LLVM optimization history (and therefore, the test is redundant and removeable).


https://reviews.llvm.org/D29729