<div dir="ltr"><div><div>Hi Zvi (and Sanjay),<br><br></div>There is a problem with this patch. <br>Checking for `isSplat()` is not enough, we also need to check if the splat mask contains undef indices. Otherwise, we risk to incorrectly propagate undef to users of the folded splat.<br><br></div><div>Consider the following example:<br><br><pre class="gmail-bz_comment_text" id="gmail-comment_text_3">t49: v4i32 = vector_shuffle<0,2,u,u> t58, undef:v4i32
t58: v4i32 = vector_shuffle<2,u,2,u> t13, undef:v4i32
<br><font face="arial,helvetica,sans-serif">A shuffle with interleaved `undef` indices is often introduced by the type legalizer when promoting from an illegal vector type to another types with same number of elements but wider element type.<br><br>In this case, the lower half of t49 is defined, while the upper half of t49 is undef.<br>This patch incorrectly simplifies that shuffle as follows:<br></font><br>t58: v4i32 = vector_shuffle<2,u,2,u> t13, undef:v4i32
<font face="arial,helvetica,sans-serif"><br>This is problematic in the following case:<br></font><br><span style="font-family:monospace,monospace">t51: i64 = extract_vector_elt t50, Constant:i64<0><br> t50: v2i64 = bitcast t49<br></span><font face="arial,helvetica,sans-serif"><span style="font-family:monospace,monospace"> t49: v4i32 = vector_shuffle<0,2,u,u> t58, undef:v4i32
t58: v4i32 = vector_shuffle<2,u,2,u> t13, undef:v4i32</span>
<br></font></pre><pre class="gmail-bz_comment_text" id="gmail-comment_text_3"><font face="arial,helvetica,sans-serif">This patch makes the upper half of t51 undefined. However, in the original code, the entire t51 was defined.<br>Here is another interesting case:<br></font><br><span style="font-family:monospace,monospace">t51: i32 = extract_vector_elt t49, Constant:i64<0><br>t50: i32 = </span><span style="font-family:monospace,monospace">extract_vector_elt t49, Constant:i64<1><br></span><font face="arial,helvetica,sans-serif"><span style="font-family:monospace,monospace"> t49: v4i32 = vector_shuffle<0,2,u,u> t58, undef:v4i32
t58: v4i32 = vector_shuffle<2,u,2,u> t13, undef:v4i32</span>
</font></pre><pre class="gmail-bz_comment_text" id="gmail-comment_text_3"><font face="arial,helvetica,sans-serif"><br></font></pre><pre class="gmail-bz_comment_text" id="gmail-comment_text_3"><font face="arial,helvetica,sans-serif">Before this patch, both t50 and t51 were defined, and the extracted value was the same. <br>With this patch, we now have this:<br></font><span style="font-family:monospace,monospace"><br>t51: i32 = extract_vector_elt t58, Constant:i64<0><br>t50: i32 = </span><span style="font-family:monospace,monospace">extract_vector_elt t58, Constant:i64<1><br></span><font face="arial,helvetica,sans-serif"><span style="font-family:monospace,monospace"> t58: v4i32 = vector_shuffle<2,u,2,u> t13, undef:v4i32</span>
</font><font face="arial,helvetica,sans-serif"><br>Here, t50 is undef and can be folded away.<br><br></font></pre><pre class="gmail-bz_comment_text" id="gmail-comment_text_3"><font face="arial,helvetica,sans-serif">Here is a small reproducible:<br><br>// ===========================================================================<br>extern "C" int printf(const char*, ...);<br><br>using v2 = unsigned int __attribute__((__vector_size__(8)));<br>using v4 = unsigned int __attribute__((__vector_size__(16)));<br><br>int main() {<br> v4 alpha = (v4){1, 2, 3, 4};<br> v2 bravo = __builtin_shufflevector(alpha, alpha, 2, 2);<br> printf("<%x %x>\n", bravo[0], bravo[1]);<br>}<br>// ===========================================================================<br></font></pre><pre class="gmail-bz_comment_text" id="gmail-comment_text_3"><font face="arial,helvetica,sans-serif">At runtime, the executable built at -O0 (-target x86_64-unknown-linux-gnu) prints out: <3 4><br></font></pre><pre class="gmail-bz_comment_text" id="gmail-comment_text_3"><font face="arial,helvetica,sans-serif">At -O2, it would print <3 3> (this is correct).<br><br><br></font></pre><pre class="gmail-bz_comment_text" id="gmail-comment_text_3"><font face="arial,helvetica,sans-serif">-Andrea<br></font></pre></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 30, 2017 at 2:42 AM, Zvi Rackover 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">Author: zvi<br>
Date: Wed Mar 29 20:42:57 2017<br>
New Revision: 299047<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=299047&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=299047&view=rev</a><br>
Log:<br>
[DAGCombine] A shuffle of a splat is always the splat itself<br>
<br>
Summary:<br>
Add a simplification:<br>
shuffle (splat-shuffle), undef, M --> splat-shuffle<br>
<br>
Fixes pr32449<br>
<br>
Patch by Sanjay Patel<br>
<br>
Reviewers: eli.friedman, RKSimon, spatel<br>
<br>
Reviewed By: spatel<br>
<br>
Subscribers: llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D31426" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D31426</a><br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp<br>
llvm/trunk/test/CodeGen/X86/<wbr>shuffle-of-splat-multiuses.ll<br>
<br>
Modified: llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=299047&r1=299046&r2=299047&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>CodeGen/SelectionDAG/<wbr>DAGCombiner.cpp?rev=299047&r1=<wbr>299046&r2=299047&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/<wbr>SelectionDAG/DAGCombiner.cpp Wed Mar 29 20:42:57 2017<br>
@@ -14643,6 +14643,12 @@ SDValue DAGCombiner::visitVECTOR_<wbr>SHUFFLE<br>
return DAG.getVectorShuffle(VT, SDLoc(N), N0, N1, NewMask);<br>
}<br>
<br>
+ // A shuffle of a splat is always the splat itself:<br>
+ // shuffle (splat-shuffle), undef, M --> splat-shuffle<br>
+ if (auto *N0Shuf = dyn_cast<ShuffleVectorSDNode>(<wbr>N0))<br>
+ if (N1.isUndef() && N0Shuf->isSplat())<br>
+ return N0;<br>
+<br>
// If it is a splat, check if the argument vector is another splat or a<br>
// build_vector.<br>
if (SVN->isSplat() && SVN->getSplatIndex() < (int)NumElts) {<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/<wbr>shuffle-of-splat-multiuses.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/shuffle-of-splat-multiuses.ll?rev=299047&r1=299046&r2=299047&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>CodeGen/X86/shuffle-of-splat-<wbr>multiuses.ll?rev=299047&r1=<wbr>299046&r2=299047&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/CodeGen/X86/<wbr>shuffle-of-splat-multiuses.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/<wbr>shuffle-of-splat-multiuses.ll Wed Mar 29 20:42:57 2017<br>
@@ -5,9 +5,8 @@<br>
define <2 x double> @foo2(<2 x double> %v, <2 x double> *%p) nounwind {<br>
; AVX2-LABEL: foo2:<br>
; AVX2: # BB#0:<br>
-; AVX2-NEXT: vpermilpd {{.*#+}} xmm1 = xmm0[1,1]<br>
-; AVX2-NEXT: vpermilpd {{.*#+}} xmm0 = xmm1[1,0]<br>
-; AVX2-NEXT: vmovapd %xmm1, (%rdi)<br>
+; AVX2-NEXT: vpermilpd {{.*#+}} xmm0 = xmm0[1,1]<br>
+; AVX2-NEXT: vmovapd %xmm0, (%rdi)<br>
; AVX2-NEXT: retq<br>
%res = shufflevector <2 x double> %v, <2 x double> undef, <2 x i32> <i32 1, i32 1><br>
%res1 = shufflevector<2 x double> %res, <2 x double> undef, <2 x i32> <i32 1, i32 undef><br>
@@ -18,9 +17,8 @@ define <2 x double> @foo2(<2 x double> %<br>
define <4 x double> @foo4(<4 x double> %v, <4 x double> *%p) nounwind {<br>
; AVX2-LABEL: foo4:<br>
; AVX2: # BB#0:<br>
-; AVX2-NEXT: vpermpd {{.*#+}} ymm1 = ymm0[2,2,2,2]<br>
-; AVX2-NEXT: vpermpd {{.*#+}} ymm0 = ymm1[2,0,2,3]<br>
-; AVX2-NEXT: vmovapd %ymm1, (%rdi)<br>
+; AVX2-NEXT: vpermpd {{.*#+}} ymm0 = ymm0[2,2,2,2]<br>
+; AVX2-NEXT: vmovapd %ymm0, (%rdi)<br>
; AVX2-NEXT: retq<br>
%res = shufflevector <4 x double> %v, <4 x double> undef, <4 x i32> <i32 2, i32 2, i32 2, i32 2><br>
%res1 = shufflevector<4 x double> %res, <4 x double> undef, <4 x i32> <i32 2, i32 0, i32 undef, i32 undef><br>
@@ -32,10 +30,8 @@ define <8 x float> @foo8(<8 x float> %v,<br>
; AVX2-LABEL: foo8:<br>
; AVX2: # BB#0:<br>
; AVX2-NEXT: vmovshdup {{.*#+}} ymm0 = ymm0[1,1,3,3,5,5,7,7]<br>
-; AVX2-NEXT: vpermpd {{.*#+}} ymm1 = ymm0[2,2,2,2]<br>
-; AVX2-NEXT: vmovaps {{.*#+}} ymm0 = <2,0,u,u,5,1,3,7><br>
-; AVX2-NEXT: vpermps %ymm1, %ymm0, %ymm0<br>
-; AVX2-NEXT: vmovapd %ymm1, (%rdi)<br>
+; AVX2-NEXT: vpermpd {{.*#+}} ymm0 = ymm0[2,2,2,2]<br>
+; AVX2-NEXT: vmovapd %ymm0, (%rdi)<br>
; AVX2-NEXT: retq<br>
%res = shufflevector <8 x float> %v, <8 x float> undef, <8 x i32> <i32 5, i32 5, i32 5, i32 5, i32 5, i32 5, i32 5, i32 5><br>
%res1 = shufflevector<8 x float> %res, <8 x float> undef, <8 x i32> <i32 2, i32 0, i32 undef, i32 undef, i32 5, i32 1, i32 3, i32 7><br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">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/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>