These are all stylistic comments. Contentwise it looks fine.<br><br>+ !isa<ConstantSDNode> (N1.getOperand(1)))<br><br>Remove the space before the parenthese.<br><br>+ if (!isSymmetric && (NumElts==8) ) {<br>
<br>Remove extra space betweeen parentheses. Also capitalize isSymmetric and distance<br><br>+ }<br>+ else if ((IdxVal >= NumElts/2) && (ExtractIdxVal< NumElts/2)) {<br><br>Put else on the same line as the closing brace<br>
<br>+ // The insert-extract pair is symmetric when we extract element "5" and <br>+ // insert it in "1".<br>+ // If the pair is not symmetric and extracted and inserted elements are not <br><br>
Couple trailing spaces on these lines<br><br>+ if (NewOp.getNode()) <br>+ return NewOp;<br><br>Trailing space on the if line.<br><br><div class="gmail_quote">On Wed, Jul 18, 2012 at 2:03 AM, Demikhovsky, Elena <span dir="ltr"><<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div link="blue" vlink="purple" lang="EN-US">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi, I’d like to commit this patch, sending the patch again..<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><i><span style="color:teal">- Elena</span></i></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<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""> <a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank">llvm-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:llvm-commits-bounces@cs.uiuc.edu" target="_blank">llvm-commits-bounces@cs.uiuc.edu</a>]
<b>On Behalf Of </b>Demikhovsky, Elena<br>
<b>Sent:</b> Monday, July 16, 2012 09:50<br>
<b>To:</b> Nick Lewycky</span></p><div><div class="h5"><br>
<b>Cc:</b> Commit Messages and Patches for LLVM<br>
<b>Subject:</b> Re: [llvm-commits] Please Review: AVX code optimization<u></u><u></u></div></div><p></p>
</div>
</div><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I checked my optimization on codegen level against –O2 optimization. This is the code for comparison:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">IR before –O2:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> %c = extractelement <8 x i32> %a, i32 1<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> %d = insertelement <8 x i32> %b, i32 %c, i32 7<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The code:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> vunpcklps %ymm0, %ymm0, %ymm0 ## ymm0 = ymm0[0,0,1,1,4,4,5,5]<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> vperm2f128 $0, %ymm0, %ymm0, %ymm0 ## ymm0 = ymm0[0,1,0,1]<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> vblendps $128, %ymm0, %ymm1, %ymm0<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">After –O2:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> %d = shufflevector <8 x i32> %b, <8 x i32> %a, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 9><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The code:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> vextractf128 $1, %ymm1, %xmm2<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> vshufps $33, %xmm2, %xmm0, %xmm0 ## xmm0 = xmm0[1,0],xmm2[2,0]<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> vshufps $36, %xmm0, %xmm2, %xmm0 ## xmm0 = xmm2[0,1],xmm0[2,0]<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> vinsertf128 $1, %xmm0, %ymm1, %ymm0<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">--------------------------<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">IR before –O2:<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> %c = extractelement <4 x i64> %a, i32 3<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> %d = insertelement <4 x i64> %b, i64 %c, i32 2<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">vunpckhpd %ymm0, %ymm0, %ymm0 ## ymm0 = ymm0[1,1,3,3]<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">vblendpd $4, %ymm0, %ymm1, %ymm0<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">IR after –O2<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> %d = shufflevector <4 x i64> %a, <4 x i64> %b, <4 x i32> <i32 0, i32 1, i32 7, i32 3><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">vextractf128 $1, %ymm1, %xmm2<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">vextractf128 $1, %ymm0, %xmm0<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">vpunpckhqdq %xmm2, %xmm0, %xmm0 ## xmm0 = xmm0[1],xmm2[1]<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">vinsertf128 $1, %xmm0, %ymm1, %ymm0<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I have to say that the code, as I generated now in my own branch requires more changes in X86ISelLowering. I plan to send the patches one-by-one.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">And answering on your question<u></u><u></u></span></p>
<p><u></u><span style="font-family:Wingdings"><span>Ř<span style="font:7.0pt "Times New Roman"">
</span></span></span><u></u><span dir="LTR"></span>Or is this a pattern that parts of the backend will produce internally where the IR optimizers couldn't see it?<u></u><u></u></p>
<p><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Right, the optimizer does not see this pattern, our backend generates it later.<u></u><u></u></span></p>
<p class="MsoNormal"><b><i><span style="color:teal"><u></u> <u></u></span></i></b></p>
<p class="MsoNormal"><b><i><span style="color:teal"><u></u> <u></u></span></i></b></p>
<p class="MsoNormal"><b><i><span style="color:teal">- Elena</span></i></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<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""> Nick Lewycky [<a href="mailto:nlewycky@google.com" target="_blank">mailto:nlewycky@google.com</a>]
<br>
<b>Sent:</b> Friday, July 13, 2012 21:43<br>
<b>To:</b> Demikhovsky, Elena<br>
<b>Cc:</b> Nick Lewycky; Commit Messages and Patches for LLVM<br>
<b>Subject:</b> Re: [llvm-commits] Please Review: AVX code optimization<u></u><u></u></span></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On 11 July 2012 03:34, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">I'm not sure that all architectures will see performance gain.<br>
While building shuffles, I know that each shuffle will be replaced with one machine instruction.<br>
I also know that shuffle is cheaper (1 cycle) than extract (3 cycles) and insert (2 cycles).<br>
I know that blend is better than other shuffle. And this information is specific for X86 and written in IA optimization guide.<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">The IR-level optimizers already do transform your testcases into shufflevector instructions. Here's the result after opt -O2:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">define <8 x i32> @test20(<8 x i32> %a, <8 x i32> %b) nounwind readnone {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> %d = shufflevector <8 x i32> %b, <8 x i32> %a, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 9><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> ret <8 x i32> %d<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">}<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">define <8 x i32> @test21(<8 x i32> %a, <8 x i32> %b) nounwind readnone {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> %d = shufflevector <8 x i32> %b, <8 x i32> %a, <8 x i32> <i32 0, i32 1, i32 9, i32 3, i32 4, i32 5, i32 6, i32 7><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> ret <8 x i32> %d<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">}<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">define <4 x i64> @test22(<4 x i64> %a, <4 x i64> %b) nounwind readnone {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> %d = shufflevector <4 x i64> %b, <4 x i64> %a, <4 x i32> <i32 0, i32 1, i32 7, i32 3><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> ret <4 x i64> %d<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">}<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">define <4 x i64> @test23(<4 x i64> %a, <4 x i64> %b) nounwind readnone {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> %d = shufflevector <4 x i64> %b, <4 x i64> %a, <4 x i32> <i32 0, i32 1, i32 7, i32 3><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> ret <4 x i64> %d<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">}<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">In what case does the patch you sent in improve generated code? Running the optimizing code generator on unoptimized IR? Or is this a pattern that parts of the backend will produce internally where the IR optimizers couldn't see it?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Nick<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="color:#888888"><br>
<span>- Elena</span></span><u></u><u></u></p>
<div>
<p class="MsoNormal">-----Original Message-----<br>
From: Nick Lewycky [mailto:<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>]<br>
Sent: Wednesday, July 11, 2012 11:47<br>
To: Demikhovsky, Elena<br>
Cc: Commit Messages and Patches for LLVM<br>
Subject: Re: [llvm-commits] Please Review: AVX code optimization<br>
<br>
Demikhovsky, Elena wrote:<br>
> I wrote an optimization for extractelement - insertelement sequences.<br>
> Please review.<br>
<br>
It looks like this is a dagcombine to turn insertelement+extractelement pairs into vector shuffles. Perhaps I'm missing a good reason, but why not do this as an IR optimization?<br>
<br>
Nick<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
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.<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">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><u></u><u></u></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New"">---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
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.</span><u></u><u></u></p>
</div></div></div><div><div class="h5">
<font face="monospace">---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
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.</font></div></div></div>
<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><br>
<br></blockquote></div><br><br clear="all"><br>-- <br>~Craig<br>