[PATCH] D72361: [DAGCombiner] reduce extract subvector of concat

[Sanjay Patel via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:18591
+    //   v2i8 extract_subvec (v16i8 concat (v8i8 X), (v8i8 Y), 14 -->
+    //   v2i8 extract_subvec v8i8 Y, 6
+    if (ConcatSrcNumElts > ExtNumElts) {
----------------
lebedev.ri wrote:
> I'm having a hard time parsing this.
> 
> Did we ensure that the original extract only extracts
> only from one single concatenated vector?
> 
> I.e. that it does not take a new elements from
> the end of first concatenated vector, and a few elements
> from the beginning of second concatenated vector?
> (That case could be implemented as a shuffle if we concatenating the same vector)
> 
I think this goes back to the assert mentioned above for extract_subvector. It's not legal to have something like this:
v2i8 extract_subvec (v16i8 X), 7

Because the index operand must be a multiple of the final vector length (7 % 2 != 0).
So that means it is impossible to extract from more than 1 operand of the concat (because the concat operands are guaranteed to be longer than this extract result).

I can add the above as a code comment if it helps. We could also assert that ExtNumElts % NewExtIdx == 0?


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

https://reviews.llvm.org/D72361