From 23bd33c6acc4fa0ddc097d3d0767860cc014f6e0 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Wed, 12 Aug 2020 15:21:17 -0400 Subject: [PATCH] [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 --- .../InstCombine/InstCombineSimplifyDemanded.cpp | 18 ++++++++++++++---- llvm/test/Transforms/InstCombine/and-xor-or.ll | 4 ++-- llvm/test/Transforms/InstCombine/or-xor.ll | 8 ++++---- llvm/test/Transforms/InstCombine/xor.ll | 8 ++++---- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp index f56d561..72be605 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 1eb871e..7df0591 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:%.*]], +; CHECK-NEXT: [[TMP1:%.*]] = xor <4 x i32> [[A:%.*]], ; CHECK-NEXT: [[TMP2:%.*]] = and <4 x i32> [[TMP1]], ; 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 6ee5999..56c5fca 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 0d86300..3038f53 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]] -- 2.7.4