[PATCH] Support APX CFCMOV

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, June 18, 2024 5:34 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: hjl.tools@gmail.com; binutils@sourceware.org
> Subject: Re: [PATCH] Support APX CFCMOV
> 
> On 18.06.2024 11:24, Cui, Lili wrote:
> >> On 17.06.2024 08:18, Cui, Lili wrote:
> >>> --- a/opcodes/i386-opc.tbl
> >>> +++ b/opcodes/i386-opc.tbl
> >>> @@ -989,6 +989,10 @@ ud0, 0xfff, i186,
> >>> Modrm|CheckOperandSize|No_bSuf|No_sSuf, { Reg16|Reg32|Reg64|Un
> >>> cmov<cc>, 0x4<cc:opc>, CMOV&APX_F,
> >>> Modrm|CheckOperandSize|No_bSuf|No_sSuf|DstVVVV|EVexMap4, {
> >>> Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64,
> >>> Reg16|Reg32|Reg64 }  cmov<cc>, 0xf4<cc:opc>, CMOV,
> >>> Modrm|CheckOperandSize|No_bSuf|No_sSuf, {
> >>> Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
> >>>
> >>> +cfcmov<cc>, 0x4<cc:opc>, CMOV&APX_F,
> >>> +Modrm|CheckOperandSize|No_bSuf|No_sSuf|DstVVVV|EVexMap4|NF, {
> >>> +Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64,
> >>> +Reg16|Reg32|Reg64 } cfcmov<cc>, 0x4<cc:opc>, CMOV&APX_F,
> >>> +Load|Modrm|CheckOperandSize|No_bSuf|No_sSuf|EVexMap4, {
> >>> +Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
> >>> +cfcmov<cc>, 0x4<cc:opc>, CMOV&APX_F,
> >>> +Modrm|CheckOperandSize|No_bSuf|No_sSuf|EVexMap4|NF, {
> >>> +Reg16|Reg32|Reg64, Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> >>
> >> Just on this one for starters (I'll get to the other parts later): NF
> >> here permits use of {nf}, which is specifically not wanted. EVEX.NF
> >> has a different purpose here, and hence the pseudo prefix, if used, needs to
> be rejected.
> >
> > Added.
> 
> To perhaps save a round trip on the eventual v2, would you mind clarifying what
> you mean by "added"? The point of my remark was that I think NF here needs
> dropping, with the setting of EVEX.NF in the resulting encoding being driven by
> other means. Yet of course the overall intended effect could also be achieved by
> keeping the attribute here, and adding checks elsewhere (which is what I'd like to
> avoid).
> 

I added 3 invalid test cases for it, and changed the code in a way you didn't like. I thought it was a relatively minor change. I think the advantage of this approach is that the changes were relatively minor.

       /* Check NF support.  */
       specific_error = progress (unsupported_nf);
-      if (i.has_nf && !t->opcode_modifier.nf)
+      if (i.has_nf && (!t->opcode_modifier.nf || is_cpu (t, CpuCMOV)))
        continue;

Thanks,
Lili.