[PATCH] D27787: Restrict some DAGCombines for SHUFFLE_VECTOR nodes.

Michael Kuperstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 14 17:32:54 PST 2016

mkuper added inline comments.

Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14099
   SDValue N1 = SVN->getOperand(1);
+  if (!isAnyConstantBuildVector(N0.getNode()))
Just to make sure I understand - the reason you're removing the hasOneUse constraint is that for the case where the BUILD_VECTOR ends up a constant pool load, it's better to just load the combined vector instead of shuffling even if other users exist?

Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14361
     ShuffleVectorSDNode *OtherSV = cast<ShuffleVectorSDNode>(N0);
+    if (OtherSV->isSplat())
+      return SDValue();
Is there a reason this has to land in the same patch as the other change?
Also, could you explain why you consider splats free? On x86, at least, I don't see why that would be the case.

Comment at: test/CodeGen/ARM/vzip.ll:270
   ; CHECK-LABEL: vzip_lower_shufflemask_undef
   ; CHECK: vzip
Regenerate this with the update script?

Comment at: test/CodeGen/X86/mmx-bitcast.ll:83
 ; CHECK-NEXT:    punpckldq {{.*#+}} xmm1 = xmm1[0],xmm0[0],xmm1[1],xmm0[1]
-; CHECK-NEXT:    movd %xmm1, %rax
+; CHECK-NEXT:    pshufd {{.*#+}} xmm0 = xmm1[0,1,1,3]
+; CHECK-NEXT:    movd %xmm0, %rax
This looks like a common enough pattern that I'm not excited about regressing it at the expense of the "zip" example.

Comment at: test/CodeGen/X86/vector-shuffle-128-v16.ll:670
 ; SSE:       # BB#0:
-; SSE-NEXT:    movzbl %dil, %eax
+; SSE-NEXT:    movd %edi, %xmm1
+; SSE-NEXT:    movl $255, %eax
Same as above.



More information about the llvm-commits mailing list