[PATCH] D23897: [SelectionDAG] Generate vector_shuffle nodes for undersized result vector sizes

[Michael Kuperstein via llvm-commits]

mkuper added a comment.

Thanks, Sanjay!

Just to be clear - the other two alternatives I see are:

1. Construct (vector_shuffle <mask> (concat_vectors t1, t2), undef) in the SelectionDAG, and have a target-specific combine in the other direction.
2. Handle this directly in the shuffle lowering code, instead of a dag combine.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26342-26343
@@ -26341,1 +26341,4 @@
 
+// We are looking for a shuffle where both sources are vectors with a width
+// of half the output's width that were concatenated with undef. 
+// AVX2 has VPERMD/Q, so if we can express this a single-source shuffle, that's
----------------
spatel wrote:
> Hard to parse that sentence. Is this accurate/better?
> "We are looking for a shuffle where both sources are concatenated with undef and have a width that is half of the output's width."
Yes, thanks!

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26344-26348
@@ +26343,7 @@
+// of half the output's width that were concatenated with undef. 
+// AVX2 has VPERMD/Q, so if we can express this a single-source shuffle, that's
+// preferable.
+static SDValue combineShuffleOfConcatUndef(SDNode *N, SelectionDAG &DAG) {
+  EVT VT = N->getValueType(0);
+
+  // We only care about shuffles of 128/256-bit vectors of i32/i64.
----------------
spatel wrote:
> spatel wrote:
> > This function is specifically for AVX2, so I think the check that is currently outside of the function:
> >   if (Subtarget.hasInt256())
> > 
> > should be inserted at the start in here. Also, it should be "hasAVX2()" rather than "hasInt256()" because this is distinguishing AVX2-ness, not 256-bit-ness.
> this a -> this as a
Right, should have been hasAVX2(), thanks.

Regarding where the check should be - I'm not sure what's better, TBH. I'll move it into the function as you suggest, if anyone objects, let me know.


https://reviews.llvm.org/D23897