<div dir="ltr"><div><div>Hi Bruno,<br><br></div>I went ahead and committed revision 250387. I merged test pr24562.ll into the existing test x86-fold-pshufb.ll.<br><br></div>-Andrea<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 15, 2015 at 10:02 AM, Andrea Di Biagio <span dir="ltr"><<a href="mailto:andrea.dibiagio@gmail.com" target="_blank">andrea.dibiagio@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Bruno,<br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Thu, Oct 15, 2015 at 12:43 AM, Bruno Cardoso Lopes via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Andrea,<br>
<br>
Nice catch!<br>
<div><div><br>
On Mon, Oct 12, 2015 at 4:25 AM, Andrea Di Biagio via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: adibiagio<br>
> Date: Mon Oct 12 06:25:41 2015<br>
> New Revision: 250027<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=250027&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=250027&view=rev</a><br>
> Log:<br>
> [x86] PR24562: fix incorrect folding of PSHUFB nodes with a mask where all indices have the most significant bit set.<br>
><br>
> This patch fixes a problem in function 'combineX86ShuffleChain' that causes a<br>
> chain of shuffles to be wrongly folded away when the combined shuffle mask has<br>
> only one element.<br>
><br>
> We may end up with a combined shuffle mask of one element as a result of<br>
> multiple calls to function 'canWidenShuffleElements()'.<br>
> Function canWidenShuffleElements attempts to simplify a shuffle mask by widening<br>
> the size of the elements being shuffled.<br>
> For every pair of shuffle indices, function canWidenShuffleElements checks if<br>
> indices refer to adjacent elements. If all pairs refer to "adjacent" elements<br>
> then the shuffle mask is safely widened. As a consequence of widening, we end up<br>
> with a new shuffle mask which is half the size of the original shuffle mask.<br>
><br>
> The byte shuffle (pshufb) from test pr24562.ll has a mask of all SM_SentinelZero<br>
> indices. Function canWidenShuffleElements would combine each pair of<br>
> SM_SentinelZero indices into a single SM_SentinelZero index. So, in a<br>
> logarithmic number of steps (4 in this case), the pshufb mask is simplified to<br>
> a mask with only one index which is equal to SM_SentinelZero.<br>
><br>
> Before this patch, function combineX86ShuffleChain wrongly assumed that a mask<br>
> of size one is always equivalent to an identity mask. So, the entire shuffle<br>
> chain was just folded away as the combined shuffle mask was treated as a no-op<br>
> mask.<br>
><br>
> With this patch we know check if the only element of a combined shuffle mask is<br>
> SM_SentinelZero. In case, we propagate a zero vector.<br>
><br>
> Differential Revision: <a href="http://reviews.llvm.org/D13364" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13364</a><br>
><br>
> Added:<br>
>     llvm/trunk/test/CodeGen/X86/pr24562.ll<br>
<br>
</div></div>Any reason why not to add this test to the<br>
appropriate-already-existent shuffle test file instead?<br></blockquote><div><br></div></div></div><div>Hi Bruno,<br><br></div><div>There is no particular reason to be honest.<br></div><div>Probably, the most 'appropriate-already-existent' test for pshufb would be 'x86-fold-pshufb.ll'. The other existing test is 'ssse3-intrinsics-x86.ll' but I personally tink that x86-fold-pshufb.ll is more appropriate. I'll move the test there if okay for you.<br></div><div><br></div><div>Cheers,<br></div><div>Andrea<br> <br></div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
<span><font color="#888888"><br>
--<br>
Bruno Cardoso Lopes<br>
<a href="http://www.brunocardoso.cc" rel="noreferrer" target="_blank">http://www.brunocardoso.cc</a><br>
</font></span><div><div>_______________________________________________<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>
</div></div></blockquote></span></div><br></div></div>
</blockquote></div><br></div>