[PATCH] D58136: GlobalISel: Implement moreElementsVector for g_insert results

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:2370
+    Observer.changingInstr(MI);
+    moreElementsVectorSrc(MI, MoreTy, 1);
+    moreElementsVectorDst(MI, MoreTy, 0);
----------------
arsenm wrote:
> arsenm wrote:
> > paquette wrote:
> > > Should we be asserting that the types are vectors for these? Or does it not matter?
> > These seem to allow any types.
> > 
> > One thing I'm sort of thinking about for some vector legalizations is if this is OK or sufficient.
> My current feeling is there is something wrong with the current set of legalization primitives for vectors, but I haven't tried to come up with a coherent description of what yet. The fact that the insert can clobber multiple vector elements seems difficult to deal with. I almost think we need a version of g_insert that handles complete vector elements, and then maybe some kind of elementwise version (where the bit offset is into each vector element) but I haven't fully thought this out yet.
> These seem to allow any types.

Oh, I meant that if we're calling `moreElementsVectorSrc`, we should probably assert that we're legalizing a vector source. That function seems to have the assumption that the type we're giving it a vector type.

> My current feeling is there is something wrong with the current set of legalization primitives for vectors

I'll need to think about that a bit


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

https://reviews.llvm.org/D58136