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

[Matt Arsenault via Phabricator via llvm-commits]

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


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:2370
+    Observer.changingInstr(MI);
+    moreElementsVectorSrc(MI, MoreTy, 1);
+    moreElementsVectorDst(MI, MoreTy, 0);
----------------
paquette wrote:
> 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
The assert I added in r352636 should catch that, but I guess you could always call the LegalizerHelper directly


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

https://reviews.llvm.org/D58136