[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);
+}
----------------
spatel wrote:
> 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.
Note: option #3 isn't as crazy as I made it sound, that 'fake type' is ConcatVT in the current patch.


https://reviews.llvm.org/D38388