This is an archive of the discontinued LLVM Phabricator instance.

[X86][AVX512] Adding new LLVM TableGen backend which generates the EVEX2VEX compressing tables.
ClosedPublic

Authored by aymanmus on Feb 28 2017, 2:13 AM.

Details

Summary

X86EvexToVex machine instruction pass compresses EVEX encoded instructions by replacing them with their identical VEX encoded instructions when possible.
It uses manually supported 2 large tables that map the EVEX instructions to their VEX ideticals.
This TableGen backend replaces the tables by automatically generating them.

Diff Detail

Repository
rL LLVM

Event Timeline

aymanmus created this revision.Feb 28 2017, 2:13 AM
craig.topper edited edge metadata.Feb 28 2017, 8:59 PM

Should we fix the deficiencies in the current tables first so that there are no test changes?

utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
91 ↗(On Diff #89991)

introduction*

92 ↗(On Diff #89991)

maximize*

craig.topper added inline comments.Feb 28 2017, 9:44 PM
utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
68 ↗(On Diff #89991)

Why is this a vector and not just a static table of strings?

Can you add a comment indicating what each of these corresponds to in VEX just so if someone is curious they easily find it?

aymanmus marked 2 inline comments as done.Mar 1 2017, 8:25 AM
aymanmus added inline comments.
utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
68 ↗(On Diff #89991)

Any table defined inside the class would require stating the size when defined (ExceptionList[11]), So I figured it's more maintainable this way.
The mappings of these instructions to VEX instructions make no sense, there is no "meaning" to the relation.

aymanmus updated this revision to Diff 90188.Mar 1 2017, 8:31 AM

Removing the changed tests from this patch.
Another patch was uploaded including manual update of the EVEX2VEX tables and all the changed tests.

craig.topper added inline comments.Mar 1 2017, 9:04 AM
utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
20 ↗(On Diff #90188)

I think LLVM coding standards say not to use iostream.

21 ↗(On Diff #90188)

using namespace std is frowned upon in LLVM. Please use the std:: prefix explicitly where needed.

84 ↗(On Diff #90188)

Can we use StringRef::startswith here?

248 ↗(On Diff #90188)

This OpRec != OpRec2 check seems redundant due to the earlier check and continue.

305 ↗(On Diff #90188)

You shouldn't need to call str(). getName() should return a StringRef that has an operator==

309 ↗(On Diff #90188)

Should we skip EVEX_LL == 2 here?

318 ↗(On Diff #90188)

I think this is calling std::find_if, but we should have an llvm::find_if that can take an object that supports with begin() and end() without needing to call them explicitly. So its two parameters instead of three.

68 ↗(On Diff #89991)

Well from a quick look it seemed like some of them were just Q versions of similar instructions in VEX that have WIG right? Is there not some way we can detect that these have W==1 and infer that we shouldn't use a VEX WIG instruction?

aymanmus marked 7 inline comments as done.Mar 2 2017, 1:20 AM
aymanmus added inline comments.
utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
68 ↗(On Diff #89991)

If an instruction has WIG value, we map it to either W == 1 or W == 0 or both if they are available.
There is no way to distinguish these specific cases apart from the rest, except for holding an exception list in some way.

aymanmus updated this revision to Diff 90298.Mar 2 2017, 1:37 AM
craig.topper added inline comments.Mar 2 2017, 9:38 AM
utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
81 ↗(On Diff #90298)

Make InstrStr a const std::string& right now there's a copy there.

111 ↗(On Diff #90298)

What special about VPERMILPD that need to be manually added?

Same question for the BROADCASTs.

316 ↗(On Diff #90298)

How does this handle the MAX and MAXC instructions both having the same opcode information?

aymanmus marked an inline comment as done.Mar 6 2017, 12:42 AM
aymanmus added inline comments.
utils/TableGen/X86EVEX2VEXTablesEmitter.cpp
111 ↗(On Diff #90298)

They do not accept on the W-bit.
VEX version have WIG0 while EVEX version have WIG1.

316 ↗(On Diff #90298)

As I said in the other patch, in this stage of the compiler, there is no difference between MAX and MAXC, so the first to match is picked.

This revision is now accepted and ready to land.Mar 6 2017, 9:44 AM
This revision was automatically updated to reflect the committed changes.
llvm/trunk/lib/Target/X86/X86EvexToVex.cpp