Page MenuHomePhabricator

[X86][AVX512F_SCALAR]: Adding full coverage of MC encoding for the AVX512F_SCALAR isa sets.<NFC>
ClosedPublic

Authored by craig.topper on Dec 13 2017, 5:07 AM.

Details

Summary

NFC.
Adding MC regressions tests to cover the AVX512F_SCALAR isa sets.
This patch is part of a larger task to cover MC encoding of all X86 isa sets started in revision: https://reviews.llvm.org/D39952

Diff Detail

Repository
rL LLVM

Event Timeline

gadi.haber created this revision.Dec 13 2017, 5:07 AM
This revision is now accepted and ready to land.Dec 13 2017, 1:29 PM
craig.topper requested changes to this revision.Dec 13 2017, 2:05 PM
craig.topper added inline comments.
test/MC/X86/AVX512F_SCALAR-32.s
4 ↗(On Diff #126734)

This is a VEX encoding so this is testing AVX1. Not sure how to trick 32-bit mode to use EVEX for the unmasked instructions since you can't use the extended registers.

This revision now requires changes to proceed.Dec 13 2017, 2:05 PM
craig.topper added inline comments.Dec 20 2018, 11:39 AM
test/MC/X86/AVX512F_SCALAR-32.s
4 ↗(On Diff #126734)

I don't know how to fix this without introducing support for the {evex} pseudo prefix that gas supports. But that's a bigger effort which I'm not even sure how to implement in the assembler implementation.

craig.topper commandeered this revision.Dec 20 2018, 12:26 PM
craig.topper edited reviewers, added: gadi.haber; removed: craig.topper.

Rebase. I had to fix the fact that we now parse vmovss.s/vmovsd.s as a VEX instruction when possible. We also had some change involving vcvtss2si and similar instructions that were picking EVEX encodings but now use VEX.

craig.topper accepted this revision.Jan 22 2019, 11:46 AM

This is an definitely a coverage improvement so I'm going to approve so we can move forward with these encoding coverage patches.

This revision is now accepted and ready to land.Jan 22 2019, 11:46 AM
RKSimon accepted this revision.Jan 22 2019, 12:23 PM

LGTM! Although if you could add a FIXME comment about the VEX vs EVEX encoding that'd be useful

This revision was automatically updated to reflect the committed changes.