[PATCH] D15477: [X86][AVX] Only shuffle the lower half of vectors if the upper half is undefined

[Simon Pilgrim via llvm-commits]

RKSimon added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:11289
@@ +11288,3 @@
+  // FIXME: Add 512-bit vector support.
+  if (VT.is256BitVector()) {
+    unsigned HalfNumElts = VT.getVectorNumElements() / 2;
----------------
delena wrote:
> Could, you, please take it into a static function?
> May be call it from lower256BitVectorShuffle ?
> 
No problem.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:11314
@@ +11313,3 @@
+    // vector_shuffle <X, X, X, X, u, u, u, u> or <X, X, u, u>
+    if (UndefUpper) {
+      SDValue Half1, Half2;
----------------
delena wrote:
> Why do you check only UndefUpper? What about UndefLower?
OK - I can add this - it will still initially just support shuffling with the lower half vectors is that OK?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:11329
@@ +11328,3 @@
+        SDValue V = (M >= (int)NumElements ? V2 : V1);
+        V = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, HalfVT, V,
+                        DAG.getIntPtrConstant(0, dl));
----------------
delena wrote:
> I don't understand this code. You are running inside loop.  for (unsigned i = 0; i != HalfNumElts; ++i)
> for v32i8 you have 16 iterations. Do you create EXTRACT_SUBVECTOR 16 times?
The DAG.getNode logic will find a equivalent node if it already exists in the DAG (search for FindNodeOrInsertPos) so although we call getNode(ISD::EXTRACT_SUBVECTOR, ...) upto HalfNumElts times, it will return at most 2 values on success - on fail case it will return a 3rd value which will then fail to match either of the Half variables.

But I can see it won't be clear, and slower then necessary, so I'll replace it with an integer index approach.


Repository:
  rL LLVM

http://reviews.llvm.org/D15477