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
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@361044
91177308-0d34-0410-b5e6-
96231b3b80d8
/// 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();
// 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.
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();
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
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
; 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
;
; 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
;
;
; 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
projects / llvm / commitdiff