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

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13774
+  EVT ShufVT = EVT::getVectorVT(*DAG.getContext(), SubVecEltVT, NumMaskVals);
+  if (!DAG.getTargetLoweringInfo().isShuffleMaskLegal(Mask, ShufVT))
+    return SDValue();
----------------
RKSimon wrote:
> How bad does it get if we allow any shuffle prior to legalization?
I'm not sure about other targets because no existing tests change, but given that this isn't a clear win, I think we need some kind of hook to opt out. 

For AVX512, it gets Bad - as in hundreds of extra instructions.

It looks like we'd need to add a concat_vectors fold to prevent this pattern:
  t76: v128i1 = concat_vectors t13, undef:v16i1, undef:v16i1, undef:v16i1, undef:v16i1, undef:v16i1, undef:v16i1, undef:v16i1
  t77: v8i16 = bitcast t76
  t78: v8i16 = vector_shuffle<0,8,2,3,4,5,6,7> t80, t77

>From becoming:
                              ...t218: i8 = extract_vector_elt t13, Constant:i64<12>
                              t217: i8 = extract_vector_elt t13, Constant:i64<13>
                              t216: i8 = extract_vector_elt t13, Constant:i64<14>
                              t215: i8 = extract_vector_elt t13, Constant:i64<15>
                            t210: v32i8 = BUILD_VECTOR t230, t229, t228, t227, t226, t225, t224, t223, t222, t221, t220, t219, t218, t217, t216, t215, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8, undef:i8


https://reviews.llvm.org/D38388