Skip to content

Commit 1357c06

Browse files
committed
Revert "[X86][SSE] MatchVectorAllZeroTest - handle OR vector reductions"
This caused a Chromium test to miscompile. See discussion on the Phabricator review. > This patch extends MatchVectorAllZeroTest to handle OR vector reduction patterns where the result is compared against zero. > > Fixes PR45378 > > Differential Revision: https://reviews.llvm.org/D81547 This reverts 057c9c7
1 parent 5e2c736 commit 1357c06

File tree

3 files changed

+779
-459
lines changed

3 files changed

+779
-459
lines changed

llvm/lib/Target/X86/X86ISelLowering.cpp

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -21360,22 +21360,12 @@ static SDValue LowerVectorAllZero(const SDLoc &DL, SDValue V, ISD::CondCode CC,
2136021360
SelectionDAG &DAG, X86::CondCode &X86CC) {
2136121361
EVT VT = V.getValueType();
2136221362

21363-
assert((CC == ISD::SETEQ || CC == ISD::SETNE) && "Unsupported ISD::CondCode");
21364-
X86CC = (CC == ISD::SETEQ ? X86::COND_E : X86::COND_NE);
21365-
21366-
// For sub-128-bit vector, cast to (legal) integer and compare with zero.
21367-
if (VT.getSizeInBits() < 128) {
21368-
EVT IntVT = EVT::getIntegerVT(*DAG.getContext(), VT.getSizeInBits());
21369-
if (!DAG.getTargetLoweringInfo().isTypeLegal(IntVT))
21370-
return SDValue();
21371-
return DAG.getNode(X86ISD::CMP, DL, MVT::i32, DAG.getBitcast(IntVT, V),
21372-
DAG.getConstant(0, DL, IntVT));
21373-
}
21374-
21375-
// Quit if not splittable to 128/256-bit vector.
21376-
if (!isPowerOf2_32(VT.getSizeInBits()))
21363+
// Quit if less than 128-bits or not splittable to 128/256-bit vector.
21364+
if (VT.getSizeInBits() < 128 || !isPowerOf2_32(VT.getSizeInBits()))
2137721365
return SDValue();
2137821366

21367+
X86CC = (CC == ISD::SETEQ ? X86::COND_E : X86::COND_NE);
21368+
2137921369
// Split down to 128/256-bit vector.
2138021370
unsigned TestSize = Subtarget.hasAVX() ? 256 : 128;
2138121371
while (VT.getSizeInBits() > TestSize) {
@@ -21399,19 +21389,18 @@ static SDValue LowerVectorAllZero(const SDLoc &DL, SDValue V, ISD::CondCode CC,
2139921389
DAG.getConstant(0xFFFF, DL, MVT::i32));
2140021390
}
2140121391

21402-
// Check whether an OR'd reduction tree is PTEST-able, or if we can fallback to
21392+
// Check whether an OR'd tree is PTEST-able, or if we can fallback to
2140321393
// CMP(MOVMSK(PCMPEQB(X,0))).
2140421394
static SDValue MatchVectorAllZeroTest(SDValue Op, ISD::CondCode CC,
21405-
const SDLoc &DL,
2140621395
const X86Subtarget &Subtarget,
2140721396
SelectionDAG &DAG, SDValue &X86CC) {
21408-
assert((CC == ISD::SETEQ || CC == ISD::SETNE) && "Unsupported ISD::CondCode");
21397+
assert(Op.getOpcode() == ISD::OR && "Only check OR'd tree.");
2140921398

2141021399
if (!Subtarget.hasSSE2() || !Op->hasOneUse())
2141121400
return SDValue();
2141221401

2141321402
SmallVector<SDValue, 8> VecIns;
21414-
if (Op.getOpcode() == ISD::OR && matchScalarReduction(Op, ISD::OR, VecIns)) {
21403+
if (matchScalarReduction(Op, ISD::OR, VecIns)) {
2141521404
EVT VT = VecIns[0].getValueType();
2141621405
assert(llvm::all_of(VecIns,
2141721406
[VT](SDValue V) { return VT == V.getValueType(); }) &&
@@ -21421,6 +21410,8 @@ static SDValue MatchVectorAllZeroTest(SDValue Op, ISD::CondCode CC,
2142121410
if (VT.getSizeInBits() < 128 || !isPowerOf2_32(VT.getSizeInBits()))
2142221411
return SDValue();
2142321412

21413+
SDLoc DL(Op);
21414+
2142421415
// If more than one full vector is evaluated, OR them first before PTEST.
2142521416
for (unsigned Slot = 0, e = VecIns.size(); e - Slot > 1;
2142621417
Slot += 2, e += 1) {
@@ -21439,19 +21430,6 @@ static SDValue MatchVectorAllZeroTest(SDValue Op, ISD::CondCode CC,
2143921430
}
2144021431
}
2144121432

21442-
if (Op.getOpcode() == ISD::EXTRACT_VECTOR_ELT) {
21443-
ISD::NodeType BinOp;
21444-
if (SDValue Match =
21445-
DAG.matchBinOpReduction(Op.getNode(), BinOp, {ISD::OR})) {
21446-
X86::CondCode CCode;
21447-
if (SDValue V =
21448-
LowerVectorAllZero(DL, Match, CC, Subtarget, DAG, CCode)) {
21449-
X86CC = DAG.getTargetConstant(CCode, DL, MVT::i8);
21450-
return V;
21451-
}
21452-
}
21453-
}
21454-
2145521433
return SDValue();
2145621434
}
2145721435

@@ -22602,10 +22580,11 @@ SDValue X86TargetLowering::emitFlagsForSetcc(SDValue Op0, SDValue Op1,
2260222580

2260322581
// Try to use PTEST/PMOVMSKB for a tree ORs equality compared with 0.
2260422582
// TODO: We could do AND tree with all 1s as well by using the C flag.
22605-
if (isNullConstant(Op1) && (CC == ISD::SETEQ || CC == ISD::SETNE))
22606-
if (SDValue CmpZ =
22607-
MatchVectorAllZeroTest(Op0, CC, dl, Subtarget, DAG, X86CC))
22583+
if (Op0.getOpcode() == ISD::OR && isNullConstant(Op1) &&
22584+
(CC == ISD::SETEQ || CC == ISD::SETNE)) {
22585+
if (SDValue CmpZ = MatchVectorAllZeroTest(Op0, CC, Subtarget, DAG, X86CC))
2260822586
return CmpZ;
22587+
}
2260922588

2261022589
// Try to lower using KORTEST or KTEST.
2261122590
if (SDValue Test = EmitAVX512Test(Op0, Op1, CC, dl, DAG, Subtarget, X86CC))
@@ -46085,14 +46064,6 @@ static SDValue combineSetCC(SDNode *N, SelectionDAG &DAG,
4608546064
if (CC == ISD::SETNE || CC == ISD::SETEQ) {
4608646065
if (SDValue V = combineVectorSizedSetCCEquality(N, DAG, Subtarget))
4608746066
return V;
46088-
46089-
if (VT == MVT::i1) {
46090-
SDValue X86CC;
46091-
if (SDValue V =
46092-
MatchVectorAllZeroTest(LHS, CC, DL, Subtarget, DAG, X86CC))
46093-
return DAG.getNode(ISD::TRUNCATE, DL, VT,
46094-
DAG.getNode(X86ISD::SETCC, DL, MVT::i8, X86CC, V));
46095-
}
4609646067
}
4609746068

4609846069
if (VT.isVector() && VT.getVectorElementType() == MVT::i1 &&

llvm/test/CodeGen/X86/pr45378.ll

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,43 @@
99
declare i64 @llvm.experimental.vector.reduce.or.v2i64(<2 x i64>)
1010

1111
define i1 @parseHeaders(i64 * %ptr) nounwind {
12-
; SSE2-LABEL: parseHeaders:
13-
; SSE2: # %bb.0:
14-
; SSE2-NEXT: movdqu (%rdi), %xmm0
15-
; SSE2-NEXT: pxor %xmm1, %xmm1
16-
; SSE2-NEXT: pcmpeqb %xmm0, %xmm1
17-
; SSE2-NEXT: pmovmskb %xmm1, %eax
18-
; SSE2-NEXT: cmpl $65535, %eax # imm = 0xFFFF
19-
; SSE2-NEXT: sete %al
20-
; SSE2-NEXT: retq
12+
; SSE-LABEL: parseHeaders:
13+
; SSE: # %bb.0:
14+
; SSE-NEXT: movdqu (%rdi), %xmm0
15+
; SSE-NEXT: pshufd {{.*#+}} xmm1 = xmm0[2,3,0,1]
16+
; SSE-NEXT: por %xmm0, %xmm1
17+
; SSE-NEXT: movq %xmm1, %rax
18+
; SSE-NEXT: testq %rax, %rax
19+
; SSE-NEXT: sete %al
20+
; SSE-NEXT: retq
2121
;
22-
; SSE41-LABEL: parseHeaders:
23-
; SSE41: # %bb.0:
24-
; SSE41-NEXT: movdqu (%rdi), %xmm0
25-
; SSE41-NEXT: ptest %xmm0, %xmm0
26-
; SSE41-NEXT: sete %al
27-
; SSE41-NEXT: retq
22+
; AVX1-LABEL: parseHeaders:
23+
; AVX1: # %bb.0:
24+
; AVX1-NEXT: vmovdqu (%rdi), %xmm0
25+
; AVX1-NEXT: vpshufd {{.*#+}} xmm1 = xmm0[2,3,0,1]
26+
; AVX1-NEXT: vpor %xmm1, %xmm0, %xmm0
27+
; AVX1-NEXT: vmovq %xmm0, %rax
28+
; AVX1-NEXT: testq %rax, %rax
29+
; AVX1-NEXT: sete %al
30+
; AVX1-NEXT: retq
2831
;
29-
; AVX-LABEL: parseHeaders:
30-
; AVX: # %bb.0:
31-
; AVX-NEXT: vmovdqu (%rdi), %xmm0
32-
; AVX-NEXT: vptest %xmm0, %xmm0
33-
; AVX-NEXT: sete %al
34-
; AVX-NEXT: retq
32+
; AVX2-LABEL: parseHeaders:
33+
; AVX2: # %bb.0:
34+
; AVX2-NEXT: vpbroadcastq 8(%rdi), %xmm0
35+
; AVX2-NEXT: vpor (%rdi), %xmm0, %xmm0
36+
; AVX2-NEXT: vmovq %xmm0, %rax
37+
; AVX2-NEXT: testq %rax, %rax
38+
; AVX2-NEXT: sete %al
39+
; AVX2-NEXT: retq
40+
;
41+
; AVX512-LABEL: parseHeaders:
42+
; AVX512: # %bb.0:
43+
; AVX512-NEXT: vpbroadcastq 8(%rdi), %xmm0
44+
; AVX512-NEXT: vpor (%rdi), %xmm0, %xmm0
45+
; AVX512-NEXT: vmovq %xmm0, %rax
46+
; AVX512-NEXT: testq %rax, %rax
47+
; AVX512-NEXT: sete %al
48+
; AVX512-NEXT: retq
3549
%vptr = bitcast i64 * %ptr to <2 x i64> *
3650
%vload = load <2 x i64>, <2 x i64> * %vptr, align 8
3751
%vreduce = call i64 @llvm.experimental.vector.reduce.or.v2i64(<2 x i64> %vload)

0 commit comments

Comments
 (0)