[PATCH] D58136: GlobalISel: Implement moreElementsVector for g_insert results
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 13 10:09:08 PST 2019
arsenm marked an inline comment as done.
arsenm added inline comments.
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:2370
+ 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
The assert I added in r352636 should catch that, but I guess you could always call the LegalizerHelper directly
CHANGES SINCE LAST ACTION
More information about the llvm-commits