This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add support for {vex2}, {vex3}, and {evex} to the assembler to match gas. Use {evex} to improve the one our 32-bit AVX512 tests.
ClosedPublic

Authored by craig.topper on Mar 12 2019, 10:34 AM.

Details

Summary

These can be used to force the encoding used for instructions.

{vex2} will fail if the instruction is not VEX encoded, but otherwise won't do anything since we prefer vex2 when possible. Might need to skip use of the _REV MOV instructions for this too, but I haven't done that yet.

{vex3} will force the instruction to use the 3 byte VEX encoding or fail if there is no VEX form.

{evex} will force the instruction to use the EVEX version or fail if there is no EVEX version.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Mar 12 2019, 10:34 AM

Might need to skip use of the _REV MOV instructions for this too, but I haven't done that yet.

Can you elaborate?

Might need to skip use of the _REV MOV instructions for this too, but I haven't done that yet.

Can you elaborate?

We have aliases like this which look for xmm8-15 and xmm0-7 being used in the wrong spots of the modrm byte requiring the 3 byte vex prefix to be used. So we switch to the otherwise redundant move encoding that has the operands swapped in modrm. This enables a 2 byte vex prefix. gcc has the same optimization, but seems to disable it when {vex3} is specified.

// Aliases to help the assembler pick two byte VEX encodings by swapping the
// operands relative to the normal instructions to use VEX.R instead of VEX.B.
def : InstAlias<"vmovdqa\t{$src, $dst|$dst, $src}",
                (VMOVDQArr_REV VR128L:$dst, VR128H:$src), 0>;
def : InstAlias<"vmovdqa\t{$src, $dst|$dst, $src}",
                (VMOVDQAYrr_REV VR256L:$dst, VR256H:$src), 0>;
def : InstAlias<"vmovdqu\t{$src, $dst|$dst, $src}",
                (VMOVDQUrr_REV VR128L:$dst, VR128H:$src), 0>;
def : InstAlias<"vmovdqu\t{$src, $dst|$dst, $src}",
                (VMOVDQUYrr_REV VR256L:$dst, VR256H:$src), 0>;

I forgot about another issue I know about. Currently the EVEX form of instruction like "vcvtss2si (%rax), %ebx" are removed from the assembly matcher table. Since none of the operands are xmm/ymm/zmm registers when the source is memory there was never a reason to use EVEX. And the sorting criteria in the AsmMatcherTable can't order it correctly to put VEX first since the operands are identical. Normally the fact that VR128 is a subclass of VR128X is what give VEX preference over EVEX in the table. But that doesn't apply here so they get sorted by the enum value from X86GenInstrInfo.inc which puts EVEX first.

With this feature, we should pick the EVEX encoding when {evex} is specified. Not sure how to make this work other than rejecting the EVEX form for a specific list of instructions in checkTargetMatchPredicate unless {evex} has been specified.

Ping. Should I address those other issues or is this a good starting point?

RKSimon accepted this revision.Apr 9 2019, 8:25 AM

Couple of minors but I think this is a good starting point - please make sure the issues you mentioned are included as TODOs somewhere or raised as bugs

lib/Target/X86/AsmParser/X86AsmParser.cpp
3140 ↗(On Diff #190291)

missing assert message

test/MC/X86/AVX-32.s
9 ↗(On Diff #190291)

I take it you will be adding full vex3 testing as a separate (bulky) commit?

This revision is now accepted and ready to land.Apr 9 2019, 8:25 AM
craig.topper marked an inline comment as done.Apr 9 2019, 11:39 AM
craig.topper added inline comments.
test/MC/X86/AVX-32.s
9 ↗(On Diff #190291)

Maybe if I can get my hands on the script that generated these tests. Updating the scalar test was painful. Also need to figure out what happened to the effort to get the rest of the reviews committed.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2019, 11:43 AM