[llvm] r361044 - [DAGCombiner] visitShiftByConstant(): drop bogus signbit check
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Fri May 17 08:52:58 PDT 2019
Author: lebedevri
Date: Fri May 17 08:52:58 2019
New Revision: 361044
URL: http://llvm.org/viewvc/llvm-project?rev=361044&view=rev
Log:
[DAGCombiner] visitShiftByConstant(): drop bogus signbit check
Summary:
That check claims that the transform is illegal otherwise.
That isn't true:
1. For `ISD::ADD`, we only process `ISD::SHL` outer shift => sign bit does not matter
https://rise4fun.com/Alive/K4A
2. For `ISD::AND`, there is no restriction on constants:
https://rise4fun.com/Alive/Wy3
3. For `ISD::OR`, there is no restriction on constants:
https://rise4fun.com/Alive/GOH
3. For `ISD::XOR`, there is no restriction on constants:
https://rise4fun.com/Alive/ml6
So, why is it there then?
This changes the testcase that was touched by @spatel in rL347478,
but i'm not sure that test tests anything particular?
Reviewers: RKSimon, spatel, craig.topper, jojo, rengolin
Reviewed By: spatel
Subscribers: javed.absar, llvm-commits, spatel
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D61918
Modified:
llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/trunk/test/CodeGen/AArch64/pull-binop-through-shift.ll
llvm/trunk/test/CodeGen/X86/pull-binop-through-shift.ll
llvm/trunk/test/CodeGen/X86/xor.ll
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=361044&r1=361043&r2=361044&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Fri May 17 08:52:58 2019
@@ -6582,11 +6582,16 @@ SDValue DAGCombiner::visitXOR(SDNode *N)
/// Handle transforms common to the three shifts, when the shift amount is a
/// constant.
+/// We are looking for: (shift being one of shl/sra/srl)
+/// shift (binop X, C0), C1
+/// And want to transform into:
+/// binop (shift X, C1), (shift C0, C1)
SDValue DAGCombiner::visitShiftByConstant(SDNode *N, ConstantSDNode *Amt) {
// Do not turn a 'not' into a regular xor.
if (isBitwiseNot(N->getOperand(0)))
return SDValue();
+ // The inner binop must be one-use, since we want to replace it.
SDNode *LHS = N->getOperand(0).getNode();
if (!LHS->hasOneUse()) return SDValue();
@@ -6594,27 +6599,23 @@ SDValue DAGCombiner::visitShiftByConstan
// instead of (shift (and)), likewise for add, or, xor, etc. This sort of
// thing happens with address calculations, so it's important to canonicalize
// it.
- bool HighBitSet = false; // Can we transform this if the high bit is set?
-
switch (LHS->getOpcode()) {
- default: return SDValue();
+ default:
+ return SDValue();
case ISD::OR:
case ISD::XOR:
- HighBitSet = false; // We can only transform sra if the high bit is clear.
- break;
case ISD::AND:
- HighBitSet = true; // We can only transform sra if the high bit is set.
break;
case ISD::ADD:
if (N->getOpcode() != ISD::SHL)
return SDValue(); // only shl(add) not sr[al](add).
- HighBitSet = false; // We can only transform sra if the high bit is clear.
break;
}
// We require the RHS of the binop to be a constant and not opaque as well.
ConstantSDNode *BinOpCst = getAsNonOpaqueConstant(LHS->getOperand(1));
- if (!BinOpCst) return SDValue();
+ if (!BinOpCst)
+ return SDValue();
// FIXME: disable this unless the input to the binop is a shift by a constant
// or is copy/select.Enable this in other cases when figure out it's exactly profitable.
@@ -6634,16 +6635,6 @@ SDValue DAGCombiner::visitShiftByConstan
EVT VT = N->getValueType(0);
- // If this is a signed shift right, and the high bit is modified by the
- // logical operation, do not perform the transformation. The highBitSet
- // boolean indicates the value of the high bit of the constant which would
- // cause it to be modified for this operation.
- if (N->getOpcode() == ISD::SRA) {
- bool BinOpRHSSignSet = BinOpCst->getAPIntValue().isNegative();
- if (BinOpRHSSignSet != HighBitSet)
- return SDValue();
- }
-
if (!TLI.isDesirableToCommuteWithShift(N, Level))
return SDValue();
Modified: llvm/trunk/test/CodeGen/AArch64/pull-binop-through-shift.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/pull-binop-through-shift.ll?rev=361044&r1=361043&r2=361044&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/pull-binop-through-shift.ll (original)
+++ llvm/trunk/test/CodeGen/AArch64/pull-binop-through-shift.ll Fri May 17 08:52:58 2019
@@ -236,8 +236,8 @@ define i32 @and_nosignbit_ashr(i32 %x, i
define i32 @or_signbit_ashr(i32 %x, i32* %dst) {
; CHECK-LABEL: or_signbit_ashr:
; CHECK: // %bb.0:
-; CHECK-NEXT: orr w8, w0, #0xffff0000
-; CHECK-NEXT: asr w0, w8, #8
+; CHECK-NEXT: lsr w8, w0, #8
+; CHECK-NEXT: orr w0, w8, #0xffffff00
; CHECK-NEXT: str w0, [x1]
; CHECK-NEXT: ret
%t0 = or i32 %x, 4294901760 ; 0xFFFF0000
@@ -261,8 +261,8 @@ define i32 @or_nosignbit_ashr(i32 %x, i3
define i32 @xor_signbit_ashr(i32 %x, i32* %dst) {
; CHECK-LABEL: xor_signbit_ashr:
; CHECK: // %bb.0:
-; CHECK-NEXT: eor w8, w0, #0xffff0000
-; CHECK-NEXT: asr w0, w8, #8
+; CHECK-NEXT: asr w8, w0, #8
+; CHECK-NEXT: eor w0, w8, #0xffffff00
; CHECK-NEXT: str w0, [x1]
; CHECK-NEXT: ret
%t0 = xor i32 %x, 4294901760 ; 0xFFFF0000
Modified: llvm/trunk/test/CodeGen/X86/pull-binop-through-shift.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pull-binop-through-shift.ll?rev=361044&r1=361043&r2=361044&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/pull-binop-through-shift.ll (original)
+++ llvm/trunk/test/CodeGen/X86/pull-binop-through-shift.ll Fri May 17 08:52:58 2019
@@ -414,8 +414,8 @@ define i32 @or_signbit_ashr(i32 %x, i32*
; X64-LABEL: or_signbit_ashr:
; X64: # %bb.0:
; X64-NEXT: movl %edi, %eax
-; X64-NEXT: orl $-65536, %eax # imm = 0xFFFF0000
-; X64-NEXT: sarl $8, %eax
+; X64-NEXT: shrl $8, %eax
+; X64-NEXT: orl $-256, %eax
; X64-NEXT: movl %eax, (%rsi)
; X64-NEXT: retq
;
@@ -459,8 +459,8 @@ define i32 @xor_signbit_ashr(i32 %x, i32
; X64-LABEL: xor_signbit_ashr:
; X64: # %bb.0:
; X64-NEXT: movl %edi, %eax
-; X64-NEXT: xorl $-65536, %eax # imm = 0xFFFF0000
; X64-NEXT: sarl $8, %eax
+; X64-NEXT: xorl $-256, %eax
; X64-NEXT: movl %eax, (%rsi)
; X64-NEXT: retq
;
Modified: llvm/trunk/test/CodeGen/X86/xor.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/xor.ll?rev=361044&r1=361043&r2=361044&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/xor.ll (original)
+++ llvm/trunk/test/CodeGen/X86/xor.ll Fri May 17 08:52:58 2019
@@ -480,16 +480,16 @@ define %struct.ref_s* @test12(%struct.re
;
; X64-LIN-LABEL: test12:
; X64-LIN: # %bb.0:
-; X64-LIN-NEXT: notl %edx
; X64-LIN-NEXT: movslq %edx, %rax
+; X64-LIN-NEXT: notq %rax
; X64-LIN-NEXT: shlq $4, %rax
; X64-LIN-NEXT: addq %rdi, %rax
; X64-LIN-NEXT: retq
;
; X64-WIN-LABEL: test12:
; X64-WIN: # %bb.0:
-; X64-WIN-NEXT: notl %r8d
; X64-WIN-NEXT: movslq %r8d, %rax
+; X64-WIN-NEXT: notq %rax
; X64-WIN-NEXT: shlq $4, %rax
; X64-WIN-NEXT: addq %rcx, %rax
; X64-WIN-NEXT: retq
More information about the llvm-commits
mailing list