[PATCH 5/8] Support APX NDD

Cui, Lili lili.cui@intel.com
Wed Nov 22 07:46:42 GMT 2023


> >>> --- a/opcodes/i386-dis-evex.h
> >>> +++ b/opcodes/i386-dis-evex.h
> >>> [...]
> >>> @@ -947,23 +947,23 @@ static const struct dis386 evex_table[][256] = {
> >>>      { Bad_Opcode },
> >>>      { Bad_Opcode },
> >>>      /* 40 */
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> +    { "cmovoS",		{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovnoS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovbS",		{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovaeS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmoveS",		{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovneS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovbeS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovaS",		{ VexGv, Gv, Ev }, 0 },
> >>>      /* 48 */
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> -    { Bad_Opcode },
> >>> +    { "cmovsS",		{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovnsS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovpS",		{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovnpS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovlS",		{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovgeS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovleS",	{ VexGv, Gv, Ev }, 0 },
> >>> +    { "cmovgS",		{ VexGv, Gv, Ev }, 0 },
> >>
> >> Considering CFCMOVcc which sits at the same opcode, doing things like
> >> this sets us up for needing to touch all of these again. Maybe that's
> >> the best that can be done, but I still wonder whether this couldn't
> >> be taken care of right away when introducing these entries.
> >
> > How about adding a special letter %CF in front of them?
> 
> Why not.
>

Done.
 
> >>> --- a/opcodes/i386-dis.c
> >>> +++ b/opcodes/i386-dis.c
> >>> [...]
> >>> @@ -2660,47 +2668,47 @@ static const struct dis386 reg_table[][8] = {
> >>>    },
> >>>    /* REG_D0 */
> >>>    {
> >>> -    { "rolA",	{ Eb, I1 }, 0 },
> >>> -    { "rorA",	{ Eb, I1 }, 0 },
> >>> -    { "rclA",	{ Eb, I1 }, 0 },
> >>> -    { "rcrA",	{ Eb, I1 }, 0 },
> >>> -    { "shlA",	{ Eb, I1 }, 0 },
> >>> -    { "shrA",	{ Eb, I1 }, 0 },
> >>> -    { "shlA",	{ Eb, I1 }, 0 },
> >>> -    { "sarA",	{ Eb, I1 }, 0 },
> >>> +    { "rolA",	{ VexGb, Eb, I1 }, 0 },
> >>> +    { "rorA",	{ VexGb, Eb, I1 }, 0 },
> >>> +    { "rclA",	{ VexGb, Eb, I1 }, 0 },
> >>> +    { "rcrA",	{ VexGb, Eb, I1 }, 0 },
> >>> +    { "shlA",	{ VexGb, Eb, I1 }, 0 },
> >>> +    { "shrA",	{ VexGb, Eb, I1 }, 0 },
> >>> +    { "shlA",	{ VexGb, Eb, I1 }, 0 },
> >>> +    { "sarA",	{ VexGb, Eb, I1 }, 0 },
> >>>    },
> >>>    /* REG_D1 */
> >>>    {
> >>> -    { "rolQ",	{ Ev, I1 }, 0 },
> >>> -    { "rorQ",	{ Ev, I1 }, 0 },
> >>> -    { "rclQ",	{ Ev, I1 }, 0 },
> >>> -    { "rcrQ",	{ Ev, I1 }, 0 },
> >>> -    { "shlQ",	{ Ev, I1 }, 0 },
> >>> -    { "shrQ",	{ Ev, I1 }, 0 },
> >>> -    { "shlQ",	{ Ev, I1 }, 0 },
> >>> -    { "sarQ",	{ Ev, I1 }, 0 },
> >>> +    { "rolQ",	{ VexGv, Ev, I1 }, 0 },
> >>> +    { "rorQ",	{ VexGv, Ev, I1 }, 0 },
> >>> +    { "rclQ",	{ VexGv, Ev, I1 }, 0 },
> >>> +    { "rcrQ",	{ VexGv, Ev, I1 }, 0 },
> >>> +    { "shlQ",	{ VexGv, Ev, I1 }, 0 },
> >>> +    { "shrQ",	{ VexGv, Ev, I1 }, 0 },
> >>> +    { "shlQ",	{ VexGv, Ev, I1 }, 0 },
> >>> +    { "sarQ",	{ VexGv, Ev, I1 }, 0 },
> >>>    },
> >>
> >> As mentioned on the assembler side already, I think we would be
> >> better off making const_1_mode print $1 in AT&T syntax at least for
> >> these new insn forms, to eliminate the ambiguity.
> >>
> >
> > It is related to correctness and should be revised. Since they share the same
> entries, I will created a new patch to modify the legacy instruction and then
> extend them to NDD. Do you agree?
> 
> I certainly appreciate any reusing, where it is possible (and it ought to be
> possible here, yes).
> 

Done.

> >>> @@ -9087,7 +9104,7 @@ get_valid_dis386 (const struct dis386 *dp,
> >> instr_info *ins)
> >>>  	return &err_opcode;
> >>>
> >>>        /* Set vector length.  */
> >>> -      if (ins->modrm.mod == 3 && ins->vex.b)
> >>> +      if (ins->modrm.mod == 3 && ins->vex.b && ins->evex_type ==
> >>> + evex_default)
> >>>  	ins->vex.length = 512;
> >>>        else
> >>>  	{
> >>
> >> Is this change really needed for anything?
> >
> > If it's NDD and ins->vex.b ==1, we need to avoid giving NDD a wrong value.
> 
> But this is recording ->vex.length, not anything NDD related (afaics).

There are some instructions that use OP_VEX, which will use ->vex.length.

For example:
"addB",             { VexGb, Eb, Gb }

 


More information about the Binutils mailing list