[llvm] 23bd33c - [InstCombine] prefer xor with -1 because 'not' is easier to understand (PR32706)

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2020-08-12T15:50:33-04:00
New Revision: 23bd33c6acc4fa0ddc097d3d0767860cc014f6e0

URL: https://github.com/llvm/llvm-project/commit/23bd33c6acc4fa0ddc097d3d0767860cc014f6e0
DIFF: https://github.com/llvm/llvm-project/commit/23bd33c6acc4fa0ddc097d3d0767860cc014f6e0.diff

LOG: [InstCombine] prefer xor with -1 because 'not' is easier to understand (PR32706)

This is a retry of rL300977 which was reverted because of infinite loops.
We have fixed all of the known places where that would happen, but there's
still a chance that this patch will cause infinite loops.

This matches the demanded bits behavior in the DAG and should fix:
https://bugs.llvm.org/show_bug.cgi?id=32706

Differential Revision: https://reviews.llvm.org/D32255

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
    llvm/test/Transforms/InstCombine/and-xor-or.ll
    llvm/test/Transforms/InstCombine/or-xor.ll
    llvm/test/Transforms/InstCombine/xor.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index f56d561e62d7..72be6059b1da 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -256,10 +256,20 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
       return InsertNewInstWith(And, *I);
     }
 
-    // If the RHS is a constant, see if we can simplify it.
-    // FIXME: for XOR, we prefer to force bits to 1 if they will make a -1.
-    if (ShrinkDemandedConstant(I, 1, DemandedMask))
-      return I;
+    // If the RHS is a constant, see if we can change it. Don't alter a -1
+    // constant because that's a canonical 'not' op, and that is better for
+    // combining, SCEV, and codegen.
+    const APInt *C;
+    if (match(I->getOperand(1), m_APInt(C)) && !C->isAllOnesValue()) {
+      if ((*C | ~DemandedMask).isAllOnesValue()) {
+        // Force bits to 1 to create a 'not' op.
+        I->setOperand(1, ConstantInt::getAllOnesValue(VTy));
+        return I;
+      }
+      // If we can't turn this into a 'not', try to shrink the constant.
+      if (ShrinkDemandedConstant(I, 1, DemandedMask))
+        return I;
+    }
 
     // If our LHS is an 'and' and if it has one use, and if any of the bits we
     // are flipping are known to be set, then the xor is just resetting those

diff  --git a/llvm/test/Transforms/InstCombine/and-xor-or.ll b/llvm/test/Transforms/InstCombine/and-xor-or.ll
index 1eb871e594cc..7df0591a8fa9 100644
--- a/llvm/test/Transforms/InstCombine/and-xor-or.ll
+++ b/llvm/test/Transforms/InstCombine/and-xor-or.ll
@@ -74,7 +74,7 @@ define <2 x i32> @and_xor_common_op_commute3(<2 x i32> %pa, <2 x i32> %pb) {
 
 define <4 x i32> @and_xor_common_op_constant(<4 x i32> %A) {
 ; CHECK-LABEL: @and_xor_common_op_constant(
-; CHECK-NEXT:    [[TMP1:%.*]] = xor <4 x i32> [[A:%.*]], <i32 7, i32 7, i32 7, i32 7>
+; CHECK-NEXT:    [[TMP1:%.*]] = xor <4 x i32> [[A:%.*]], <i32 -1, i32 -1, i32 -1, i32 -1>
 ; CHECK-NEXT:    [[TMP2:%.*]] = and <4 x i32> [[TMP1]], <i32 1, i32 2, i32 3, i32 4>
 ; CHECK-NEXT:    ret <4 x i32> [[TMP2]]
 ;
@@ -87,7 +87,7 @@ define <4 x i32> @and_xor_common_op_constant(<4 x i32> %A) {
 
 define i32 @and_xor_not_common_op(i32 %a, i32 %b) {
 ; CHECK-LABEL: @and_xor_not_common_op(
-; CHECK-NEXT:    [[T4:%.*]] = and i32 [[B:%.*]], [[A:%.*]]
+; CHECK-NEXT:    [[T4:%.*]] = and i32 [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    ret i32 [[T4]]
 ;
   %b2 = xor i32 %b, -1

diff  --git a/llvm/test/Transforms/InstCombine/or-xor.ll b/llvm/test/Transforms/InstCombine/or-xor.ll
index 6ee5999d1a36..56c5fca2e76b 100644
--- a/llvm/test/Transforms/InstCombine/or-xor.ll
+++ b/llvm/test/Transforms/InstCombine/or-xor.ll
@@ -234,8 +234,8 @@ define i32 @test16(i32 %a, i32 %b) {
 
 define i8 @not_or(i8 %x) {
 ; CHECK-LABEL: @not_or(
-; CHECK-NEXT:    [[TMP1:%.*]] = or i8 [[X:%.*]], 7
-; CHECK-NEXT:    [[OR:%.*]] = xor i8 [[TMP1]], -8
+; CHECK-NEXT:    [[NOTX:%.*]] = xor i8 [[X:%.*]], -1
+; CHECK-NEXT:    [[OR:%.*]] = or i8 [[NOTX]], 7
 ; CHECK-NEXT:    ret i8 [[OR]]
 ;
   %notx = xor i8 %x, -1
@@ -245,8 +245,8 @@ define i8 @not_or(i8 %x) {
 
 define i8 @not_or_xor(i8 %x) {
 ; CHECK-LABEL: @not_or_xor(
-; CHECK-NEXT:    [[TMP1:%.*]] = or i8 [[X:%.*]], 7
-; CHECK-NEXT:    [[XOR:%.*]] = xor i8 [[TMP1]], -12
+; CHECK-NEXT:    [[TMP1:%.*]] = and i8 [[X:%.*]], -8
+; CHECK-NEXT:    [[XOR:%.*]] = xor i8 [[TMP1]], -13
 ; CHECK-NEXT:    ret i8 [[XOR]]
 ;
   %notx = xor i8 %x, -1

diff  --git a/llvm/test/Transforms/InstCombine/xor.ll b/llvm/test/Transforms/InstCombine/xor.ll
index 0d863000944e..3038f53bbd20 100644
--- a/llvm/test/Transforms/InstCombine/xor.ll
+++ b/llvm/test/Transforms/InstCombine/xor.ll
@@ -600,10 +600,10 @@ define i8 @xor_or_not(i8 %x, i8* %p) {
 
 define i8 @xor_or_not_uses(i8 %x, i8* %p) {
 ; CHECK-LABEL: @xor_or_not_uses(
-; CHECK-NEXT:    [[TMP1:%.*]] = or i8 [[X:%.*]], 7
-; CHECK-NEXT:    [[OR:%.*]] = xor i8 [[TMP1]], -8
+; CHECK-NEXT:    [[NX:%.*]] = xor i8 [[X:%.*]], -1
+; CHECK-NEXT:    [[OR:%.*]] = or i8 [[NX]], 7
 ; CHECK-NEXT:    store i8 [[OR]], i8* [[P:%.*]], align 1
-; CHECK-NEXT:    [[R:%.*]] = xor i8 [[TMP1]], -12
+; CHECK-NEXT:    [[R:%.*]] = xor i8 [[OR]], 12
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %nx = xor i8 %x, -1
@@ -1000,7 +1000,7 @@ define i4 @or_or_xor_use2(i4 %x, i4 %y, i4 %z, i4* %p) {
 
 define i32 @not_is_canonical(i32 %x, i32 %y) {
 ; CHECK-LABEL: @not_is_canonical(
-; CHECK-NEXT:    [[SUB:%.*]] = xor i32 [[X:%.*]], 1073741823
+; CHECK-NEXT:    [[SUB:%.*]] = xor i32 [[X:%.*]], -1
 ; CHECK-NEXT:    [[ADD:%.*]] = add i32 [[SUB]], [[Y:%.*]]
 ; CHECK-NEXT:    [[MUL:%.*]] = shl i32 [[ADD]], 2
 ; CHECK-NEXT:    ret i32 [[MUL]]