[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 a comment.

This looks good to me but i find naming choice really confusing here.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16420
 SDValue DAGCombiner::combineInsertEltToShuffle(SDNode *N, unsigned InsIndex) {
   SDValue InsertVal = N->getOperand(1);
+  SDValue Vec = N->getOperand(0);
----------------
Hm, please rename this to ExtractVal instead.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16423
+
+  // (insert_vector_elt (vector_shuffle X, Y), (extract_vector_elt X, N), IdxC)
+  //   --> (vector_shuffle X, Y)
----------------
Comment is wrong i think, should be
s/IdxC/InsIndex/


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:16447
+
+      auto *IndexC = cast<ConstantSDNode>(InsertVal.getOperand(1));
+      NewMask[InsIndex] = Offset + IndexC->getZExtValue();
----------------
ExtrIndex


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66718