Skip to content

Commit 9002dad

Browse files
committed
[X86] Fix miscompile in combineShiftRightArithmetic
When folding (ashr (shl, x, c1), c2) we need to treat c1 and c2 as unsigned to find out if the combined shift should be a left or right shift. Also do an early out during pre-legalization in case c1 and c2 has differet types, as that otherwise complicated the comparison of c1 and c2 a bit.
1 parent ed11010 commit 9002dad

File tree

2 files changed

+11
-13
lines changed

2 files changed

+11
-13
lines changed

llvm/lib/Target/X86/X86ISelLowering.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47428,6 +47428,8 @@ static SDValue combineShiftRightArithmetic(SDNode *N, SelectionDAG &DAG,
4742847428
APInt SarConst = N1->getAsAPIntVal();
4742947429
EVT CVT = N1.getValueType();
4743047430

47431+
if (CVT != N01.getValueType())
47432+
return SDValue();
4743147433
if (SarConst.isNegative())
4743247434
return SDValue();
4743347435

@@ -47440,14 +47442,13 @@ static SDValue combineShiftRightArithmetic(SDNode *N, SelectionDAG &DAG,
4744047442
SDLoc DL(N);
4744147443
SDValue NN =
4744247444
DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, VT, N00, DAG.getValueType(SVT));
47443-
SarConst = SarConst - (Size - ShiftSize);
47444-
if (SarConst == 0)
47445+
if (SarConst.eq(ShlConst))
4744547446
return NN;
47446-
if (SarConst.isNegative())
47447+
if (SarConst.ult(ShlConst))
4744747448
return DAG.getNode(ISD::SHL, DL, VT, NN,
47448-
DAG.getConstant(-SarConst, DL, CVT));
47449+
DAG.getConstant(ShlConst - SarConst, DL, CVT));
4744947450
return DAG.getNode(ISD::SRA, DL, VT, NN,
47450-
DAG.getConstant(SarConst, DL, CVT));
47451+
DAG.getConstant(SarConst - ShlConst, DL, CVT));
4745147452
}
4745247453
return SDValue();
4745347454
}

llvm/test/CodeGen/X86/sar_fold.ll

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,17 @@ define void @shl144sar48(ptr %p) #0 {
6767
ret void
6868
}
6969

70-
; This is incorrect. The 142 least significant bits in the stored value should
71-
; be zero, and but 142-157 should be taken from %a with a sign-extend into the
72-
; two most significant bits.
7370
define void @shl144sar2(ptr %p) #0 {
7471
; CHECK-LABEL: shl144sar2:
7572
; CHECK: # %bb.0:
7673
; CHECK-NEXT: movl {{[0-9]+}}(%esp), %eax
7774
; CHECK-NEXT: movswl (%eax), %ecx
78-
; CHECK-NEXT: sarl $31, %ecx
75+
; CHECK-NEXT: shll $14, %ecx
7976
; CHECK-NEXT: movl %ecx, 16(%eax)
80-
; CHECK-NEXT: movl %ecx, 8(%eax)
81-
; CHECK-NEXT: movl %ecx, 12(%eax)
82-
; CHECK-NEXT: movl %ecx, 4(%eax)
83-
; CHECK-NEXT: movl %ecx, (%eax)
77+
; CHECK-NEXT: movl $0, 8(%eax)
78+
; CHECK-NEXT: movl $0, 12(%eax)
79+
; CHECK-NEXT: movl $0, 4(%eax)
80+
; CHECK-NEXT: movl $0, (%eax)
8481
; CHECK-NEXT: retl
8582
%a = load i160, ptr %p
8683
%1 = shl i160 %a, 144

0 commit comments

Comments
 (0)