[PATCH] Support APX CCMP and CTEST

[Cui, Lili]

> On 31.05.2024 08:40, Cui, Lili wrote:
> >>> --- /dev/null
> >>> +++ b/gas/testsuite/gas/i386/x86-64-apx-ccmp-ctest-intel.d
> >>> @@ -0,0 +1,220 @@
> >>> +#as:
> >>> +#objdump: -dw -Mintel
> >>> +#name: x86_64 APX_F CCMP and CTEST insns (Intel disassembly)
> >>> +#source: x86-64-apx-ccmp-ctest.s
> >>> +
> >>> +.*: +file format .*
> >>> +
> >>> +Disassembly of section \.text:
> >>> +
> >>> +0+ <_start>:
> >>> +\s*[a-f0-9]+:\s*62 d4 8c 02 83 bc 80 23 01 00 00 7b[ 	]+ccmpb
> >> \{dfv=cf\} QWORD PTR \[r8\+rax\*4\+0x123\],0x7b
> >>
> >> In order to spot possible issues, it helps is test expectations are readable.
> >> \s is imo hampering readability, as in not being a visual separator. However,
> >> what I strongly object to is a _mix_ of \s and [ 	].
> >>
> >
> > [space+tab] is certainly more intuitive, but it takes up too much screen
> space, especially for long assembler and disassembler instructions, so I prefer
> to use \s. I remember you asked in a previous comment to replace the last \s
> with [], so I replaced the last \s before I sent out the patch, resulting in the
> current hybrid state. Do you mind if I change the last [] to \s? Keep one style.
> 
> If \s is used, then consistently, yes. But "taking up screen space" is an imo
> pretty week argument nowadays, considering on how wide typical monitors
> have been already years ago. I won't veto \s uses, but I'd really like to ask you
> to reconsider. In new tests I add I won't ever use \s, and I may go and replace
> \s in expectations I need to touch anyway at more than just very few lines.
> 

Ok, [] is used here.

> >>> @@ -14089,3 +14110,55 @@ JMPABS_Fixup (instr_info *ins, int
> >>> bytemode,
> >> int sizeflag)
> >>>      return OP_IMREG (ins, bytemode, sizeflag);
> >>>    return OP_OFF64 (ins, bytemode, sizeflag);  }
> >>> +
> >>> +static const char *const oszc_flags[16] = {
> >>> +  " {dfv=}", " {dfv=cf}", " {dfv=zf}", " {dfv=zf, cf}", "
> >>> +{dfv=sf}",
> >>> +  " {dfv=sf, cf}", " {dfv=sf, zf}", " {dfv=sf, zf, cf}", "
> >>> +{dfv=of}",
> >>> +  " {dfv=of, cf}", " {dfv=of, zf}", " {dfv=of, zf, cf}", " {dfv=of,
> >>> +sf}",
> >>> +  " {dfv=of, sf, cf}", " {dfv=of, sf, zf}", " {dfv=of, sf, zf, cf}"
> >>> +};
> >>> +
> >>> +static const char *const scc_suffix[16] = {
> >>> +  "o", "no", "b", "nb", "z", "nz", "be", "nbe", "s", "ns", "t",
> >>> +"f",
> >>> +  "l", "nl", "le", "nle"
> >>> +};
> >>
> >> Can these please be in sync with the forms we use for e.g. Jcc and SETcc?
> >> I actually have a patch pending harmonizing these for CMPccXADD as well.
> >>
> >
> > I'm ok to change it like Jcc, but there are still 2 bits that are different from
> JCC.
> 
> Of course.
> 
> > "SCC values 0b1010 and 0b1011 (which are P and NP in normal condition
> > codes) should be written as T and F because they now mean True and
> > False."  -- apx-asm-syntax
> >
> > static const char *const scc_suffix[16] = {
> >   "o", "no", "b", "ae", "e", "ne", "be", "a", "s", "ns", "t", "f",
> >   "l", "ge", "le", "g"
> > };
> 
> Looks better, yes, but please still consider introducing a %SC macro paralleling
> the %CC I'm introducing in "x86: disassembler macro for condition code".
> That'll then also avoid you needing to perhaps insert stuff into the middle of
> something that was output before.
> 

Introduced %SC an %DF, and removed SCC_And_OSZC_Flags_Fixup.

> >>> --- a/opcodes/i386-opc.tbl
> >>> +++ b/opcodes/i386-opc.tbl
> >>> @@ -341,9 +341,19 @@ cmp, 0x83/7, 0, Modrm|No_bSuf|No_sSuf,
> >> { Imm8S,
> >>> Reg16|Reg32|Reg64|Unspecified|Ba  cmp, 0x3c, 0, W|No_sSuf, {
> >>> Imm8|Imm16|Imm32|Imm32S, Acc|Byte|Word|Dword|Qword }  cmp,
> >> 0x80/7, 0,
> >>> W|Modrm|No_sSuf, { Imm8|Imm16|Imm32|Imm32S,
> >>> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> >>>
> >>> +<cc:opc, o:0, no:1, b:2, c:2, nae:2, nb:3, nc:3, ae:3, e:4, z:4,
> >>> +ne:5, nz:5,
> >> be:6, na:6, nbe:7, a:7, +
> >>> +         s:8, ns:9, t:a, p:a, pe:a, f:b, np:b, po:b, l:c, nge:c,
> >>> +nl:d, ge:d, le:e, ng:e, nle:f, g:f>
> >>> +
> >>> +ccmp<cc>, 0x380<cc:opc>, APX_F,
> >>> +D|W|CheckOperandSize|Modrm|EVexMap4|SCC|No_sSuf, {
> >>> +Reg8|Reg16|Reg32|Reg64,
> >> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex
> >>> +} ccmp<cc>, 0x830<cc:opc>/7, APX_F,
> >>> +Modrm|EVexMap4|SCC|No_bSuf|No_sSuf, { Imm8S,
> >>> +Reg16|Reg32|Reg64|Unspecified|BaseIndex } ccmp<cc>,
> >> 0x800<cc:opc>/7,
> >>> +APX_F, W|Modrm|EVexMap4|SCC|No_sSuf,
> >> { Imm8|Imm16|Imm32|Imm32S,
> >>> +Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> >>> +
> >>>  test, 0x84, 0, D|W|C|CheckOperandSize|Modrm|No_sSuf, {
> >>> Reg8|Reg16|Reg32|Reg64,
> >> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> >>> test, 0xa8, 0, W|No_sSuf|Optimize, { Imm8|Imm16|Imm32|Imm32S,
> >>> Acc|Byte|Word|Dword|Qword }  test, 0xf6/0, 0,
> >>> W|Modrm|No_sSuf|Optimize, { Imm8|Imm16|Imm32|Imm32S,
> >>> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> >>> +ctest<cc>, 0x840<cc:opc>, APX_F,
> >>> +D|W|C|CheckOperandSize|Modrm|EVexMap4|SCC|No_sSuf, {
> >>> +Reg8|Reg16|Reg32|Reg64,
> >> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex
> >>> +} ctest<cc>, 0xf60<cc:opc>/0, APX_F,
> W|Modrm|EVexMap4|SCC|No_sSuf,
> >>> +{
> >>> +Imm8|Imm16|Imm32|Imm32S,
> >> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex
> >>> +} ctest<cc>, 0xf60<cc:opc>/1, APX_F,
> W|Modrm|EVexMap4|SCC|No_sSuf,
> >>> +{
> >>> +Imm8|Imm16|Imm32|Imm32S,
> >> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex
> >>> +}
> >>
> >> In the context of this patch (as a whole) - what meaning to we intend e.g.
> >>
> >> 	{evex} cmp %eax, %ecx
> >>
> >> to have? Imo that should translate to CCMPT.
> >>
> >
> > Could you describe it in more detail? I'm a bit confused. Why should we
> support {evex} cmp %eax, %ecx ?
> 
> To parallel {evex} use with other insns. And CCMPT really _is_ the EVEX
> encoding of CMP, considering CCMPcc's placement(s) in the opcode map.
> 

Ok, got it, added templates and test cases.

> And just to mention it here: I also think there might better be a pseudo-insn
> aliasing to one of CCMPF or CTESTF, with no operands. I.e. merely specifying
> the new value for the respective EFLAGS bits. Yet adding such - requiring a
> mnemonic which then won't be used elsewhere in the future - can't very well
> be done without the assembler doc at least permitting for it.
> 

Ok, I'll feedback this issue.

Thanks,
Lili.