Page MenuHomePhabricator

[X86][XOP] Support for VPPERM shuffle mask decoding

Authored by RKSimon on Mar 24 2016, 5:11 AM.



This patch adds support for decoding XOP VPPERM instruction when it represents a basic shuffle.

The mask decoding required the existing MCInstrLowering code to be updated to support binary shuffles - the implementation now matches what is done in X86InstrComments.cpp. This should be useful for some AVX512 binary shuffles (VPERMT2 etc.) as well.

Split off from D18189

Diff Detail


Event Timeline

RKSimon updated this revision to Diff 51541.Mar 24 2016, 5:11 AM
RKSimon retitled this revision from to [X86][XOP] Support for VPPERM shuffle mask decoding.
RKSimon updated this object.
RKSimon added reviewers: spatel, silvas, andreadb, delena.
RKSimon set the repository for this revision to rL LLVM.
RKSimon added a subscriber: llvm-commits.
spatel accepted this revision.Apr 6 2016, 2:27 PM
spatel edited edge metadata.

LGTM - see inline comments for nits.

1042 ↗(On Diff #51541)

Add a comment here to explain. Something like: if there's really only one source operand, fix the mask so we can print all elements in one span.

1044–1046 ↗(On Diff #51541)

The >=0 check is redundant?

180 ↗(On Diff #51541)

warning: comparison of signed and unsigned

203 ↗(On Diff #51541)

"APElt" made me think I was seeing a new type rather than a variable. "MaskElt"?

35 ↗(On Diff #51541)

auto brief is on, so don't need to use \brief explicitly.
I know it doesn't match anything else here. Separate cleanup commit?

36 ↗(On Diff #51541)

Decode -> decode.
I know it doesn't match anything else again. Fix all of these function names to be lowercase in a separate commit?

24–31 ↗(On Diff #51541)

Is there a way to test the scaled (element != i8) mask decoder path?

This revision is now accepted and ready to land.Apr 6 2016, 2:27 PM
This revision was automatically updated to reflect the committed changes.