<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Tahoma;
        panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
span.EmailStyle17
        {mso-style-type:personal-reply;
        font-family:"Calibri","sans-serif";
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-family:"Calibri","sans-serif";}
.MsoPapDefault
        {mso-style-type:export-only;
        line-height:115%;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Argh, I’m really sorry about llvm-commits, my bad.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">It looks like in the cited PR it was the best sequence, but I agree with you, it may not be the case globally.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Which stalls are you talking about? I think domain crossing shouldn’t be a problem in this case, as the zexts would imply you want to be in the integer domain.
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Regarding systematic testing – no, since this is a fairly specific pattern.
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Do you have any examples in mind that will match this, but be negatively impacted?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Regarding patterns impacted by this - if I understand correctly, the pattern that this was introduced to catch was precisely the one the LIT test checks – 64-bit
 GEPs that use indexes extracted from a 4xi32 vector. There’s a rdar linked to the test.  Quentin, do you think it’s worth checking what the impact of this is on the original issue?
<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Chandler Carruth [mailto:chandlerc@google.com]
<br>
<b>Sent:</b> Thursday, December 04, 2014 18:02<br>
<b>To:</b> Kuperstein, Michael M<br>
<b>Cc:</b> Commit Messages and Patches for LLVM<br>
<b>Subject:</b> Re: [llvm] r223360 - [X86] Improve a dag-combine that handles a vector extract -> zext sequence.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal" style="line-height:115%">This patch's code review only went to llvm-commits when you submitted it. =/ Really unfortunate.<o:p></o:p></p>
<div>
<p class="MsoNormal" style="line-height:115%"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="line-height:115%">I'm not at all convinced this is moving in the right direction. pextr and pinsr are just about the slowest, and most damaging instructions to use in any sequence of vector code. They cause *huge* stalls. Even the
 measurements in the cited PR show that these instruction patterns are deeply problematic.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="line-height:115%"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="line-height:115%">I have on a many, many occasions seen loads and stores to stack memory be dramatically faster than pextr. I don't think that we've actually root-caused the problem here, and worry that this is essentially over-fitting
 the transforms in LLVM to a single benchmark this is unlikely to represent all of the patterns that are actually impacted by it...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="line-height:115%"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal" style="line-height:115%">Has there been any systematic measurement? Has there been any effort to actually produce an *efficient* instruction sequence here?<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal" style="line-height:115%"><o:p> </o:p></p>
<div>
<p class="MsoNormal" style="line-height:115%">On Thu, Dec 4, 2014 at 5:49 AM, Michael Kuperstein <<a href="mailto:michael.m.kuperstein@intel.com" target="_blank">michael.m.kuperstein@intel.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal" style="line-height:115%">Author: mkuper<br>
Date: Thu Dec  4 07:49:51 2014<br>
New Revision: 223360<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=223360&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=223360&view=rev</a><br>
Log:<br>
[X86] Improve a dag-combine that handles a vector extract -> zext sequence.<br>
<br>
The current DAG combine turns a sequence of extracts from <4 x i32> followed by zexts into a store followed by scalar loads.<br>
According to measurements by Martin Krastev (see PR 21269) for x86-64, a sequence of an extract, movs and shifts gives better performance. However, for 32-bit x86, the previous sequence still seems better.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D6501" target="_blank">http://reviews.llvm.org/D6501</a><br>
<br>
Modified:<br>
    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
    llvm/trunk/test/CodeGen/X86/gather-addresses.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=223360&r1=223359&r2=223360&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=223360&r1=223359&r2=223360&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Thu Dec  4 07:49:51 2014<br>
@@ -22558,7 +22558,9 @@ static SDValue XFormVExtractWithShuffleI<br>
<br>
 /// PerformEXTRACT_VECTOR_ELTCombine - Detect vector gather/scatter index<br>
 /// generation and convert it from being a bunch of shuffles and extracts<br>
-/// to a simple store and scalar loads to extract the elements.<br>
+/// into a somewhat faster sequence. For i686, the best sequence is apparently<br>
+/// storing the value and loading scalars back, while for x64 we should<br>
+/// use 64-bit extracts and shifts.<br>
 static SDValue PerformEXTRACT_VECTOR_ELTCombine(SDNode *N, SelectionDAG &DAG,<br>
                                          TargetLowering::DAGCombinerInfo &DCI) {<br>
   SDValue NewOp = XFormVExtractWithShuffleIntoLoad(N, DAG, DCI);<br>
@@ -22617,36 +22619,61 @@ static SDValue PerformEXTRACT_VECTOR_ELT<br>
     return SDValue();<br>
<br>
   // Ok, we've now decided to do the transformation.<br>
+  // If 64-bit shifts are legal, use the extract-shift sequence,<br>
+  // otherwise bounce the vector off the cache.<br>
+  const TargetLowering &TLI = DAG.getTargetLoweringInfo();<br>
+  SDValue Vals[4];<br>
   SDLoc dl(InputVector);<br>
+<br>
+  if (TLI.isOperationLegal(ISD::SRA, MVT::i64)) {<br>
+    SDValue Cst = DAG.getNode(ISD::BITCAST, dl, MVT::v2i64, InputVector);<br>
+    EVT VecIdxTy = DAG.getTargetLoweringInfo().getVectorIdxTy();<br>
+    SDValue BottomHalf = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, MVT::i64, Cst,<br>
+      DAG.getConstant(0, VecIdxTy));<br>
+    SDValue TopHalf = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, MVT::i64, Cst,<br>
+      DAG.getConstant(1, VecIdxTy));<br>
+<br>
+    SDValue ShAmt = DAG.getConstant(32,<br>
+      DAG.getTargetLoweringInfo().getShiftAmountTy(MVT::i64));<br>
+    Vals[0] = DAG.getNode(ISD::TRUNCATE, dl, MVT::i32, BottomHalf);<br>
+    Vals[1] = DAG.getNode(ISD::TRUNCATE, dl, MVT::i32,<br>
+      DAG.getNode(ISD::SRA, dl, MVT::i64, BottomHalf, ShAmt));<br>
+    Vals[2] = DAG.getNode(ISD::TRUNCATE, dl, MVT::i32, TopHalf);<br>
+    Vals[3] = DAG.getNode(ISD::TRUNCATE, dl, MVT::i32,<br>
+      DAG.getNode(ISD::SRA, dl, MVT::i64, TopHalf, ShAmt));<br>
+  } else {<br>
+    // Store the value to a temporary stack slot.<br>
+    SDValue StackPtr = DAG.CreateStackTemporary(InputVector.getValueType());<br>
+    SDValue Ch = DAG.getStore(DAG.getEntryNode(), dl, InputVector, StackPtr,<br>
+      MachinePointerInfo(), false, false, 0);<br>
+<br>
+    EVT ElementType = InputVector.getValueType().getVectorElementType();<br>
+    unsigned EltSize = ElementType.getSizeInBits() / 8;<br>
+<br>
+    // Replace each use (extract) with a load of the appropriate element.<br>
+    for (unsigned i = 0; i < 4; ++i) {<br>
+      uint64_t Offset = EltSize * i;<br>
+      SDValue OffsetVal = DAG.getConstant(Offset, TLI.getPointerTy());<br>
+<br>
+      SDValue ScalarAddr = DAG.getNode(ISD::ADD, dl, TLI.getPointerTy(),<br>
+                                       StackPtr, OffsetVal);<br>
+<br>
+      // Load the scalar.<br>
+      Vals[i] = DAG.getLoad(ElementType, dl, Ch,<br>
+                            ScalarAddr, MachinePointerInfo(),<br>
+                            false, false, false, 0);<br>
<br>
-  // Store the value to a temporary stack slot.<br>
-  SDValue StackPtr = DAG.CreateStackTemporary(InputVector.getValueType());<br>
-  SDValue Ch = DAG.getStore(DAG.getEntryNode(), dl, InputVector, StackPtr,<br>
-                            MachinePointerInfo(), false, false, 0);<br>
+    }<br>
+  }<br>
<br>
-  // Replace each use (extract) with a load of the appropriate element.<br>
+  // Replace the extracts<br>
   for (SmallVectorImpl<SDNode *>::iterator UI = Uses.begin(),<br>
-       UE = Uses.end(); UI != UE; ++UI) {<br>
+    UE = Uses.end(); UI != UE; ++UI) {<br>
     SDNode *Extract = *UI;<br>
<br>
-    // cOMpute the element's address.<br>
     SDValue Idx = Extract->getOperand(1);<br>
-    unsigned EltSize =<br>
-        InputVector.getValueType().getVectorElementType().getSizeInBits()/8;<br>
-    uint64_t Offset = EltSize * cast<ConstantSDNode>(Idx)->getZExtValue();<br>
-    const TargetLowering &TLI = DAG.getTargetLoweringInfo();<br>
-    SDValue OffsetVal = DAG.getConstant(Offset, TLI.getPointerTy());<br>
-<br>
-    SDValue ScalarAddr = DAG.getNode(ISD::ADD, dl, TLI.getPointerTy(),<br>
-                                     StackPtr, OffsetVal);<br>
-<br>
-    // Load the scalar.<br>
-    SDValue LoadScalar = DAG.getLoad(Extract->getValueType(0), dl, Ch,<br>
-                                     ScalarAddr, MachinePointerInfo(),<br>
-                                     false, false, false, 0);<br>
-<br>
-    // Replace the exact with the load.<br>
-    DAG.ReplaceAllUsesOfValueWith(SDValue(Extract, 0), LoadScalar);<br>
+    uint64_t IdxVal = cast<ConstantSDNode>(Idx)->getZExtValue();<br>
+    DAG.ReplaceAllUsesOfValueWith(SDValue(Extract, 0), Vals[IdxVal]);<br>
   }<br>
<br>
   // The replacement was made in place; don't return anything.<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/gather-addresses.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/gather-addresses.ll?rev=223360&r1=223359&r2=223360&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/gather-addresses.ll?rev=223360&r1=223359&r2=223360&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/gather-addresses.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/gather-addresses.ll Thu Dec  4 07:49:51 2014<br>
@@ -1,35 +1,38 @@<br>
 ; RUN: llc -mtriple=x86_64-linux -mcpu=nehalem < %s | FileCheck %s --check-prefix=LIN<br>
 ; RUN: llc -mtriple=x86_64-win32 -mcpu=nehalem < %s | FileCheck %s --check-prefix=WIN<br>
+; RUN: llc -mtriple=i686-win32 -mcpu=nehalem < %s | FileCheck %s --check-prefix=LIN32<br>
 ; rdar://7398554<br>
<br>
 ; When doing vector gather-scatter index calculation with 32-bit indices,<br>
-; bounce the vector off of cache rather than shuffling each individual<br>
+; use an efficient mov/shift sequence rather than shuffling each individual<br>
 ; element out of the index vector.<br>
<br>
-; CHECK: foo:<br>
-; LIN: movaps  (%rsi), %xmm0<br>
-; LIN: andps   (%rdx), %xmm0<br>
-; LIN: movaps  %xmm0, -24(%rsp)<br>
-; LIN: movslq  -24(%rsp), %[[REG1:r.+]]<br>
-; LIN: movslq  -20(%rsp), %[[REG2:r.+]]<br>
-; LIN: movslq  -16(%rsp), %[[REG3:r.+]]<br>
-; LIN: movslq  -12(%rsp), %[[REG4:r.+]]<br>
-; LIN: movsd   (%rdi,%[[REG1]],8), %xmm0<br>
-; LIN: movhpd  (%rdi,%[[REG2]],8), %xmm0<br>
-; LIN: movsd   (%rdi,%[[REG3]],8), %xmm1<br>
-; LIN: movhpd  (%rdi,%[[REG4]],8), %xmm1<br>
+; CHECK-LABEL: foo:<br>
+; LIN: movdqa  (%rsi), %xmm0<br>
+; LIN: pand    (%rdx), %xmm0<br>
+; LIN: pextrq  $1, %xmm0, %r[[REG4:.+]]<br>
+; LIN: movd    %xmm0, %r[[REG2:.+]]<br>
+; LIN: movslq  %e[[REG2]], %r[[REG1:.+]]<br>
+; LIN: sarq    $32, %r[[REG2]]<br>
+; LIN: movslq  %e[[REG4]], %r[[REG3:.+]]<br>
+; LIN: sarq    $32, %r[[REG4]]<br>
+; LIN: movsd   (%rdi,%r[[REG1]],8), %xmm0<br>
+; LIN: movhpd  (%rdi,%r[[REG2]],8), %xmm0<br>
+; LIN: movsd   (%rdi,%r[[REG3]],8), %xmm1<br>
+; LIN: movhpd  (%rdi,%r[[REG4]],8), %xmm1<br>
<br>
-; WIN: movaps  (%rdx), %xmm0<br>
-; WIN: andps   (%r8), %xmm0<br>
-; WIN: movaps  %xmm0, (%rsp)<br>
-; WIN: movslq  (%rsp), %[[REG1:r.+]]<br>
-; WIN: movslq  4(%rsp), %[[REG2:r.+]]<br>
-; WIN: movslq  8(%rsp), %[[REG3:r.+]]<br>
-; WIN: movslq  12(%rsp), %[[REG4:r.+]]<br>
-; WIN: movsd   (%rcx,%[[REG1]],8), %xmm0<br>
-; WIN: movhpd  (%rcx,%[[REG2]],8), %xmm0<br>
-; WIN: movsd   (%rcx,%[[REG3]],8), %xmm1<br>
-; WIN: movhpd  (%rcx,%[[REG4]],8), %xmm1<br>
+; WIN: movdqa  (%rdx), %xmm0<br>
+; WIN: pand    (%r8), %xmm0<br>
+; WIN: pextrq  $1, %xmm0, %r[[REG4:.+]]<br>
+; WIN: movd    %xmm0, %r[[REG2:.+]]<br>
+; WIN: movslq  %e[[REG2]], %r[[REG1:.+]]<br>
+; WIN: sarq    $32, %r[[REG2]]<br>
+; WIN: movslq  %e[[REG4]], %r[[REG3:.+]]<br>
+; WIN: sarq    $32, %r[[REG4]]<br>
+; WIN: movsd   (%rcx,%r[[REG1]],8), %xmm0<br>
+; WIN: movhpd  (%rcx,%r[[REG2]],8), %xmm0<br>
+; WIN: movsd   (%rcx,%r[[REG3]],8), %xmm1<br>
+; WIN: movhpd  (%rcx,%r[[REG4]],8), %xmm1<br>
<br>
 define <4 x double> @foo(double* %p, <4 x i32>* %i, <4 x i32>* %h) nounwind {<br>
   %a = load <4 x i32>* %i<br>
@@ -53,3 +56,35 @@ define <4 x double> @foo(double* %p, <4<br>
   %v3 = insertelement <4 x double> %v2, double %r3, i32 3<br>
   ret <4 x double> %v3<br>
 }<br>
+<br>
+; Check that the sequence previously used above, which bounces the vector off the<br>
+; cache works for x86-32. Note that in this case it will not be used for index<br>
+; calculation, since indexes are 32-bit, not 64.<br>
+; CHECK-LABEL: old:<br>
+; LIN32: movaps        %xmm0, (%esp)<br>
+; LIN32-DAG: {{(mov|and)}}l    (%esp),<br>
+; LIN32-DAG: {{(mov|and)}}l    4(%esp),<br>
+; LIN32-DAG: {{(mov|and)}}l    8(%esp),<br>
+; LIN32-DAG: {{(mov|and)}}l    12(%esp),<br>
+define <4 x i64> @old(double* %p, <4 x i32>* %i, <4 x i32>* %h, i64 %f) nounwind {<br>
+  %a = load <4 x i32>* %i<br>
+  %b = load <4 x i32>* %h<br>
+  %j = and <4 x i32> %a, %b<br>
+  %d0 = extractelement <4 x i32> %j, i32 0<br>
+  %d1 = extractelement <4 x i32> %j, i32 1<br>
+  %d2 = extractelement <4 x i32> %j, i32 2<br>
+  %d3 = extractelement <4 x i32> %j, i32 3<br>
+  %q0 = zext i32 %d0 to i64<br>
+  %q1 = zext i32 %d1 to i64<br>
+  %q2 = zext i32 %d2 to i64<br>
+  %q3 = zext i32 %d3 to i64<br>
+  %r0 = and i64 %q0, %f<br>
+  %r1 = and i64 %q1, %f<br>
+  %r2 = and i64 %q2, %f<br>
+  %r3 = and i64 %q3, %f<br>
+  %v0 = insertelement <4 x i64> undef, i64 %r0, i32 0<br>
+  %v1 = insertelement <4 x i64> %v0, i64 %r1, i32 1<br>
+  %v2 = insertelement <4 x i64> %v1, i64 %r2, i32 2<br>
+  %v3 = insertelement <4 x i64> %v2, i64 %r3, i32 3<br>
+  ret <4 x i64> %v3<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><o:p></o:p></p>
</div>
<p class="MsoNormal" style="line-height:115%"><o:p> </o:p></p>
</div>
</div>
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited</p>

<p>This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.</p></body>
</html>