Re: [PATCH 06/10] Support Intel RAO-INT

[Jan Beulich <jbeulich@suse.com>]

On 06.11.2022 13:50, Kong, Lingling wrote:
>>>>> +aadd, 0xf38fc, None, CpuRAOINT,
>>>>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>> +aand, 0x660f38fc, None, CpuRAOINT,
>>>>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>> +aor, 0xf20f38fc, None, CpuRAOINT,
>>>>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>> +axor, 0xf30f38fc, None, CpuRAOINT,
>>>>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>
>>>> Why IgnoreSize? Instead I think you need CheckRegSize (assuming it
>>>> does enough for Intel syntax memory operands - please double check;
>>>> if not this will need fixing).
>>>>
> Yes,  now it changed to CheckRegSize. 

For one I'm pretty sure I said I was wrong about IgnoreSize. And as to
CheckRegSize - did you actually double check as indicated? While I
think the attribute should be there, I've also observed recently that
it missing on e.g. MOVBE does not currently make any difference (there
are the same bogus diagnostics with and without it). So while I don't
mind it being put there, H.J. may view this differently as long as the
attribute doesn't really have any effect.

> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -223,6 +223,8 @@ enum
>    CpuMSRLIST,
>    /* Intel AVX NE CONVERT Instructions support required.  */
>    CpuAVX_NE_CONVERT,
> +  /* Intel RAO INT Instructions support required.  */  CpuRAO_INT,
>    /* mwaitx instruction required */
>    CpuMWAITX,
>    /* Clzero instruction required */

This and ...

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3321,3 +3321,12 @@ rdmsrlist, 0xf20f01c6, None, CpuMSRLIST|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|N  wrmsrlist, 0xf30f01c6, None, CpuMSRLIST|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {}
>  
>  // MSRLIST instructions end.
> +
> +// RAO-INT instructions.
> +
> +aadd, 0xf38fc, None, CpuRAO_INT, 
> +Modrm|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf,{ 
> +Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex } aand, 0x660f38fc, 
> +None, CpuRAO_INT, 
> +Modrm|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf,{ 
> +Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex } aor, 0xf20f38fc, None, 
> +CpuRAO_INT, 
> +Modrm|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf,{ 
> +Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex } axor, 0xf30f38fc, 
> +None, CpuRAO_INT, 
> +Modrm|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf,{ 
> +Reg32|Reg64, Dword|Qword|Unspecified|BaseIndex }
> +
> +// RAO-INT instructions end.

... this doesn't look like valid changes - line breaks are at the wrong spots.

Jan