[PATCH] D38388: [DAGCombiner, x86] convert insertelement of bitcasted vector into shuffle

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13787
+  // Step 3: Shuffle in the padded subvector.
+  return DAG.getVectorShuffle(VT, DL, DestVec, PaddedSubVec, ShufMaskVals);
+}
----------------
RKSimon wrote:
> We need to test for isShuffleMaskLegal here and I think it will remove the need for shouldConvertInsSubVecToShuffle
Aha...I thought we must have something here already. I was trying something similar to that in an earlier draft, but it's tricky:

1. If we use the actual shuffle type (VT), the subvector that we're trying to avoid messing with will already be cast from vXi1 to a potentially legal mask type. AVX512 disaster will occur for existing test cases.

2. If we use the subvector type (SubVecVT), we will fail to get any of the cases shown here because the subvector type isn't legal (eg, v2i32). This is similar to why I couldn't use insert_subvector nodes - they require legal types.

3. If we hack it to check for a hybrid fake type (the number of elements in the result + the element type of the subvector), we won't get the 512-bit cases for the AVX2 targets. This probably doesn't matter much in practice (shouldn't have larger than legal vectors in the first place?).

So it's not quite what we're looking for, but I can post a draft of option #3 if that's a reasonable alternative to a new hook. Or let me know if you see another way out.


https://reviews.llvm.org/D38388