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

["Jiang, Haochen" <haochen.jiang@intel.com>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, October 25, 2022 3:01 PM
> To: Jiang, Haochen <haochen.jiang@intel.com>
> Cc: hjl.tools@gmail.com; Kong, Lingling <lingling.kong@intel.com>;
> binutils@sourceware.org
> Subject: Re: [PATCH 06/10] Support Intel RAO-INT
> 
> 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).

It should be Mdq here, will change that.

> >>> +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.

I still agree with H.J.. Also I suppose the previous usage of suffixes are quite
messy and we should not stick on the confusing code.

Keeping consistent with SPEC from now on to avoid confusion on that might
be a better solution for future.

> 
> There's also still no CheckRegSize here.

I suppose you take that question back before. But I realized that there are two
questions there. I checked testcases and they might not be sufficient. Let me
add some testcases with author @Kong, Lingling to see whether CheckRegSize
might be needed here. My answer now is yes but might change.

Haochen