<div dir="ltr"><div class="gmail_extra">High level comment, using Phabricator would make reviewing this *much* easier for me.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 16, 2015 at 1:57 PM, Fiona Glaser <span dir="ltr"><<a href="mailto:fglaser@apple.com" target="_blank">fglaser@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><div>> 1. Run your patch through clang-format please. The patch does not follow the LLVM formatting guidelines.</div><div><br></div></span><div>Done, and changed per Mehdi’s suggestions.</div><span class=""><div><br></div><div>> 2. What is the impact of this on arm64 and armv7s generated code? Although the approach makes sense to me, I want to be sure we do not degrade other targets. Note that I do not expect you to run tests if you cannot :).</div><div><br></div></span><div>I don’t think it should even affect any target that doesn’t have canonical vector sizes of both N and 2*N, for a data type of N/2 or smaller. Otherwise the case this patch targets can’t come up.</div><div><span class=""><br><blockquote type="cite"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">3. What are the runtime performance impact on x86_64, with and without -mavx2?<br></blockquote></div></blockquote><div><br></div></span><div>I’m not sure in general; this affects a few very specific vector constructs that were being pessimized.</div><span class=""><br><blockquote type="cite"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">4. Add a run line for avx  too:<br>+; RUN: llc < %s -march=x86-64 -mattr=+avx2 | FileCheck %s<br></blockquote></div></blockquote><div><br></div></span><div>Done.</div><span class=""><div><br></div><blockquote type="cite"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">5. Should we use a triple instead of march?</blockquote></div></blockquote><div><br></div></span>Do triples let you specify particular attributes (AVX, etc?)</div></blockquote></div><br>Please use the same tetsing strategy as the vector-shuffle-{128,256,512}-*.ll test files. This has made it *much* easier to understand the feature sets tested and how we generate code for each operation.</div></div>