<div dir="ltr">This crashes on some vector code, reduced test case attached and reverted in r336189.<div><br></div><div>Reproduce with:</div><div>$ llc bugpoint-reduced-simplified.ll -mcpu=corei7-avx</div><div>llc: llvm/include/llvm/Support/Casting.h:92: static bool llvm::isa_impl_cl<llvm::ConstantSDNode, llvm::SDNode *>::doit(const From *) [To = llvm::ConstantSDNode, From = llvm::SDNode *]: Assertion `Val && "isa<> used on a null pointer"' failed.<br></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jul 2, 2018 at 5:18 PM Simon Pilgrim via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rksimon<br>
Date: Mon Jul 2 08:14:07 2018<br>
New Revision: 336113<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=336113&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=336113&view=rev</a><br>
Log:<br>
[X86][SSE] Blend any v8i16/v4i32 shift with 2 shift unique values<br>
<br>
We were only doing this for basic blends, despite shuffle lowering now being good enough to handle more complex blends. This means that the two v8i16 splat shifts are performed in parallel instead of serially as the general shift case.<br>
<br>
Modified:<br>
llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
llvm/trunk/test/CodeGen/X86/lower-vec-shift.ll<br>
<br>
Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=336113&r1=336112&r2=336113&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=336113&r1=336112&r2=336113&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon Jul 2 08:14:07 2018<br>
@@ -23441,7 +23441,7 @@ static SDValue LowerShift(SDValue Op, co<br>
return DAG.getNode(ISD::MUL, dl, VT, R, Scale);<br>
<br>
// If possible, lower this shift as a sequence of two shifts by<br>
- // constant plus a MOVSS/MOVSD/PBLEND instead of scalarizing it.<br>
+ // constant plus a BLENDing shuffle instead of scalarizing it.<br>
// Example:<br>
// (v4i32 (srl A, (build_vector < X, Y, Y, Y>)))<br>
//<br>
@@ -23449,64 +23449,39 @@ static SDValue LowerShift(SDValue Op, co<br>
// (v4i32 (MOVSS (srl A, <Y,Y,Y,Y>), (srl A, <X,X,X,X>)))<br>
//<br>
// The advantage is that the two shifts from the example would be<br>
- // lowered as X86ISD::VSRLI nodes. This would be cheaper than scalarizing<br>
- // the vector shift into four scalar shifts plus four pairs of vector<br>
- // insert/extract.<br>
+ // lowered as X86ISD::VSRLI nodes in parallel before blending.<br>
if (ConstantAmt && (VT == MVT::v8i16 || VT == MVT::v4i32)) {<br>
- bool UseMOVSD = false;<br>
- bool CanBeSimplified;<br>
- // The splat value for the first packed shift (the 'X' from the example).<br>
- SDValue Amt1 = Amt->getOperand(0);<br>
- // The splat value for the second packed shift (the 'Y' from the example).<br>
- SDValue Amt2 = (VT == MVT::v4i32) ? Amt->getOperand(1) : Amt->getOperand(2);<br>
-<br>
- // See if it is possible to replace this node with a sequence of<br>
- // two shifts followed by a MOVSS/MOVSD/PBLEND.<br>
- if (VT == MVT::v4i32) {<br>
- // Check if it is legal to use a MOVSS.<br>
- CanBeSimplified = Amt2 == Amt->getOperand(2) &&<br>
- Amt2 == Amt->getOperand(3);<br>
- if (!CanBeSimplified) {<br>
- // Otherwise, check if we can still simplify this node using a MOVSD.<br>
- CanBeSimplified = Amt1 == Amt->getOperand(1) &&<br>
- Amt->getOperand(2) == Amt->getOperand(3);<br>
- UseMOVSD = true;<br>
- Amt2 = Amt->getOperand(2);<br>
+ SDValue Amt1, Amt2;<br>
+ unsigned NumElts = VT.getVectorNumElements();<br>
+ SmallVector<int, 8> ShuffleMask;<br>
+ for (unsigned i = 0; i != NumElts; ++i) {<br>
+ SDValue A = Amt->getOperand(i);<br>
+ if (A.isUndef()) {<br>
+ ShuffleMask.push_back(SM_SentinelUndef);<br>
+ continue;<br>
}<br>
- } else {<br>
- // Do similar checks for the case where the machine value type<br>
- // is MVT::v8i16.<br>
- CanBeSimplified = Amt1 == Amt->getOperand(1);<br>
- for (unsigned i=3; i != 8 && CanBeSimplified; ++i)<br>
- CanBeSimplified = Amt2 == Amt->getOperand(i);<br>
-<br>
- if (!CanBeSimplified) {<br>
- UseMOVSD = true;<br>
- CanBeSimplified = true;<br>
- Amt2 = Amt->getOperand(4);<br>
- for (unsigned i=0; i != 4 && CanBeSimplified; ++i)<br>
- CanBeSimplified = Amt1 == Amt->getOperand(i);<br>
- for (unsigned j=4; j != 8 && CanBeSimplified; ++j)<br>
- CanBeSimplified = Amt2 == Amt->getOperand(j);<br>
+ if (!Amt1 || Amt1 == A) {<br>
+ ShuffleMask.push_back(i);<br>
+ Amt1 = A;<br>
+ continue;<br>
+ }<br>
+ if (!Amt2 || Amt2 == A) {<br>
+ ShuffleMask.push_back(i + NumElts);<br>
+ Amt2 = A;<br>
+ continue;<br>
}<br>
+ break;<br>
}<br>
<br>
- if (CanBeSimplified && isa<ConstantSDNode>(Amt1) &&<br>
+ if (ShuffleMask.size() == NumElts && isa<ConstantSDNode>(Amt1) &&<br>
isa<ConstantSDNode>(Amt2)) {<br>
- // Replace this node with two shifts followed by a MOVSS/MOVSD/PBLEND.<br>
SDValue Splat1 =<br>
DAG.getConstant(cast<ConstantSDNode>(Amt1)->getAPIntValue(), dl, VT);<br>
SDValue Shift1 = DAG.getNode(Op->getOpcode(), dl, VT, R, Splat1);<br>
SDValue Splat2 =<br>
DAG.getConstant(cast<ConstantSDNode>(Amt2)->getAPIntValue(), dl, VT);<br>
SDValue Shift2 = DAG.getNode(Op->getOpcode(), dl, VT, R, Splat2);<br>
- SDValue BitCast1 = DAG.getBitcast(MVT::v4i32, Shift1);<br>
- SDValue BitCast2 = DAG.getBitcast(MVT::v4i32, Shift2);<br>
- if (UseMOVSD)<br>
- return DAG.getBitcast(VT, DAG.getVectorShuffle(MVT::v4i32, dl, BitCast1,<br>
- BitCast2, {0, 1, 6, 7}));<br>
- return DAG.getBitcast(VT, DAG.getVectorShuffle(MVT::v4i32, dl, BitCast1,<br>
- BitCast2, {0, 5, 6, 7}));<br>
+ return DAG.getVectorShuffle(VT, dl, Shift1, Shift2, ShuffleMask);<br>
}<br>
}<br>
<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/lower-vec-shift.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/lower-vec-shift.ll?rev=336113&r1=336112&r2=336113&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/lower-vec-shift.ll?rev=336113&r1=336112&r2=336113&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/lower-vec-shift.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/lower-vec-shift.ll Mon Jul 2 08:14:07 2018<br>
@@ -211,31 +211,21 @@ define <4 x i32> @test8(<4 x i32> %a) {<br>
define <8 x i16> @test9(<8 x i16> %a) {<br>
; SSE-LABEL: test9:<br>
; SSE: # %bb.0:<br>
-; SSE-NEXT: movdqa {{.*#+}} xmm2 = [65535,0,65535,65535,65535,0,0,0]<br>
; SSE-NEXT: movdqa %xmm0, %xmm1<br>
-; SSE-NEXT: pand %xmm2, %xmm1<br>
-; SSE-NEXT: psraw $2, %xmm0<br>
-; SSE-NEXT: pandn %xmm0, %xmm2<br>
-; SSE-NEXT: por %xmm2, %xmm1<br>
-; SSE-NEXT: psraw $1, %xmm1<br>
-; SSE-NEXT: movdqa %xmm1, %xmm0<br>
+; SSE-NEXT: psraw $3, %xmm1<br>
+; SSE-NEXT: movdqa {{.*#+}} xmm2 = [65535,0,65535,65535,65535,0,0,0]<br>
+; SSE-NEXT: psraw $1, %xmm0<br>
+; SSE-NEXT: pand %xmm2, %xmm0<br>
+; SSE-NEXT: pandn %xmm1, %xmm2<br>
+; SSE-NEXT: por %xmm2, %xmm0<br>
; SSE-NEXT: retq<br>
;<br>
-; AVX1-LABEL: test9:<br>
-; AVX1: # %bb.0:<br>
-; AVX1-NEXT: vpsraw $2, %xmm0, %xmm1<br>
-; AVX1-NEXT: vpblendw {{.*#+}} xmm0 = xmm0[0],xmm1[1],xmm0[2,3,4],xmm1[5,6,7]<br>
-; AVX1-NEXT: vpsraw $1, %xmm0, %xmm0<br>
-; AVX1-NEXT: retq<br>
-;<br>
-; AVX2-LABEL: test9:<br>
-; AVX2: # %bb.0:<br>
-; AVX2-NEXT: vpmovsxwd %xmm0, %ymm0<br>
-; AVX2-NEXT: vpsravd {{.*}}(%rip), %ymm0, %ymm0<br>
-; AVX2-NEXT: vextracti128 $1, %ymm0, %xmm1<br>
-; AVX2-NEXT: vpackssdw %xmm1, %xmm0, %xmm0<br>
-; AVX2-NEXT: vzeroupper<br>
-; AVX2-NEXT: retq<br>
+; AVX-LABEL: test9:<br>
+; AVX: # %bb.0:<br>
+; AVX-NEXT: vpsraw $3, %xmm0, %xmm1<br>
+; AVX-NEXT: vpsraw $1, %xmm0, %xmm0<br>
+; AVX-NEXT: vpblendw {{.*#+}} xmm0 = xmm0[0],xmm1[1],xmm0[2,3,4],xmm1[5,6,7]<br>
+; AVX-NEXT: retq<br>
%lshr = ashr <8 x i16> %a, <i16 1, i16 3, i16 1, i16 1, i16 1, i16 3, i16 3, i16 3><br>
ret <8 x i16> %lshr<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>