[PATCH] D66718: [DAGCombiner] (insert_vector_elt (vector_shuffle X, Y), (extract_vector_elt X, N), IdxC) -> (vector_shuffle X, Y)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16441
+    } else if (InsertVal.getOperand(0) == SV1) {
+      Offset = Mask.size();
+    }
----------------
RKSimon wrote:
> lebedev.ri wrote:
> > Actually, this is not correct.
> > While both shuffle inputs must have same types,
> > the shuffle mask can have different dimensions.
> > So this should be closer to `Offset = X.getType().getVectorNumElements()`.
> > 
> > https://llvm.org/docs/LangRef.html#shufflevector-instruction
> That is only true for IR - in the DAG, the result must have the same type as the shuffle operands - so the number of mask elts must match the number of result/operand elts - see SelectionDAG::getVectorShuffle
Oh, forgot about that, sorry. An assert later on will be fine then.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66718/new/

https://reviews.llvm.org/D66718