[PATCH] D107068: [DAG] DAGCombiner::visitVECTOR_SHUFFLE - recognise INSERT_SUBVECTOR patterns.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D107068#2922316 <https://reviews.llvm.org/D107068#2922316>, @RKSimon wrote:

> In D107068#2922214 <https://reviews.llvm.org/D107068#2922214>, @lebedev.ri wrote:
>
>> What about insertion of multiple subvectors?
>
> Do you have an example? visitINSERT_SUBVECTOR already does some canonicalizations and can fold to CONCAT_VECTORS

I mean something along the lines of

  // e.g. v2i32 into v8i32:
  // shuffle(lhs,concat(rhs0,rhs1,rhs2,rhs3),0,1,2,3,10,11,12,13).
  // --> insert_subvector(insert_subvector(lhs,rhs1,4),rhs2,6).

and

  // e.g. v2i32 into v8i32:
  // shuffle(lhs,concat(rhs0,rhs1,rhs2,rhs3),8,9,2,3,4,5,12,13).
  // --> insert_subvector(insert_subvector(lhs,rhs0,0),rhs2,6).

I haven't checked what happens currently, i only saw that the patch only handles a single insertion, iff the shuffle goes away.

>> This looks complicated. Can't we do this simpler?
>> If we want to keep the current general logic, can't you just generate a shuffle mask that you'd need when inserting n'th subvector of RHS into identity LHS, and compare that the mask matches the actual mask? (modulo `undef`s)
>
> I'll look at this - but an earlier prototype tried something similar and it ended up being rather messy.





================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:21308-21310
+      // Don't bother if we have a unary shuffle.
+      if (all_of(Mask, [NumElts](int M) { return M < (int)NumElts; }))
+        return SDValue();
----------------
+` Undef's are okay.`

I believe we can't have `-2` (zero) mask elements yet here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107068