Page MenuHomePhabricator

GlobalISel: Change representation of shuffle masks
ClosedPublic

Authored by arsenm on Mon, Aug 12, 2:29 PM.

Details

Summary

Currently shufflemasks get emitted as any other constant, and you end
up with a bunch of virtual registers of G_CONSTANT with a
G_BUILD_VECTOR. The AArch64 selector then asserts on anything that
doesn't fit this pattern. This isn't an ideal representation, and
should avoid legalization and have fewer opportunities for a
representational error.

Rather than invent a new shuffle mask operand type, similar to what
ShuffleVectorSDNode does, just track the original IR Constant mask
operand. I don't completely like the idea of adding another link to
the IR, but MIR is already quite dependent on IR constants already,
and this will allow sharing the shuffle mask utility functions with
the IR.

Diff Detail

Event Timeline

arsenm created this revision.Mon, Aug 12, 2:29 PM

Thanks for doing this. Overall the patch LGTM. On a side note, should we try to document all the links we have to IR in GlobalISel somewhere?

Thanks for doing this. Overall the patch LGTM. On a side note, should we try to document all the links we have to IR in GlobalISel somewhere?

Not sure where that would go

aemerson accepted this revision.Mon, Aug 12, 11:26 PM

This looks reasonable to me. Thanks.

This revision is now accepted and ready to land.Mon, Aug 12, 11:26 PM

Thanks for doing this. Overall the patch LGTM. On a side note, should we try to document all the links we have to IR in GlobalISel somewhere?

Not sure where that would go

Perhaps in the bottom of https://www.llvm.org/docs/GlobalISel.html ? Alternatively we can document in GenericOpcodes.td right along the instructions which have links.

Thanks for doing this. Overall the patch LGTM. On a side note, should we try to document all the links we have to IR in GlobalISel somewhere?

Not sure where that would go

Perhaps in the bottom of https://www.llvm.org/docs/GlobalISel.html ? Alternatively we can document in GenericOpcodes.td right along the instructions which have links.

I added a note to GenericOpcodes.td

arsenm closed this revision.Tue, Aug 13, 8:33 AM

r368704