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

[Michael Kuperstein via Phabricator via llvm-commits]

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
 entry:
   ; 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.


Repository:
  rL LLVM

https://reviews.llvm.org/D27787