[X86][X87]: Adding full coverage of MC encoding for all X87 ISA Sets.<NFC>
ClosedPublic

Authored by gadi.haber on Mon, Nov 13, 1:36 AM.

Details

Summary

NFC.
Currently, not all the X86 ISA Sets are covered by the MC regressions tests for X86.
A full coverage needs to be added for each ISA set and for both 32bit and 64bit instructions + registers.
This patch includes MC assembly tests for the X87 32bit and 64bit.

Diff Detail

Repository
rL LLVM
gadi.haber created this revision.Mon, Nov 13, 1:36 AM

Are you intending to include the generating script code?

What are you intending to include in the tests like 'I486', 'I486REAL', etc ?

Unfortunately, I am using an internal DB with python scripts.

I486 will include encoding + asm of the following instrs:

#iclass extension category iform isa_set attributes
BSWAP BASE DATAXFER BSWAP_GPRv I486REAL SCALABLE
CMPXCHG BASE SEMAPHORE CMPXCHG_GPR8_GPR8 I486REAL BYTEOP
CMPXCHG BASE SEMAPHORE CMPXCHG_GPRv_GPRv I486REAL SCALABLE
CMPXCHG BASE SEMAPHORE CMPXCHG_MEMb_GPR8 I486REAL BYTEOP:LOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE
CMPXCHG BASE SEMAPHORE CMPXCHG_MEMv_GPRv I486REAL LOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE:SCALABLE
CPUID BASE MISC CPUID I486REAL NOTSX
INVD BASE SYSTEM INVD I486REAL RING0:NOTSX
INVLPG BASE SYSTEM INVLPG_MEMb I486REAL ATT_OPERAND_ORDER_EXCEPTION:BYTEOP:RING0:NOTSX
RSM BASE SYSRET RSM I486 NOTSX
WBINVD BASE SYSTEM WBINVD I486REAL RING0:NOTSX
XADD BASE SEMAPHORE XADD_GPR8_GPR8 I486REAL BYTEOP
XADD BASE SEMAPHORE XADD_GPRv_GPRv I486REAL SCALABLE
XADD BASE SEMAPHORE XADD_MEMb_GPR8 I486REAL BYTEOP:LOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE
XADD BASE SEMAPHORE XADD_MEMv_GPRv I486REAL LOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE:SCALABLE

Sorry. Here is a more readbale table of I486 + I486REAL:

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

#iclassextensioncategoryiformisa_setattributes

+============+==============+==============+======================+=================+=======================================================+

BSWAPBASEDATAXFERBSWAP_GPRvI486REALSCALABLE

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

CMPXCHGBASESEMAPHORECMPXCHG_GPR8_GPR8I486REALBYTEOP

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

CMPXCHGBASESEMAPHORECMPXCHG_GPRv_GPRvI486REALSCALABLE

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

CMPXCHGBASESEMAPHORECMPXCHG_MEMb_GPR8I486REALBYTEOP:LOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

CMPXCHGBASESEMAPHORECMPXCHG_MEMv_GPRvI486REALLOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE:SCALABLE

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

CPUIDBASEMISCCPUIDI486REALNOTSX

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

INVDBASESYSTEMINVDI486REALRING0:NOTSX

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

INVLPGBASESYSTEMINVLPG_MEMbI486REALATT_OPERAND_ORDER_EXCEPTION:BYTEOP:RING0:NOTSX

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

RSMBASESYSRETRSMI486NOTSX

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

WBINVDBASESYSTEMWBINVDI486REALRING0:NOTSX

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

XADDBASESEMAPHOREXADD_GPR8_GPR8I486REALBYTEOP

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

XADDBASESEMAPHOREXADD_GPRv_GPRvI486REALSCALABLE

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

XADDBASESEMAPHOREXADD_MEMb_GPR8I486REALBYTEOP:LOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

XADDBASESEMAPHOREXADD_MEMv_GPRvI486REALLOCKABLE:HLE_ACQ_ABLE:HLE_REL_ABLE:SCALABLE

+------------+----------------------------------------------------------------------+-------------------------------------------------------+

gadi.haber retitled this revision from [X86]: Adding full coverage of MC encoding for all X86 ISA Sets to [X86]: Adding full coverage of MC encoding for all X86 ISA Sets.NFC.Mon, Nov 13, 11:22 PM
gadi.haber edited the summary of this revision. (Show Details)

Two instruction sets I noticed were missing from your list: CLZERO and LWP. Do you know if your 3DNow tests cover all versions of it?

Do you mean the following LWP instructions that belong to the XOP ISA set?:
LLWPCB LLWPCB_GPRvqq
LWPINS LWPINS_GPRyqq_GPRvd_IMMd
LWPINS LWPINS_GPRyqq_MEMd_IMMd
LWPVAL LWPVAL_GPRyqq_GPRvd_IMMd
LWPVAL LWPVAL_GPRyqq_MEMd_IMMd
SLWPCB SLWPCB_GPRvqq

I think CLZERO is very new and is part of ZEN. I will check.

Ok. The CLZERO instruction is going to be part of AMD ISA Set. It is very recent and part of of the Rizen CPU.

I don't recognize some of these

BBX2_128
BBX2_256
BBX2_512
BBX2_KOP
XNMOV

ICL_NONPOR seems questionable.

The BBX* ISA Sets are new AVX512 instrs. Here are some examples below (<format: #iclass, extension, category, iform, isa_set>:
VPSLCTLASTD, AVX512EVEX, BBX2, VPSLCTLASTD_XMMu32_MASKmskw_MEMu32_BBX, BBX2_128
VPSLCTLASTD, AVX512EVEX, BBX2, VPSLCTLASTD_XMMu32_MASKmskw_XMMu32_BBX, BBX2_128
VPSLCTLASTD, AVX512EVEX, BBX2, VPSLCTLASTD_YMMu32_MASKmskw_MEMu32_BBX, BBX2_256
VPSLCTLASTD, AVX512EVEX, BBX2, VPSLCTLASTD_YMMu32_MASKmskw_YMMu32_BBX, BBX2_256
VPSLCTLASTD, AVX512EVEX, BBX2, VPSLCTLASTD_ZMMu32_MASKmskw_MEMu32_BBX, BBX2_512
VPSLCTLASTD, AVX512EVEX, BBX2, VPSLCTLASTD_ZMMu32_MASKmskw_ZMMu32_BBX, BBX2_512
VPSLCTLASTQ, AVX512EVEX, BBX2, VPSLCTLASTQ_XMMu64_MASKmskw_MEMu64_BBX, BBX2_128
VPSLCTLASTQ, AVX512EVEX, BBX2, VPSLCTLASTQ_XMMu64_MASKmskw_XMMu64_BBX, BBX2_128
VPSLCTLASTQ, AVX512EVEX, BBX2, VPSLCTLASTQ_YMMu64_MASKmskw_MEMu64_BBX, BBX2_256
VPSLCTLASTQ, AVX512EVEX, BBX2, VPSLCTLASTQ_YMMu64_MASKmskw_YMMu64_BBX, BBX2_256
VPSLCTLASTQ, AVX512EVEX, BBX2, VPSLCTLASTQ_ZMMu64_MASKmskw_MEMu64_BBX, BBX2_512
VPSLCTLASTQ, AVX512EVEX, BBX2, VPSLCTLASTQ_ZMMu64_MASKmskw_ZMMu64_BBX, BBX2_512

Please ignore my previous comment. These instructions are not used by the compiler Ring 1.
I will remove them from the ISA Set.

Do you mean the following LWP instructions that belong to the XOP ISA set?:
LLWPCB LLWPCB_GPRvqq
LWPINS LWPINS_GPRyqq_GPRvd_IMMd
LWPINS LWPINS_GPRyqq_MEMd_IMMd
LWPVAL LWPVAL_GPRyqq_GPRvd_IMMd
LWPVAL LWPVAL_GPRyqq_MEMd_IMMd
SLWPCB SLWPCB_GPRvqq

Would it be better to consistently split all these instruction groups by cpuid feature bit instead of ISA set? For instance, LWP uses the XOP encoding but IIRC so does TBM and that is split.

gadi.haber edited the summary of this revision. (Show Details)Wed, Nov 15, 3:31 AM

I updated the list of ISA Sets.
I think we can consdier removing the REAL and the Protected ISA right?

Simon, splitting the instructions based on the CPUID bit is referred actually according to "CPU extension".
There are a total of 62 extensions.
I can do it according to extensions. The only problem with that is that it will complicate the generation of the tests.

zvi added inline comments.Sun, Nov 19, 12:41 PM
test/MC/X87-32.s
3 ↗(On Diff #122621)

Would be nice if the instructions were sorted by name.

sorted the instructions per Zvi's comment

gadi.haber marked an inline comment as done.Mon, Nov 20, 4:37 AM
gadi.haber edited the summary of this revision. (Show Details)Tue, Nov 21, 3:24 AM
gadi.haber retitled this revision from [X86]: Adding full coverage of MC encoding for all X86 ISA Sets.NFC to [X86]: Adding full coverage of MC encoding for all X86 ISA Sets.<NFC>.

There are some duplicates - I've tagged a couple but there are probably others.

test/MC/X86/X87-32.s
1241 ↗(On Diff #123567)

Duplicate?

1273 ↗(On Diff #123567)

Duplicate?

gadi.haber retitled this revision from [X86]: Adding full coverage of MC encoding for all X86 ISA Sets.<NFC> to [X86][X87]: Adding full coverage of MC encoding for all X87 ISA Sets.<NFC>.Wed, Nov 29, 1:39 AM
gadi.haber edited the summary of this revision. (Show Details)

Removed duplicates per Simon's comment

gadi.haber marked 2 inline comments as done.Wed, Nov 29, 1:53 AM
RKSimon accepted this revision.Wed, Dec 6, 8:16 AM

LGTM

This revision is now accepted and ready to land.Wed, Dec 6, 8:16 AM
This revision was automatically updated to reflect the committed changes.