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

[Jan Beulich <jbeulich@suse.com>]

On 24.10.2022 07:56, Jiang, Haochen wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Friday, October 14, 2022 10:38 PM
>>
>> On 14.10.2022 11:12, Haochen Jiang wrote:
>>> --- a/opcodes/i386-dis.c
>>> +++ b/opcodes/i386-dis.c
>>> @@ -887,6 +887,7 @@ enum
>>>    MOD_0F38F9,
>>>    MOD_0F38FA_PREFIX_1,
>>>    MOD_0F38FB_PREFIX_1,
>>> +  MOD_0F38FC,
>>>    MOD_0F3A0F_PREFIX_1,
>>>
>>>    MOD_VEX_0F12_PREFIX_0,
>>> @@ -1086,6 +1087,7 @@ enum
>>>    PREFIX_0F38F8,
>>>    PREFIX_0F38FA,
>>>    PREFIX_0F38FB,
>>> +  PREFIX_0F38FC,
>>
>> PREFIX_0F38FC_M_0 please (see comment on an earlier patch). However, like in
>> the earlier patch - if you used Mdq below, you could avoid going through
>> mod_table[] altogether.
> 
> Removed pass modrm table since Edq seems also judges modrm.

Edq? That one allows for register operands as well. I did suggest to use
Mdq (iirc introduced by an earlier patch).

>>> --- a/opcodes/i386-opc.tbl
>>> +++ b/opcodes/i386-opc.tbl
>>> @@ -3317,3 +3317,12 @@ cmpsxadd, 0x66e8, None, CpuCMPCCXADD|Cpu64,
>>> Modrm|Vex128|Space0F38|VexVVVV=1|Swa
>>>  cmpzxadd, 0x66e4, None, CpuCMPCCXADD|Cpu64,
>>>
>> Modrm|Vex128|Space0F38|VexVVVV=1|SwapSources|CheckRegSize|No_bSuf|
>> No_w
>>> Suf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32|Reg64, Reg32|Reg64,
>>> Dword|Qword|Unspecified|BaseIndex }
>>>
>>>  // CMPCCXADD instructions end.
>>> +
>>> +// RAOINT instructions.
>>
>> Nit: Better RAO-INT, like in the title?
> 
> Done.
> 
>>
>>> +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).
>>
> 
> For table, we aligned with CMPccXADD and added No_lSuf and No_qSuf since
> the suffixes are not required.

Which, as said, I consider wrong both here and there.

There's also still no CheckRegSize here.

> In future, if suffixes are not required, we will add all the No_xxSuf.
> 
> BTW, can we write a macro named No_allSuf including all of them to shorten
> the line?

We probably could (yet then perhaps named No_Suf or NoSuf), but I have
other patches pending to help this at least some. In the long run we
really want to turn things around and specify which suffixes are
permitted, as the majority of insns doesn't permit any.

Jan