Skip to content
This repository has been archived by the owner on Apr 23, 2020. It is now read-only.

Commit

Permalink
[SelectionDAG] fix bug in translating funnel shift with non-power-of-…
Browse files Browse the repository at this point in the history 
…2 type

The bug is visible in the constant-folded x86 tests. We can't use the
negated shift amount when the type is not power-of-2:
https://rise4fun.com/Alive/US1r

...so in that case, use the regular lowering that includes a select
to guard against a shift-by-bitwidth. This path is improved by only
calculating the modulo shift amount once now.

Also, improve the rotate (with power-of-2 size) lowering to use
a negate rather than subtract from bitwidth. This improves the
codegen whether we have a rotate instruction or not (although
we can still see that we're not matching to a legal rotate in
all cases).



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@338592 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@rotateright
rotateright committed Aug 1, 2018
1 parent 887eb8d commit 8fe02fa
Show file tree
Hide file tree
Showing 7 changed files with 217 additions and 312 deletions.
70 changes: 39 additions & 31 deletions lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5693,43 +5693,51 @@ SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, unsigned Intrinsic) {
SDValue Y = getValue(I.getArgOperand(1));
SDValue Z = getValue(I.getArgOperand(2));
EVT VT = X.getValueType();
SDValue BitWidthC = DAG.getConstant(VT.getScalarSizeInBits(), sdl, VT);
SDValue Zero = DAG.getConstant(0, sdl, VT);
SDValue ShAmt = DAG.getNode(ISD::UREM, sdl, VT, Z, BitWidthC);

// When X == Y, this is rotate. Create the node directly if legal.
// TODO: This should also be done if the operation is custom, but we have
// to make sure targets are handling the modulo shift amount as expected.
// TODO: If the rotate direction (left or right) corresponding to the shift
// is not available, adjust the shift value and invert the direction.
auto RotateOpcode = IsFSHL ? ISD::ROTL : ISD::ROTR;
if (X == Y && TLI.isOperationLegal(RotateOpcode, VT)) {
setValue(&I, DAG.getNode(RotateOpcode, sdl, VT, X, Z));
// When X == Y, this is rotate. If the data type has a power-of-2 size, we
// avoid the select that is necessary in the general case to filter out
// the 0-shift possibility that leads to UB.
if (X == Y && isPowerOf2_32(VT.getScalarSizeInBits())) {
// TODO: This should also be done if the operation is custom, but we have
// to make sure targets are handling the modulo shift amount as expected.
// TODO: If the rotate direction (left or right) corresponding to the
// shift is not available, adjust the shift value and invert the
// direction.
auto RotateOpcode = IsFSHL ? ISD::ROTL : ISD::ROTR;
if (TLI.isOperationLegal(RotateOpcode, VT)) {
setValue(&I, DAG.getNode(RotateOpcode, sdl, VT, X, Z));
return nullptr;
}
// fshl (rotl): (X << (Z % BW)) | (X >> ((0 - Z) % BW))
// fshr (rotr): (X << ((0 - Z) % BW)) | (X >> (Z % BW))
SDValue NegZ = DAG.getNode(ISD::SUB, sdl, VT, Zero, Z);
SDValue NShAmt = DAG.getNode(ISD::UREM, sdl, VT, NegZ, BitWidthC);
SDValue ShX = DAG.getNode(ISD::SHL, sdl, VT, X, IsFSHL ? ShAmt : NShAmt);
SDValue ShY = DAG.getNode(ISD::SRL, sdl, VT, X, IsFSHL ? NShAmt : ShAmt);
setValue(&I, DAG.getNode(ISD::OR, sdl, VT, ShX, ShY));
return nullptr;
}

// Get the shift amount and inverse shift amount, modulo the bit-width.
SDValue BitWidthC = DAG.getConstant(VT.getScalarSizeInBits(), sdl, VT);
SDValue ShAmt = DAG.getNode(ISD::UREM, sdl, VT, Z, BitWidthC);
SDValue NegZ = DAG.getNode(ISD::SUB, sdl, VT, BitWidthC, Z);
SDValue InvShAmt = DAG.getNode(ISD::UREM, sdl, VT, NegZ, BitWidthC);

// fshl: (X << (Z % BW)) | (Y >> ((BW - Z) % BW))
// fshr: (X << ((BW - Z) % BW)) | (Y >> (Z % BW))
// fshl: (X << (Z % BW)) | (Y >> (BW - (Z % BW)))
// fshr: (X << (BW - (Z % BW))) | (Y >> (Z % BW))
SDValue InvShAmt = DAG.getNode(ISD::SUB, sdl, VT, BitWidthC, ShAmt);
SDValue ShX = DAG.getNode(ISD::SHL, sdl, VT, X, IsFSHL ? ShAmt : InvShAmt);
SDValue ShY = DAG.getNode(ISD::SRL, sdl, VT, Y, IsFSHL ? InvShAmt : ShAmt);
SDValue Res = DAG.getNode(ISD::OR, sdl, VT, ShX, ShY);

// If (Z % BW == 0), then (BW - Z) % BW is also zero, so the result would
// be X | Y. If X == Y (rotate), that's fine. If not, we have to select.
if (X != Y) {
SDValue Zero = DAG.getConstant(0, sdl, VT);
EVT CCVT = MVT::i1;
if (VT.isVector())
CCVT = EVT::getVectorVT(*Context, CCVT, VT.getVectorNumElements());
// For fshl, 0 shift returns the 1st arg (X).
// For fshr, 0 shift returns the 2nd arg (Y).
SDValue IsZeroShift = DAG.getSetCC(sdl, CCVT, ShAmt, Zero, ISD::SETEQ);
Res = DAG.getSelect(sdl, VT, IsZeroShift, IsFSHL ? X : Y, Res);
}
setValue(&I, Res);
SDValue Or = DAG.getNode(ISD::OR, sdl, VT, ShX, ShY);

// If (Z % BW == 0), then the opposite direction shift is shift-by-bitwidth,
// and that is undefined. We must compare and select to avoid UB.
EVT CCVT = MVT::i1;
if (VT.isVector())
CCVT = EVT::getVectorVT(*Context, CCVT, VT.getVectorNumElements());

// For fshl, 0-shift returns the 1st arg (X).
// For fshr, 0-shift returns the 2nd arg (Y).
SDValue IsZeroShift = DAG.getSetCC(sdl, CCVT, ShAmt, Zero, ISD::SETEQ);
setValue(&I, DAG.getSelect(sdl, VT, IsZeroShift, IsFSHL ? X : Y, Or));
return nullptr;
}
case Intrinsic::stacksave: {
Expand Down
40 changes: 17 additions & 23 deletions test/CodeGen/AArch64/funnel-shift-rot.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ define i64 @rotl_i64_const_shift(i64 %x) {
define i16 @rotl_i16(i16 %x, i16 %z) {
; CHECK-LABEL: rotl_i16:
; CHECK: // %bb.0:
; CHECK-NEXT: orr w10, wzr, #0x10
; CHECK-NEXT: sub w10, w10, w1
; CHECK-NEXT: neg w10, w1
; CHECK-NEXT: and w8, w0, #0xffff
; CHECK-NEXT: and w9, w1, #0xf
; CHECK-NEXT: and w10, w10, #0xf
Expand All @@ -56,8 +55,7 @@ define i16 @rotl_i16(i16 %x, i16 %z) {
define i32 @rotl_i32(i32 %x, i32 %z) {
; CHECK-LABEL: rotl_i32:
; CHECK: // %bb.0:
; CHECK-NEXT: orr w8, wzr, #0x20
; CHECK-NEXT: sub w8, w8, w1
; CHECK-NEXT: neg w8, w1
; CHECK-NEXT: ror w0, w0, w8
; CHECK-NEXT: ret
%f = call i32 @llvm.fshl.i32(i32 %x, i32 %x, i32 %z)
Expand All @@ -67,8 +65,7 @@ define i32 @rotl_i32(i32 %x, i32 %z) {
define i64 @rotl_i64(i64 %x, i64 %z) {
; CHECK-LABEL: rotl_i64:
; CHECK: // %bb.0:
; CHECK-NEXT: orr w9, wzr, #0x40
; CHECK-NEXT: sub w9, w9, w1
; CHECK-NEXT: neg w9, w1
; CHECK-NEXT: lsl x8, x0, x1
; CHECK-NEXT: lsr x9, x0, x9
; CHECK-NEXT: orr x0, x8, x9
Expand All @@ -83,14 +80,13 @@ define <4 x i32> @rotl_v4i32(<4 x i32> %x, <4 x i32> %z) {
; CHECK-LABEL: rotl_v4i32:
; CHECK: // %bb.0:
; CHECK-NEXT: movi v2.4s, #31
; CHECK-NEXT: movi v3.4s, #32
; CHECK-NEXT: and v4.16b, v1.16b, v2.16b
; CHECK-NEXT: sub v1.4s, v3.4s, v1.4s
; CHECK-NEXT: neg v3.4s, v1.4s
; CHECK-NEXT: and v1.16b, v1.16b, v2.16b
; CHECK-NEXT: neg v1.4s, v1.4s
; CHECK-NEXT: ushl v3.4s, v0.4s, v4.4s
; CHECK-NEXT: ushl v0.4s, v0.4s, v1.4s
; CHECK-NEXT: orr v0.16b, v3.16b, v0.16b
; CHECK-NEXT: and v2.16b, v3.16b, v2.16b
; CHECK-NEXT: neg v2.4s, v2.4s
; CHECK-NEXT: ushl v1.4s, v0.4s, v1.4s
; CHECK-NEXT: ushl v0.4s, v0.4s, v2.4s
; CHECK-NEXT: orr v0.16b, v1.16b, v0.16b
; CHECK-NEXT: ret
%f = call <4 x i32> @llvm.fshl.v4i32(<4 x i32> %x, <4 x i32> %x, <4 x i32> %z)
ret <4 x i32> %f
Expand Down Expand Up @@ -140,10 +136,9 @@ define i16 @rotr_i16(i16 %x, i16 %z) {
; CHECK: // %bb.0:
; CHECK-NEXT: and w8, w0, #0xffff
; CHECK-NEXT: and w9, w1, #0xf
; CHECK-NEXT: orr w10, wzr, #0x10
; CHECK-NEXT: neg w10, w1
; CHECK-NEXT: lsr w8, w8, w9
; CHECK-NEXT: sub w9, w10, w1
; CHECK-NEXT: and w9, w9, #0xf
; CHECK-NEXT: and w9, w10, #0xf
; CHECK-NEXT: lsl w9, w0, w9
; CHECK-NEXT: orr w0, w9, w8
; CHECK-NEXT: ret
Expand Down Expand Up @@ -175,14 +170,13 @@ define <4 x i32> @rotr_v4i32(<4 x i32> %x, <4 x i32> %z) {
; CHECK-LABEL: rotr_v4i32:
; CHECK: // %bb.0:
; CHECK-NEXT: movi v2.4s, #31
; CHECK-NEXT: movi v3.4s, #32
; CHECK-NEXT: and v4.16b, v1.16b, v2.16b
; CHECK-NEXT: sub v1.4s, v3.4s, v1.4s
; CHECK-NEXT: neg v3.4s, v4.4s
; CHECK-NEXT: neg v3.4s, v1.4s
; CHECK-NEXT: and v1.16b, v1.16b, v2.16b
; CHECK-NEXT: ushl v2.4s, v0.4s, v3.4s
; CHECK-NEXT: ushl v0.4s, v0.4s, v1.4s
; CHECK-NEXT: orr v0.16b, v0.16b, v2.16b
; CHECK-NEXT: and v2.16b, v3.16b, v2.16b
; CHECK-NEXT: neg v1.4s, v1.4s
; CHECK-NEXT: ushl v1.4s, v0.4s, v1.4s
; CHECK-NEXT: ushl v0.4s, v0.4s, v2.4s
; CHECK-NEXT: orr v0.16b, v0.16b, v1.16b
; CHECK-NEXT: ret
%f = call <4 x i32> @llvm.fshr.v4i32(<4 x i32> %x, <4 x i32> %x, <4 x i32> %z)
ret <4 x i32> %f
Expand Down
66 changes: 29 additions & 37 deletions test/CodeGen/AArch64/funnel-shift.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ declare <4 x i32> @llvm.fshr.v4i32(<4 x i32>, <4 x i32>, <4 x i32>)
define i32 @fshl_i32(i32 %x, i32 %y, i32 %z) {
; CHECK-LABEL: fshl_i32:
; CHECK: // %bb.0:
; CHECK-NEXT: orr w9, wzr, #0x20
; CHECK-NEXT: sub w9, w9, w2
; CHECK-NEXT: and w9, w2, #0x1f
; CHECK-NEXT: neg w9, w9
; CHECK-NEXT: lsl w8, w0, w2
; CHECK-NEXT: lsr w9, w1, w9
; CHECK-NEXT: orr w8, w8, w9
Expand All @@ -35,26 +35,22 @@ declare i37 @llvm.fshl.i37(i37, i37, i37)
define i37 @fshl_i37(i37 %x, i37 %y, i37 %z) {
; CHECK-LABEL: fshl_i37:
; CHECK: // %bb.0:
; CHECK-NEXT: mov x11, #31883
; CHECK-NEXT: mov w10, #37
; CHECK-NEXT: movk x11, #3542, lsl #16
; CHECK-NEXT: movk x11, #51366, lsl #32
; CHECK-NEXT: sub x12, x10, x2
; CHECK-NEXT: and x8, x2, #0x1fffffffff
; CHECK-NEXT: movk x11, #56679, lsl #48
; CHECK-NEXT: and x12, x12, #0x1fffffffff
; CHECK-NEXT: umulh x13, x8, x11
; CHECK-NEXT: umulh x11, x12, x11
; CHECK-NEXT: lsr x13, x13, #5
; CHECK-NEXT: lsr x11, x11, #5
; CHECK-NEXT: and x9, x1, #0x1fffffffff
; CHECK-NEXT: msub x8, x13, x10, x8
; CHECK-NEXT: msub x10, x11, x10, x12
; CHECK-NEXT: lsl x13, x0, x8
; CHECK-NEXT: lsr x9, x9, x10
; CHECK-NEXT: orr x9, x13, x9
; CHECK-NEXT: cmp x8, #0 // =0
; CHECK-NEXT: csel x0, x0, x9, eq
; CHECK-NEXT: mov x10, #31883
; CHECK-NEXT: movk x10, #3542, lsl #16
; CHECK-NEXT: movk x10, #51366, lsl #32
; CHECK-NEXT: and x9, x2, #0x1fffffffff
; CHECK-NEXT: movk x10, #56679, lsl #48
; CHECK-NEXT: umulh x10, x9, x10
; CHECK-NEXT: mov w11, #37
; CHECK-NEXT: lsr x10, x10, #5
; CHECK-NEXT: msub x9, x10, x11, x9
; CHECK-NEXT: and x8, x1, #0x1fffffffff
; CHECK-NEXT: sub x11, x11, x9
; CHECK-NEXT: lsl x10, x0, x9
; CHECK-NEXT: lsr x8, x8, x11
; CHECK-NEXT: orr x8, x10, x8
; CHECK-NEXT: cmp x9, #0 // =0
; CHECK-NEXT: csel x0, x0, x8, eq
; CHECK-NEXT: ret
%f = call i37 @llvm.fshl.i37(i37 %x, i37 %y, i37 %z)
ret i37 %f
Expand Down Expand Up @@ -150,8 +146,8 @@ define i8 @fshl_i8_const_fold() {
define i32 @fshr_i32(i32 %x, i32 %y, i32 %z) {
; CHECK-LABEL: fshr_i32:
; CHECK: // %bb.0:
; CHECK-NEXT: orr w9, wzr, #0x20
; CHECK-NEXT: sub w9, w9, w2
; CHECK-NEXT: and w9, w2, #0x1f
; CHECK-NEXT: neg w9, w9
; CHECK-NEXT: lsr w8, w1, w2
; CHECK-NEXT: lsl w9, w0, w9
; CHECK-NEXT: orr w8, w9, w8
Expand All @@ -167,21 +163,17 @@ declare i37 @llvm.fshr.i37(i37, i37, i37)
define i37 @fshr_i37(i37 %x, i37 %y, i37 %z) {
; CHECK-LABEL: fshr_i37:
; CHECK: // %bb.0:
; CHECK-NEXT: mov x11, #31883
; CHECK-NEXT: mov w10, #37
; CHECK-NEXT: movk x11, #3542, lsl #16
; CHECK-NEXT: movk x11, #51366, lsl #32
; CHECK-NEXT: sub x12, x10, x2
; CHECK-NEXT: mov x10, #31883
; CHECK-NEXT: movk x10, #3542, lsl #16
; CHECK-NEXT: movk x10, #51366, lsl #32
; CHECK-NEXT: and x9, x2, #0x1fffffffff
; CHECK-NEXT: movk x11, #56679, lsl #48
; CHECK-NEXT: and x12, x12, #0x1fffffffff
; CHECK-NEXT: umulh x13, x9, x11
; CHECK-NEXT: umulh x11, x12, x11
; CHECK-NEXT: lsr x13, x13, #5
; CHECK-NEXT: lsr x11, x11, #5
; CHECK-NEXT: movk x10, #56679, lsl #48
; CHECK-NEXT: umulh x10, x9, x10
; CHECK-NEXT: mov w11, #37
; CHECK-NEXT: lsr x10, x10, #5
; CHECK-NEXT: msub x9, x10, x11, x9
; CHECK-NEXT: and x8, x1, #0x1fffffffff
; CHECK-NEXT: msub x9, x13, x10, x9
; CHECK-NEXT: msub x10, x11, x10, x12
; CHECK-NEXT: sub x10, x11, x9
; CHECK-NEXT: lsr x8, x8, x9
; CHECK-NEXT: lsl x10, x0, x10
; CHECK-NEXT: orr x8, x10, x8
Expand Down
32 changes: 14 additions & 18 deletions test/CodeGen/PowerPC/funnel-shift-rot.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ define i64 @rotl_i64_const_shift(i64 %x) {
define i16 @rotl_i16(i16 %x, i16 %z) {
; CHECK-LABEL: rotl_i16:
; CHECK: # %bb.0:
; CHECK-NEXT: subfic 5, 4, 16
; CHECK-NEXT: neg 5, 4
; CHECK-NEXT: clrlwi 6, 3, 16
; CHECK-NEXT: rlwinm 4, 4, 0, 28, 31
; CHECK-NEXT: clrlwi 5, 5, 28
Expand Down Expand Up @@ -75,13 +75,11 @@ define i64 @rotl_i64(i64 %x, i64 %z) {
define <4 x i32> @rotl_v4i32(<4 x i32> %x, <4 x i32> %z) {
; CHECK-LABEL: rotl_v4i32:
; CHECK: # %bb.0:
; CHECK-NEXT: addis 3, 2, .LCPI5_0@toc@ha
; CHECK-NEXT: addi 3, 3, .LCPI5_0@toc@l
; CHECK-NEXT: lvx 4, 0, 3
; CHECK-NEXT: vsubuwm 4, 4, 3
; CHECK-NEXT: vslw 3, 2, 3
; CHECK-NEXT: vsrw 2, 2, 4
; CHECK-NEXT: xxlor 34, 35, 34
; CHECK-NEXT: xxlxor 36, 36, 36
; CHECK-NEXT: vslw 5, 2, 3
; CHECK-NEXT: vsubuwm 3, 4, 3
; CHECK-NEXT: vsrw 2, 2, 3
; CHECK-NEXT: xxlor 34, 37, 34
; CHECK-NEXT: blr
%f = call <4 x i32> @llvm.fshl.v4i32(<4 x i32> %x, <4 x i32> %x, <4 x i32> %z)
ret <4 x i32> %f
Expand Down Expand Up @@ -131,7 +129,7 @@ define i32 @rotr_i32_const_shift(i32 %x) {
define i16 @rotr_i16(i16 %x, i16 %z) {
; CHECK-LABEL: rotr_i16:
; CHECK: # %bb.0:
; CHECK-NEXT: subfic 5, 4, 16
; CHECK-NEXT: neg 5, 4
; CHECK-NEXT: clrlwi 6, 3, 16
; CHECK-NEXT: rlwinm 4, 4, 0, 28, 31
; CHECK-NEXT: clrlwi 5, 5, 28
Expand All @@ -146,7 +144,7 @@ define i16 @rotr_i16(i16 %x, i16 %z) {
define i32 @rotr_i32(i32 %x, i32 %z) {
; CHECK-LABEL: rotr_i32:
; CHECK: # %bb.0:
; CHECK-NEXT: subfic 4, 4, 32
; CHECK-NEXT: neg 4, 4
; CHECK-NEXT: clrlwi 4, 4, 27
; CHECK-NEXT: rlwnm 3, 3, 4, 0, 31
; CHECK-NEXT: blr
Expand All @@ -157,7 +155,7 @@ define i32 @rotr_i32(i32 %x, i32 %z) {
define i64 @rotr_i64(i64 %x, i64 %z) {
; CHECK-LABEL: rotr_i64:
; CHECK: # %bb.0:
; CHECK-NEXT: subfic 4, 4, 64
; CHECK-NEXT: neg 4, 4
; CHECK-NEXT: rlwinm 4, 4, 0, 26, 31
; CHECK-NEXT: rotld 3, 3, 4
; CHECK-NEXT: blr
Expand All @@ -170,13 +168,11 @@ define i64 @rotr_i64(i64 %x, i64 %z) {
define <4 x i32> @rotr_v4i32(<4 x i32> %x, <4 x i32> %z) {
; CHECK-LABEL: rotr_v4i32:
; CHECK: # %bb.0:
; CHECK-NEXT: addis 3, 2, .LCPI12_0@toc@ha
; CHECK-NEXT: addi 3, 3, .LCPI12_0@toc@l
; CHECK-NEXT: lvx 4, 0, 3
; CHECK-NEXT: vsubuwm 4, 4, 3
; CHECK-NEXT: vsrw 3, 2, 3
; CHECK-NEXT: vslw 2, 2, 4
; CHECK-NEXT: xxlor 34, 34, 35
; CHECK-NEXT: xxlxor 36, 36, 36
; CHECK-NEXT: vsrw 5, 2, 3
; CHECK-NEXT: vsubuwm 3, 4, 3
; CHECK-NEXT: vslw 2, 2, 3
; CHECK-NEXT: xxlor 34, 34, 37
; CHECK-NEXT: blr
%f = call <4 x i32> @llvm.fshr.v4i32(<4 x i32> %x, <4 x i32> %x, <4 x i32> %z)
ret <4 x i32> %f
Expand Down
Loading

0 comments on commit 8fe02fa

Please sign in to comment.