This is an archive of the discontinued LLVM Phabricator instance.

[X86][CET]: Adding full coverage of MC encoding for the CET instructions.<NFC>
ClosedPublic

Authored by gadi.haber on Dec 17 2017, 2:38 AM.

Details

Summary

NFC.
Adding MC regressions tests to cover the CET instructions.
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 17 2017, 2:38 AM

Do you plan to add the ENDBR to this when that instruction gets added?

Yes, Added Andrei who is going to take over this task and add the new instructions.

Oren has a patch for CET that includes the ENDBR instructions in the .td files. Maybe we should have him split that fragment out so we can get those in for the assembler/disassembler

good call. I will add Oren to the review.

I added encoding test in the following review:
https://reviews.llvm.org/D40223

cet-encoding.s checks only 64 bit encoding of all CET instructions (IBT and SHSTK).
Since all instructions that are supported in 32 bit, are also supported in 64 bit, I am not sure 32 bit encoding test is required.

If you do think it is needed, there are some instructions that are missing in your test: incspp, endbr64, endbr32 and setssbsy.

For more information on CET, please read section 6 in the following document:
https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf

gadi.haber added a comment.EditedFeb 4 2018, 12:40 AM

The MC tests for the CET ISA Set are fully covered in test/MC/X86/cet-encoding.s added in https://reviews.llvm.org/D40223
I, therefore, abandon this review.

The MC tests for the CET ISA Set are fully covered in test/MC/X86/cet-encoding.s added in https://reviews.llvm.org/D40223
I, therefore, abandon this review.

Don't we still need to add 32-bit coverage tests?

gadi.haber abandoned this revision.Feb 5 2018, 2:14 AM

The 32bit encoding is identical to the 64bit CET encoding (similar to the RTM ISA).

I assume Simon's point was that we should still have a test case that verifies that LLVM knows the encodings are the same in 32-bit mode.

gadi.haber reclaimed this revision.Feb 19 2018, 12:09 AM

reclaimed the review in order to add the 32-bit encoding tests.

Updated diff to include the complete 32bit and 64bit tests

This revision is now accepted and ready to land.Feb 19 2018, 9:32 AM
This revision was automatically updated to reflect the committed changes.