RE: [PATCH V2 3/8] Support APX GPR32 with extend evex prefix

["Cui, Lili" <lili.cui@intel.com>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, November 15, 2023 5:12 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; binutils@sourceware.org
> Subject: Re: [PATCH V2 3/8] Support APX GPR32 with extend evex prefix
> 
> On 15.11.2023 07:03, Cui, Lili wrote:
> >>> --- a/opcodes/i386-dis-evex-reg.h
> >>> +++ b/opcodes/i386-dis-evex-reg.h
> >>> @@ -49,3 +49,17 @@
> >>>      { "vscatterpf0qp%XW",  { MVexVSIBQWpX }, PREFIX_DATA },
> >>>      { "vscatterpf1qp%XW",  { MVexVSIBQWpX }, PREFIX_DATA },
> >>>    },
> >>> +  /* REG_EVEX_0F38F3_L_0 */
> >>> +  {
> >>> +    { Bad_Opcode },
> >>> +    { "blsrS",		{ VexGdq, Edq }, 0 },
> >>> +    { "blsmskS",	{ VexGdq, Edq }, 0 },
> >>> +    { "blsiS",		{ VexGdq, Edq }, 0 },
> >>> +  },
> >>
> >> Compared to the REG_VEX_0F38F3_L_0 entry this lacks PREFIX_OPCODE,
> >> but then the question is why you do not re-use that entry in the first place.
> >
> > Added PREFIX_TABLE for them.
> 
> PREFIX_OPCODE you mean? Or a decoding step through prefix_table[]? (The
> former would be contrary to what we discussed elsewhere for use of
> PREFIX_OPCODE with EVEX encodings. Albeit I still think the goal should be to
> avoid going through prefix_table[] when we have a large set of insns all with
> similar properties.)
> 

Haha, I let them pass prefix_table[].

> >>> --- /dev/null
> >>> +++ b/opcodes/i386-dis-evex-x86-64.h
> >>> @@ -0,0 +1,140 @@
> >>> +  /* X86_64_EVEX_0F3849 */
> >>> +  {
> >>> +    { Bad_Opcode },
> >>> +    { VEX_LEN_TABLE (VEX_LEN_0F3849_X86_64) },  },
> >>> +  /* X86_64_EVEX_0F384B */
> >>> +  {
> >>> +    { Bad_Opcode },
> >>> +    { VEX_LEN_TABLE (VEX_LEN_0F384B_X86_64) },  },
> >>> +  /* X86_64_EVEX_0F38E0 */
> >>> +  {
> >>> +    { Bad_Opcode },
> >>> +    { "cmpoxadd", { Mdq, Gdq, VexGdq }, PREFIX_DATA },  },
> >>
> >> This and its sibling entries look to again fully match
> X86_64_VEX_0F38E<n>.
> >
> > It cannot be moved to up level, it needs to go through
> > i386-dis-evex-x86-64.h
> 
> I didn't suggest to move up a level. I suggested to re-use the existing VEX
> entries.
> 

It's currently in the x86-64 table, it's a bit weird to revisit the x86-64 table, and moving up one level won't work.

{ X86_64_TABLE (X86_64_VEX_0F38E0) }

  /* X86_64_VEX_0F38E0 */
  {
    { Bad_Opcode },
    { "cmpoxadd", { Mdq, Gdq, VexGdq }, PREFIX_DATA },
  },

> >>> @@ -11242,7 +11367,7 @@ print_register (instr_info *ins, unsigned
> >>> int reg,
> >> unsigned int rexmask,
> >            case b_mode:
> >>>      case b_swap_mode:
> >>>        if (reg & 4)
> >>>  	USED_REX (0);
> >>> -      if (ins->rex)
> >>> +      if (ins->rex || ins->rex2)
> >>>  	names = att_names8rex;
> >>>        else
> >>>  	names = att_names8;
> >>
> >> Isn't this already needed in the REX2 patch?
> >>
> >
> > CRC32  --> Eb ---> b_mode
> 
> I'm afraid I don't understand this reply. b_swap_mode is used by EbS, which
> in turn is used by ADD, SUB, etc. Similarly b_mode / Eb are used by more than
> just CRC32.
> 

I misunderstood, will move it to the patch 1/8.

Thanks,
Lili.