[PATCH] D73940: GlobalISel: Reimplement moreElementsVectorDst

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D73940#1862043 <https://reviews.llvm.org/D73940#1862043>, @Petar.Avramovic wrote:

> If not a new LegalizeAction maybe some other way  ( like one more argument ) for targets to chose whether to create G_EXTRACT or CONCAT_VECTORS + G_UNMERGE_VALUES ?
>  I am not that familiar with the problem we are dealing here, but shouldn't insert + extract that are created during moreElements combine away like anyext + trunc for widen scalar ?
>  So this would be `extract + insert into implicit def` -> `copy extract source` if it had same type like implicit def (this is 3 instruction combine). Might that fix some of the problems?


I think that this has the same fundamental problem of dealing with mismatched source and result types, while also having more options for irregularity. The advantage of this is how it composes with moreElementsVector on source operands. For sources, there's a bigger advantage from actually performing all operations on the requested type, rather than having a weird leftover piece with a different type with an irregular insert. A def and use changing to the same number of elements, or an common multiple between them, should be the common case which will fold out nicely with the unmarke

In D73940#1862043 <https://reviews.llvm.org/D73940#1862043>, @Petar.Avramovic wrote:

> If not a new LegalizeAction maybe some other way  ( like one more argument ) for targets to chose whether to create G_EXTRACT or CONCAT_VECTORS + G_UNMERGE_VALUES ?


I think this is not a place to add target customization. This is a core decision of how the legalizer deals with vectors. I've pretty much come to the conclusion that G_INSERT/G_EXTRACT are kind of terrible, and the generic legalizer should never produce them. They're too free-form to be really useful. You can't enforce many properties on them in the verifier, and anywhere you would want to do anything with them would require more complicated checks on the operands than the other legalization artifacts. It would generally be simpler to have the common properties implicit in the opcode.

In this particular situation, we should probably have an equivalent of the old ISD::EXTRACT_SUBVECTOR. The expansion for that would look like the code in this patch, creating the unmerge with padding operands. Your custom legalization could then choose to use a different expansion for G_EXTRACT_SUBVECTOR. I think the path forward here is to go ahead with this strategy as a first step. A further cleanup would be to introduce the new artifact, and then move this expansion into the lowering decision for it.


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

https://reviews.llvm.org/D73940