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

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
Herald added a subscriber: mcrosier.

I was looking at an improvement to InstSimplify when I noticed that this add fold in InstCombine was preventing icmp simplifies.

There's a later fold that special-cases (add (xor with signbit), C), so I don't think we're losing anything by limiting this.

'test20' and 'xor_sign_bit' in add.ll verify that the later fold still works, so there are no regressions in existing tests.


https://reviews.llvm.org/D29729

Files:
  lib/Transforms/InstCombine/InstCombineAddSub.cpp
  test/Transforms/InstCombine/add.ll
  test/Transforms/InstCombine/icmp-add.ll


Index: test/Transforms/InstCombine/icmp-add.ll
===================================================================
--- test/Transforms/InstCombine/icmp-add.ll
+++ test/Transforms/InstCombine/icmp-add.ll
@@ -106,24 +106,22 @@
   ret <2 x i1> %cmp
 }
 
-; FIXME: InstCombine should not lose wrapping information by changing the add to xor.
+; InstCombine should not lose wrapping information by changing the add to xor.
 
 define i1 @slt_zero_add_nsw_signbit(i8 %x) {
 ; CHECK-LABEL: @slt_zero_add_nsw_signbit(
-; CHECK-NEXT:    [[Z:%.*]] = icmp sgt i8 %x, -1
-; CHECK-NEXT:    ret i1 [[Z]]
+; CHECK-NEXT:    ret i1 true
 ;
   %y = add nsw i8 %x, -128
   %z = icmp slt i8 %y, 0
   ret i1 %z
 }
 
-; FIXME: InstCombine should not lose wrapping information by changing the add to xor.
+; InstCombine should not lose wrapping information by changing the add to xor.
 
 define i1 @slt_zero_add_nuw_signbit(i8 %x) {
 ; CHECK-LABEL: @slt_zero_add_nuw_signbit(
-; CHECK-NEXT:    [[Z:%.*]] = icmp sgt i8 %x, -1
-; CHECK-NEXT:    ret i1 [[Z]]
+; CHECK-NEXT:    ret i1 true
 ;
   %y = add nuw i8 %x, 128
   %z = icmp slt i8 %y, 0
Index: test/Transforms/InstCombine/add.ll
===================================================================
--- test/Transforms/InstCombine/add.ll
+++ test/Transforms/InstCombine/add.ll
@@ -265,24 +265,24 @@
   ret i32 %add
 }
 
-; Lose no-wrap info by converting to xor? %x is known non-negative
+; Don't lose no-wrap info by converting to xor. %x is known non-negative
 ; here, but not after converting to xor.
 
 define i8 @add_nsw_signbit(i8 %x) {
 ; CHECK-LABEL: @add_nsw_signbit(
-; CHECK-NEXT:    [[Y:%.*]] = xor i8 %x, -128
+; CHECK-NEXT:    [[Y:%.*]] = add nsw i8 %x, -128
 ; CHECK-NEXT:    ret i8 [[Y]]
 ;
   %y = add nsw i8 %x, -128
   ret i8 %y
 }
 
-; Lose no-wrap info by converting to xor? %x is known non-negative
+; Don't lose no-wrap info by converting to xor. %x is known non-negative
 ; (x < 128 unsigned), but not after converting to xor.
 
 define i8 @add_nuw_signbit(i8 %x) {
 ; CHECK-LABEL: @add_nuw_signbit(
-; CHECK-NEXT:    [[Y:%.*]] = xor i8 %x, -128
+; CHECK-NEXT:    [[Y:%.*]] = add nuw i8 %x, -128
 ; CHECK-NEXT:    ret i8 [[Y]]
 ;
   %y = add nuw i8 %x, 128
Index: lib/Transforms/InstCombine/InstCombineAddSub.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1044,8 +1044,8 @@
 
   const APInt *Val;
   if (match(RHS, m_APInt(Val))) {
-    // X + (signbit) --> X ^ signbit
-    if (Val->isSignBit())
+    // X + (signbit) --> X ^ signbit, but only if we're not giving up NSW/NUW.
+    if (Val->isSignBit() && !I.hasNoSignedWrap() && !I.hasNoUnsignedWrap())
       return BinaryOperator::CreateXor(LHS, RHS);
 
     // Is this add the last step in a convoluted sext?


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29729.87714.patch
Type: text/x-patch
Size: 2851 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170208/875cab46/attachment.bin>