<div dir="ltr">s/<4 x i16>/<4 x i8>/ above- typo.<br></div><br><div class="gmail_quote">On Tue, 12 May 2015 at 14:48 James Molloy <<a href="mailto:james@jamesmolloy.co.uk">james@jamesmolloy.co.uk</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Sam,<br><br>Thanks for making those changes.<div><br></div><div>  * Don't use a newline between } and else. Good: "} else {" Bad: "}\nelse{".</div><div>  * Trivial one-liners should not have braces.</div><div>  * Other vectorizer testcases check the debug output to check the correct costs are being considered internally. It would be a good idea to do that here too.</div><div>  * I think both your patches have an issue: all other instructions should be clamped to the widest type before cost analysis. Consider:</div><div><br></div><div>getWidestType() == i8</div><div>%1 = zext i8 %foo to i32</div><div>%2 = add i32 %1, 5</div><div><br></div><div>You will query TTI for the %2 instruction and ask "what is the cost of a <4 x i32> add?". You should be asking "What is the cost of an <4 x i16> add?".</div><div><br></div><div>This is going to be more difficult for the SLP case, so I suggest you peel the SLP stuff out into a separate patch.</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote">On Tue, 12 May 2015 at 13:05 Sam Parker <<a href="mailto:sam.parker@arm.com" target="_blank">sam.parker@arm.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi James,<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’ve added the two test files for loop- and slp-vectorize. I have also added an additional check so that if the widest type is less than the target ‘VectorTy’, the cost is actually calculated as a cast to a vector of the widest type instead.<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">Cheers,<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Sam<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"><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""> James Molloy [mailto:<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>] <br><b>Sent:</b> 11 May 2015 16:34<br><b>To:</b> Sam Parker; Demikhovsky, Elena</span></p></div></div><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""><br><b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><b>Subject:</b> Re: [PATCH] improve vectorizers by removing cost of unnecessary truncs and exts.<u></u><u></u></span></p></div></div><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">Hi Sam,<br><br>It's a mismodelling, where you'll say something is cheaper than it actually is.<u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">Cheers,<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">James<u></u><u></u></p></div></div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">On Mon, 11 May 2015 at 15:46 Sam Parker <<a href="mailto:sam.parker@arm.com" target="_blank">sam.parker@arm.com</a>> wrote:<u></u><u></u></p><div><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi James,</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">By not totally sound, do you mean incorrect or that I’m just missing an extra opportunity and I should check that extensions aren’t targeting a larger than necessary bitwidth? As for the test file, I didn’t know if I was supposed to add it along with the patch or not, and will attach it in the next version.</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers,</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Sam</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></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""> James Molloy [mailto:<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>] <br><b>Sent:</b> 11 May 2015 13:49<br><b>To:</b> Demikhovsky, Elena; Sam Parker<br><b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><b>Subject:</b> Re: [PATCH] improve vectorizers by removing cost of unnecessary truncs and exts.</span><u></u><u></u></p></div></div><div><div><p class="MsoNormal"> <u></u><u></u></p><div><p class="MsoNormal">Hi Sam,<br><br>Thanks for working on this! This has been a running sore in ARM vectorization for a long time now.<u></u><u></u></p><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">Initial comments:<u></u><u></u></p></div><div><p class="MsoNormal">   * In the future, could you please use Phabricator (<a href="http://reviews.llvm.org" target="_blank">http://reviews.llvm.org</a>) to upload patches to; it makes it much easier to review.<u></u><u></u></p></div><div><p class="MsoNormal">   * There are no regression tests included in this patch - did you forget to git add them?<u></u><u></u></p></div><div><p class="MsoNormal">   * I'm not sure the logic is totally sound. Consider this:<u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">loop:<u></u><u></u></p></div><div><p class="MsoNormal">  %1 = load i8* %foo<u></u><u></u></p></div><div><p class="MsoNormal">  %2 = zext i8 %1 to i32<u></u><u></u></p></div><div><p class="MsoNormal">  %3 = add i32 %2, 42<u></u><u></u></p></div><div><p class="MsoNormal">  %4 = trunc i32 %3 to i16<u></u><u></u></p></div><div><p class="MsoNormal">  %5 = add i16 %4, 42<u></u><u></u></p></div><div><p class="MsoNormal">  store i16* %5, %bar<u></u><u></u></p></div><div><p class="MsoNormal">  br loop<u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">getWidestType() will return i16. So the first cast is not free, but neither is it a cast to i32, it's a cast to i16.<u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">I'm not exactly sure what Elena's query was; it looks like the implementation here should be architecture-agnostic as it's just modelling changes to the IR (truncate nodes disappear) and the rest is the TTI's responsibility.<u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">Cheers,<u></u><u></u></p></div><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">James<u></u><u></u></p></div></div><p class="MsoNormal"> <u></u><u></u></p><div><p class="MsoNormal">On Mon, 11 May 2015 at 12:17 Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:<u></u><u></u></p><div><div><p class="MsoNormal"><span style="color:#1f497d">+        if (Opcode == Instruction::Trunc) {</span><u></u><u></u></p><p class="MsoNormal"><span style="color:#1f497d">+          if (TTI->isTypeLegal(DstVecTy)) {</span><u></u><u></u></p><p class="MsoNormal"><span style="color:#1f497d">+            VecCost = 0;</span><u></u><u></u></p><p class="MsoNormal"><span style="color:#1f497d">+          }</span><u></u><u></u></p><p class="MsoNormal"><span style="color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="color:#1f497d">On AVX-512 the “truncate” is usually one instruction, the VecCost should be 1.</span><u></u><u></u></p><p class="MsoNormal"><span style="color:#1f497d">On AVX the type may be legal, but “truncate” is more than one instruction.</span><u></u><u></u></p><p class="MsoNormal"><span style="color:#1f497d"> </span><u></u><u></u></p><div><p class="MsoNormal" style="margin-left:36.0pt"><span style="color:#31849b">-</span><span style="font-size:7.0pt;color:#31849b">          </span><b><i><span style="color:#31849b"> Elena</span></i></b><u></u><u></u></p></div><p class="MsoNormal"><span style="color:#1f497d"> </span><u></u><u></u></p><div><div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0cm 0cm 0cm"><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>Sam Parker<br><b>Sent:</b> Monday, May 11, 2015 13:57<br><b>To:</b> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><b>Subject:</b> [PATCH] improve vectorizers by removing cost of unnecessary truncs and exts.</span><u></u><u></u></p></div></div></div></div><div><div><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal">Hi,<u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal">I’ve attached a patch to both the loop vectorizer and slp-vectorizer which checks to see whether truncs and extensions would actually be required if the code was vectorized. This is so that the vectorizers understand that the cost of these instructions is effectively zero if vectorization happens. This is helpful when working on smaller data types, such as i8 and i16, that do not have native support in general purpose registers, but are supported in vector register files.<u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal">Regards,<u></u><u></u></p><p class="MsoNormal">Sam<u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p></div></div><div><p>---------------------------------------------------------------------<br>Intel Israel (74) Limited<u></u><u></u></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.<u></u><u></u></p></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></div></div></div></div></blockquote></div></blockquote></div>