[PATCH] D40209: [DAGCombiner] eliminate shuffle of insert element

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15737
+  for (int i = 0; i != MaskSize; ++i) {
+    if (Mask[i] >= 0 && Mask[i] < MaskSize) {
+      // We're looking for a shuffle of exactly one element from operand 0.
----------------
efriedma wrote:
> Do we need to handle the case where `Mask[i] < 0`?
I was going to postpone that to a follow-up because I know we can get into some gray areas with undefs in shuffle masks. Ie, is it better to preserve the undef-ness of some vector lanes or eliminate an op but then have all lanes defined?

Ok, if I add a TODO in this patch?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15778
+         "Shuffle mask value must be from operand 0");
+  if (Op0.getOpcode() != ISD::INSERT_VECTOR_ELT)
+    return SDValue();
----------------
efriedma wrote:
> I guess there's sort of a family of related transforms here... does it make sense to optimize other cases where we know the value?  A SCALAR_TO_VECTOR?  A constant?  A BUILD_VECTOR?  Another shuffle we could examine recursively?  (Not that you need to implement any of that right now, but it would make sense to note the direction you expect for future generalizations.)
Good point - I haven't seen the other patterns in real code, but it's worth noting here.


https://reviews.llvm.org/D40209