[6/8] Support APX Push2/Pop2

Message ID 20231102112911.2372810-7-lili.cui@intel.com
State New
Headers
Series Support Intel APX EGPR |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_check--master-arm fail Patch failed to apply

Commit Message

Cui, Lili Nov. 2, 2023, 11:29 a.m. UTC
  From: "Mo, Zewei" <zewei.mo@intel.com>

PPX functionality for PUSH/POP is not implemented in this patch
and will be implemented separately.

gas/ChangeLog:

	* config/tc-i386.c: (enum i386_error):
	New unsupported_rsp_register.
	(md_assemble): Add handler for unsupported_rsp_register.
	(check_VecOperands): Add invalid check for push2/pop2.
	* testsuite/gas/i386/i386.exp: Add apx-push2pop2 tests.
	* testsuite/gas/i386/x86-64.exp: Ditto.
	* testsuite/gas/i386/x86-64-apx-push2pop2.d: New test.
	* testsuite/gas/i386/x86-64-apx-push2pop2.s: Ditto.
	* testsuite/gas/i386/x86-64-apx-push2pop2-intel.d: Ditto.
	* testsuite/gas/i386/x86-64-apx-push2pop2-inval.l: Ditto.
	* testsuite/gas/i386/x86-64-apx-push2pop2-inval.s: Ditto.
	* testsuite/gas/i386/apx-push2pop2-inval.s: Ditto.
	* testsuite/gas/i386/apx-push2pop2-inval.d: Ditto.
	* testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d: Added bad
	testcases for POP.
	* testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s: Ditto.

opcodes/ChangeLog:

	* i386-dis-evex-mod.h: Add MOD_EVEX_MAP4_8F_R_0
	and MOD_EVEX_MAP4_FF_R_6
	* i386-dis-evex-prefix.h: Add PREFIX_EVEX_MAP4_8F_R_0_M_1
	and PREFIX_EVEX_MAP4_FF_R_6_M_1.
	* i386-dis-evex-reg.h: Add REG_EVEX_MAP4_8F.
	* i386-dis-evex-w.h: Add EVEX_W_MAP4_8F_R_0_M_1_P_0
	and EVEX_W_MAP4_FF_R_6_M_1_P_0
	* i386-dis-evex.h: Add REG_EVEX_MAP4_8F.
	* i386-dis.c (PUSH2_POP2_Fixup): Add special handling for PUSH2/POP2.
	(get_valid_dis386): Add handler for vector length and address_mode for
	APX-Push2/Pop2 insn.
	(OP_VEX): Add handler of 64-bit vvvv register for APX-Push2/Pop2 insn.
	* i386-gen.c: Add Push2Pop2 bitfield.
	* i386-opc.h: Regenerated.
	* i386-opc.tbl: Regenerated.
---
 gas/config/tc-i386.c                          | 22 ++++++++++
 gas/testsuite/gas/i386/apx-push2pop2-inval.l  |  5 +++
 gas/testsuite/gas/i386/apx-push2pop2-inval.s  |  9 ++++
 gas/testsuite/gas/i386/i386.exp               |  1 +
 .../gas/i386/x86-64-apx-evex-promoted-bad.d   |  6 ++-
 .../gas/i386/x86-64-apx-evex-promoted-bad.s   |  6 +++
 .../gas/i386/x86-64-apx-push2pop2-intel.d     | 42 +++++++++++++++++++
 .../gas/i386/x86-64-apx-push2pop2-inval.l     | 11 +++++
 .../gas/i386/x86-64-apx-push2pop2-inval.s     | 15 +++++++
 gas/testsuite/gas/i386/x86-64-apx-push2pop2.d | 42 +++++++++++++++++++
 gas/testsuite/gas/i386/x86-64-apx-push2pop2.s | 39 +++++++++++++++++
 gas/testsuite/gas/i386/x86-64.exp             |  3 ++
 opcodes/i386-dis-evex-mod.h                   | 10 +++++
 opcodes/i386-dis-evex-prefix.h                |  8 ++++
 opcodes/i386-dis-evex-reg.h                   |  9 ++++
 opcodes/i386-dis-evex-w.h                     | 10 +++++
 opcodes/i386-dis-evex.h                       |  2 +-
 opcodes/i386-dis.c                            | 40 +++++++++++++++++-
 opcodes/i386-gen.c                            |  1 +
 opcodes/i386-opc.h                            |  4 ++
 opcodes/i386-opc.tbl                          |  7 ++++
 21 files changed, 287 insertions(+), 5 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/apx-push2pop2-inval.l
 create mode 100644 gas/testsuite/gas/i386/apx-push2pop2-inval.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-apx-push2pop2-intel.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.l
 create mode 100644 gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.s
 create mode 100644 gas/testsuite/gas/i386/x86-64-apx-push2pop2.d
 create mode 100644 gas/testsuite/gas/i386/x86-64-apx-push2pop2.s
  

Comments

Jan Beulich Nov. 8, 2023, 11:44 a.m. UTC | #1
On 02.11.2023 12:29, Cui, Lili wrote:
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -256,6 +256,7 @@ enum i386_error
>      mask_not_on_destination,
>      no_default_mask,
>      unsupported_rc_sae,
> +    unsupported_rsp_register,
>      invalid_register_operand,
>      internal_error,
>    };
> @@ -5476,6 +5477,9 @@ md_assemble (char *line)
>  	case unsupported_rc_sae:
>  	  err_msg = _("unsupported static rounding/sae");
>  	  break;
> +	case unsupported_rsp_register:
> +	  err_msg = _("unsupported rsp register");
> +	  break;

Perhaps you mean "cannot be used with" or some such? Also the register
name needs conditionally prefixing with % in diagnostics.

> @@ -6854,6 +6858,24 @@ check_VecOperands (const insn_template *t)
>  	}
>      }
>  
> +  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same registers.  */
> +  if (t->opcode_modifier.push2pop2)

I question this way of recognizing these two insns: You introduce a
whole new table column here just to have two entries set this bit.
This is cheaper by comparing the mnemonic offsets, as we do elsewhere
in various cases.

I also disagree with putting the check in check_VecOperands():
There's nothing vector-ish here. Either you put it straight in the
caller, or you introduce a new check_APX_operands().

> +    {
> +      unsigned int reg1 = register_number (i.op[0].regs);
> +      unsigned int reg2 = register_number (i.op[1].regs);
> +
> +      if (reg1 == 0x4 || reg2 == 0x4)
> +	{
> +	  i.error = unsupported_rsp_register;
> +	  return 1;
> +	}
> +      if (t->base_opcode == 0x8f && reg1 == reg2)
> +	{
> +	  i.error = invalid_dest_and_src_register_set;

This enumerator's disagnostic talks about source and destination
register, which isn't applicable here.

> --- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> @@ -30,3 +30,9 @@ _start:
>          .byte 0xff
>          #{evex} inc %rax EVEX.vvvv' > 0 (illegal value).
>          .byte 0x62, 0xf4, 0xec, 0x08, 0xff, 0xc0
> +        .byte 0xff, 0xff
> +	# pop2 %rax, %rbx set EVEX.ND=0.
> +        .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
> +        .byte 0xff, 0xff, 0xff
> +	# pop2 %rax, %rsp set EVEX.VVVV=0xf.
> +        .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0

This 2nd comment looks bogus. What is it that's being tested here?

Also again note indentation inconsistencies.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.s
> @@ -0,0 +1,15 @@
> +# Check illegal APX-Push2Pop2 instructions
> +
> +	.allow_index_reg
> +	.text
> +_start:
> +	push2  %eax, %ebx

It's okay to test 32-bit operands, but more important is to test
16-bit ones, as only those could (also) be used with PUSH/POP.

> --- a/opcodes/i386-dis-evex-mod.h
> +++ b/opcodes/i386-dis-evex-mod.h
> @@ -1,4 +1,9 @@
>  /* Nothing at present.  */
> +  /* MOD_EVEX_MAP4_8F_R_0 */
> +  {
> +    { Bad_Opcode },
> +    { PREFIX_TABLE (PREFIX_EVEX_MAP4_8F_R_0_M_1) },
> +  },
>    /* MOD_EVEX_MAP4_DA_PREFIX_1 */
>    {
>      { Bad_Opcode },
> @@ -41,3 +46,8 @@
>    {
>      { "movdiri",	{ Edq, Gdq }, 0 },
>    },
> +  /* MOD_EVEX_MAP4_FF_R_6 */
> +  {
> +    { Bad_Opcode },
> +    { PREFIX_TABLE (PREFIX_EVEX_MAP4_FF_R_6_M_1) },
> +  },

Same comment as before regarding additions to this file.

> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
>[...]
> @@ -9011,6 +9020,8 @@ get_valid_dis386 (const struct dis386 *dp, instr_info *ins)
>  	case 0x4:
>  	  vex_table_index = EVEX_MAP4;
>  	  ins->evex_type = evex_from_legacy;
> +	  if (ins->address_mode != mode_64bit)
> +	    return &bad_opcode;
>  	  break;

This looks to belong into an earlier patch.

> @@ -9073,8 +9084,9 @@ get_valid_dis386 (const struct dis386 *dp, instr_info *ins)
>  	{
>  	  /* EVEX from legacy instructions, when the EVEX.ND bit is 0,
>  	     all bits of EVEX.vvvv and EVEX.V' must be 1.  */
> -	  if (!ins->vex.b && (ins->vex.register_specifier
> -				  || !ins->vex.v))
> +	  if (ins->vex.ll || (!ins->vex.b
> +			      && (ins->vex.register_specifier
> +				  || !ins->vex.v)))
>  	    return &bad_opcode;

This as well.

> @@ -13821,3 +13836,24 @@ PREFETCHI_Fixup (instr_info *ins, int bytemode, int sizeflag)
>  
>    return OP_M (ins, bytemode, sizeflag);
>  }
> +
> +static bool
> +PUSH2_POP2_Fixup (instr_info *ins, int bytemode, int sizeflag)
> +{
> +  unsigned int vvvv_reg = ins->vex.register_specifier
> +    | !ins->vex.v << 4;

Nit: Please parenthesize the shift.

> +  unsigned int rm_reg = ins->modrm.rm + (ins->rex & REX_B ? 8 : 0)
> +    + (ins->rex2 & REX_B ? 16 : 0);
> +
> +  /* Here vex.b is treated as "EVEX.ND.  */
> +  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same registers.  */

The two comments want folding. As to the former, though: How about having

#define nd b

in the EVEX struct declaration (provided we don't have any variables named
"nd" right now), ...

> +  if (!ins->vex.b || vvvv_reg == 0x4 || rm_reg == 0x4

... allowing to use ins->vex.nd here (at which point that comment is
unnecessary)?

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3494,3 +3494,10 @@ erets, 0xf20f01ca, FRED|x64, NoSuf, {}
>  eretu, 0xf30f01ca, FRED|x64, NoSuf, {}
>  
>  // FRED instructions end.
> +
> +// APX Push2/Pop2 instruction.
> +
> +push2, 0xff/6, APX_F, Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
> +push2p, 0xff/6, APX_F, Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
> +pop2, 0x8f/0, APX_F, Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
> +pop2p, 0x8f/0, APX_F, Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }

Like other extensions have it, there also wants to be an "end" comment.

Jan
  
Jan Beulich Nov. 8, 2023, 12:52 p.m. UTC | #2
On 08.11.2023 12:44, Jan Beulich wrote:
> On 02.11.2023 12:29, Cui, Lili wrote:
>> @@ -6854,6 +6858,24 @@ check_VecOperands (const insn_template *t)
>>  	}
>>      }
>>  
>> +  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same registers.  */
>> +  if (t->opcode_modifier.push2pop2)
> 
> I question this way of recognizing these two insns: You introduce a
> whole new table column here just to have two entries set this bit.
> This is cheaper by comparing the mnemonic offsets, as we do elsewhere
> in various cases.

Well, it's 4 rows, not 2, so the mnemonic offset suggestion isn't quite
nice. But there still is no need for a whole new attribute. Just another
OperandConstraint value ought to do.

Jan
  
Jan Beulich Nov. 9, 2023, 9:57 a.m. UTC | #3
On 02.11.2023 12:29, Cui, Lili wrote:
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3494,3 +3494,10 @@ erets, 0xf20f01ca, FRED|x64, NoSuf, {}
>  eretu, 0xf30f01ca, FRED|x64, NoSuf, {}
>  
>  // FRED instructions end.
> +
> +// APX Push2/Pop2 instruction.
> +
> +push2, 0xff/6, APX_F, Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
> +push2p, 0xff/6, APX_F, Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
> +pop2, 0x8f/0, APX_F, Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
> +pop2p, 0x8f/0, APX_F, Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }

I think the use of the VexVVVV attribute here wants to be consistent with
that in the earlier patch used for e.g. NEG: Nominally (i.e. notation-wise)
the last operand is the destination operand in both cases (even if in
practice it's a source one here). The resulting encoding being the same is
merely an effect resulting from ModR/M.reg being used for an opcode
extension in both cases (which takes priority over register assignment in
build_modrm_byte()). But: I came to spot the inconsistency when looking at
patch 7, which keys off of VexVVVVDst. Obviously when making things
consistent here, there would then need to be measures to avoid the
optimization kicking in on the insns here. Hence an alternative to using
VexVVVVDst here would be a clarifying / justifying comment.

Jan
  
Cui, Lili Nov. 22, 2023, 5:48 a.m. UTC | #4
> On 02.11.2023 12:29, Cui, Lili wrote:
> > --- a/gas/config/tc-i386.c
> > +++ b/gas/config/tc-i386.c
> > @@ -256,6 +256,7 @@ enum i386_error
> >      mask_not_on_destination,
> >      no_default_mask,
> >      unsupported_rc_sae,
> > +    unsupported_rsp_register,
> >      invalid_register_operand,
> >      internal_error,
> >    };
> > @@ -5476,6 +5477,9 @@ md_assemble (char *line)
> >  	case unsupported_rc_sae:
> >  	  err_msg = _("unsupported static rounding/sae");
> >  	  break;
> > +	case unsupported_rsp_register:
> > +	  err_msg = _("unsupported rsp register");
> > +	  break;
> 
> Perhaps you mean "cannot be used with" or some such? Also the register
> name needs conditionally prefixing with % in diagnostics.
> 

Done.

> > @@ -6854,6 +6858,24 @@ check_VecOperands (const insn_template *t)
> >  	}
> >      }
> >
> > +  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same
> > + registers.  */  if (t->opcode_modifier.push2pop2)
> 
> I question this way of recognizing these two insns: You introduce a whole new
> table column here just to have two entries set this bit.
> This is cheaper by comparing the mnemonic offsets, as we do elsewhere in
> various cases.
> 

Done.

> I also disagree with putting the check in check_VecOperands():
> There's nothing vector-ish here. Either you put it straight in the caller, or you
> introduce a new check_APX_operands().
> 

How about  putting check_EgprOperands into check_APX_operands ?

      /* Check if EGRPS operands(r16-r31) are valid.  */
      if (check_EgprOperands (t))
        {
          specific_error = progress (i.error);
          continue;
        }

      /* Check if APX operands are valid.  */
      if (check_APX_operands (t))
        {
          specific_error = progress (i.error);
          continue;
        }

> > +    {
> > +      unsigned int reg1 = register_number (i.op[0].regs);
> > +      unsigned int reg2 = register_number (i.op[1].regs);
> > +
> > +      if (reg1 == 0x4 || reg2 == 0x4)
> > +	{
> > +	  i.error = unsupported_rsp_register;
> > +	  return 1;
> > +	}
> > +      if (t->base_opcode == 0x8f && reg1 == reg2)
> > +	{
> > +	  i.error = invalid_dest_and_src_register_set;
> 
> This enumerator's disagnostic talks about source and destination register,
> which isn't applicable here.
> 

Done.

> > --- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> > +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> > @@ -30,3 +30,9 @@ _start:
> >          .byte 0xff
> >          #{evex} inc %rax EVEX.vvvv' > 0 (illegal value).
> >          .byte 0x62, 0xf4, 0xec, 0x08, 0xff, 0xc0
> > +        .byte 0xff, 0xff
> > +	# pop2 %rax, %rbx set EVEX.ND=0.
> > +        .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
> > +        .byte 0xff, 0xff, 0xff
> > +	# pop2 %rax, %rsp set EVEX.VVVV=0xf.
> > +        .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0
> 
> This 2nd comment looks bogus. What is it that's being tested here?
> 

I think it should be  # pop2 %rax set EVEX.vvvv' = 1111. It wants to test that pop2 has only one operand when decoding.

> Also again note indentation inconsistencies.
> 

Done.

> > --- /dev/null
> > +++ b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.s
> > @@ -0,0 +1,15 @@
> > +# Check illegal APX-Push2Pop2 instructions
> > +
> > +	.allow_index_reg
> > +	.text
> > +_start:
> > +	push2  %eax, %ebx
> 
> It's okay to test 32-bit operands, but more important is to test 16-bit ones, as
> only those could (also) be used with PUSH/POP.
> 

Done.

> > --- a/opcodes/i386-dis-evex-mod.h
> > +++ b/opcodes/i386-dis-evex-mod.h
> > @@ -1,4 +1,9 @@
> >  /* Nothing at present.  */
> > +  /* MOD_EVEX_MAP4_8F_R_0 */
> > +  {
> > +    { Bad_Opcode },
> > +    { PREFIX_TABLE (PREFIX_EVEX_MAP4_8F_R_0_M_1) },  },
> >    /* MOD_EVEX_MAP4_DA_PREFIX_1 */
> >    {
> >      { Bad_Opcode },
> > @@ -41,3 +46,8 @@
> >    {
> >      { "movdiri",	{ Edq, Gdq }, 0 },
> >    },
> > +  /* MOD_EVEX_MAP4_FF_R_6 */
> > +  {
> > +    { Bad_Opcode },
> > +    { PREFIX_TABLE (PREFIX_EVEX_MAP4_FF_R_6_M_1) },  },
> 
> Same comment as before regarding additions to this file.
> 

Done.

> > --- a/opcodes/i386-dis.c
> > +++ b/opcodes/i386-dis.c
> >[...]
> > @@ -9011,6 +9020,8 @@ get_valid_dis386 (const struct dis386 *dp,
> instr_info *ins)
> >  	case 0x4:
> >  	  vex_table_index = EVEX_MAP4;
> >  	  ins->evex_type = evex_from_legacy;
> > +	  if (ins->address_mode != mode_64bit)
> > +	    return &bad_opcode;
> >  	  break;
> 
> This looks to belong into an earlier patch.
> 

Done.

> > @@ -9073,8 +9084,9 @@ get_valid_dis386 (const struct dis386 *dp,
> instr_info *ins)
> >  	{
> >  	  /* EVEX from legacy instructions, when the EVEX.ND bit is 0,
> >  	     all bits of EVEX.vvvv and EVEX.V' must be 1.  */
> > -	  if (!ins->vex.b && (ins->vex.register_specifier
> > -				  || !ins->vex.v))
> > +	  if (ins->vex.ll || (!ins->vex.b
> > +			      && (ins->vex.register_specifier
> > +				  || !ins->vex.v)))
> >  	    return &bad_opcode;
> 
> This as well.
> 

Deleted, redundant.

> > @@ -13821,3 +13836,24 @@ PREFETCHI_Fixup (instr_info *ins, int
> > bytemode, int sizeflag)
> >
> >    return OP_M (ins, bytemode, sizeflag);  }
> > +
> > +static bool
> > +PUSH2_POP2_Fixup (instr_info *ins, int bytemode, int sizeflag) {
> > +  unsigned int vvvv_reg = ins->vex.register_specifier
> > +    | !ins->vex.v << 4;
> 
> Nit: Please parenthesize the shift.
> 

Done.

> > +  unsigned int rm_reg = ins->modrm.rm + (ins->rex & REX_B ? 8 : 0)
> > +    + (ins->rex2 & REX_B ? 16 : 0);
> > +
> > +  /* Here vex.b is treated as "EVEX.ND.  */
> > +  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same
> > + registers.  */
> 
> The two comments want folding. As to the former, though: How about having
> 
> #define nd b
> 
> in the EVEX struct declaration (provided we don't have any variables named
> "nd" right now), ...
> 
> > +  if (!ins->vex.b || vvvv_reg == 0x4 || rm_reg == 0x4
> 
> ... allowing to use ins->vex.nd here (at which point that comment is
> unnecessary)?
> 

Done.

> > --- a/opcodes/i386-opc.tbl
> > +++ b/opcodes/i386-opc.tbl
> > @@ -3494,3 +3494,10 @@ erets, 0xf20f01ca, FRED|x64, NoSuf, {}  eretu,
> > 0xf30f01ca, FRED|x64, NoSuf, {}
> >
> >  // FRED instructions end.
> > +
> > +// APX Push2/Pop2 instruction.
> > +
> > +push2, 0xff/6, APX_F,
> >
> +Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No
> _wSuf|No_
> > +lSuf|No_sSuf, { Reg64, Reg64 } push2p, 0xff/6, APX_F,
> >
> +Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No
> _wSuf|No_
> > +lSuf|No_sSuf, { Reg64, Reg64 } pop2, 0x8f/0, APX_F,
> >
> +Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No
> _wSuf|No_
> > +lSuf|No_sSuf, { Reg64, Reg64 } pop2p, 0x8f/0, APX_F,
> >
> +Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No
> _wSuf|No_
> > +lSuf|No_sSuf, { Reg64, Reg64 }
> 
> Like other extensions have it, there also wants to be an "end" comment.

Done.

Thanks,
Lili.
  
Jan Beulich Nov. 22, 2023, 8:53 a.m. UTC | #5
On 22.11.2023 06:48, Cui, Lili wrote:
>> On 02.11.2023 12:29, Cui, Lili wrote:
>>> @@ -6854,6 +6858,24 @@ check_VecOperands (const insn_template *t)
>>>  	}
>>>      }
>>>
>>> +  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same
>>> + registers.  */  if (t->opcode_modifier.push2pop2)
>>
>> I question this way of recognizing these two insns: You introduce a whole new
>> table column here just to have two entries set this bit.
>> This is cheaper by comparing the mnemonic offsets, as we do elsewhere in
>> various cases.
>>
> 
> Done.
> 
>> I also disagree with putting the check in check_VecOperands():
>> There's nothing vector-ish here. Either you put it straight in the caller, or you
>> introduce a new check_APX_operands().
>>
> 
> How about  putting check_EgprOperands into check_APX_operands ?
> 
>       /* Check if EGRPS operands(r16-r31) are valid.  */
>       if (check_EgprOperands (t))
>         {
>           specific_error = progress (i.error);
>           continue;
>         }
> 
>       /* Check if APX operands are valid.  */
>       if (check_APX_operands (t))
>         {
>           specific_error = progress (i.error);
>           continue;
>         }

Hmm, question and suggested code don't fit together. The suggested code
certainly fits what I was suggesting earlier.

>>> --- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
>>> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
>>> @@ -30,3 +30,9 @@ _start:
>>>          .byte 0xff
>>>          #{evex} inc %rax EVEX.vvvv' > 0 (illegal value).
>>>          .byte 0x62, 0xf4, 0xec, 0x08, 0xff, 0xc0
>>> +        .byte 0xff, 0xff
>>> +	# pop2 %rax, %rbx set EVEX.ND=0.
>>> +        .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
>>> +        .byte 0xff, 0xff, 0xff
>>> +	# pop2 %rax, %rsp set EVEX.VVVV=0xf.
>>> +        .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0
>>
>> This 2nd comment looks bogus. What is it that's being tested here?
>>
> 
> I think it should be  # pop2 %rax set EVEX.vvvv' = 1111. It wants to test that pop2 has only one operand when decoding.

But POP2 has two operands, one encoded in EVEX.vvvv. The use of %rsp as
an operand is what I would think is being tested here, but then I don't
see why the comment mentions EVEX.vvvv.

Jan
  
Cui, Lili Nov. 22, 2023, 12:26 p.m. UTC | #6
> >>> --- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> >>> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> >>> @@ -30,3 +30,9 @@ _start:
> >>>          .byte 0xff
> >>>          #{evex} inc %rax EVEX.vvvv' > 0 (illegal value).
> >>>          .byte 0x62, 0xf4, 0xec, 0x08, 0xff, 0xc0
> >>> +        .byte 0xff, 0xff
> >>> +	# pop2 %rax, %rbx set EVEX.ND=0.
> >>> +        .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
> >>> +        .byte 0xff, 0xff, 0xff
> >>> +	# pop2 %rax, %rsp set EVEX.VVVV=0xf.
> >>> +        .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0
> >>
> >> This 2nd comment looks bogus. What is it that's being tested here?
> >>
> >
> > I think it should be  # pop2 %rax set EVEX.vvvv' = 1111. It wants to test that
> pop2 has only one operand when decoding.
> 
> But POP2 has two operands, one encoded in EVEX.vvvv. The use of %rsp as an
> operand is what I would think is being tested here, but then I don't see why
> the comment mentions EVEX.vvvv.
> 

Ok, I changed it to test %rsp.

Thanks,
Lili.
  

Patch

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 5b925505435..7a86aff1828 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -256,6 +256,7 @@  enum i386_error
     mask_not_on_destination,
     no_default_mask,
     unsupported_rc_sae,
+    unsupported_rsp_register,
     invalid_register_operand,
     internal_error,
   };
@@ -5476,6 +5477,9 @@  md_assemble (char *line)
 	case unsupported_rc_sae:
 	  err_msg = _("unsupported static rounding/sae");
 	  break;
+	case unsupported_rsp_register:
+	  err_msg = _("unsupported rsp register");
+	  break;
 	case invalid_register_operand:
 	  err_msg = _("invalid register operand");
 	  break;
@@ -6854,6 +6858,24 @@  check_VecOperands (const insn_template *t)
 	}
     }
 
+  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same registers.  */
+  if (t->opcode_modifier.push2pop2)
+    {
+      unsigned int reg1 = register_number (i.op[0].regs);
+      unsigned int reg2 = register_number (i.op[1].regs);
+
+      if (reg1 == 0x4 || reg2 == 0x4)
+	{
+	  i.error = unsupported_rsp_register;
+	  return 1;
+	}
+      if (t->base_opcode == 0x8f && reg1 == reg2)
+	{
+	  i.error = invalid_dest_and_src_register_set;
+	  return 1;
+	}
+    }
+
   /* Check if broadcast is supported by the instruction and is applied
      to the memory operand.  */
   if (i.broadcast.type || i.broadcast.bytes)
diff --git a/gas/testsuite/gas/i386/apx-push2pop2-inval.l b/gas/testsuite/gas/i386/apx-push2pop2-inval.l
new file mode 100644
index 00000000000..a55a71520c8
--- /dev/null
+++ b/gas/testsuite/gas/i386/apx-push2pop2-inval.l
@@ -0,0 +1,5 @@ 
+.* Assembler messages:
+.*:6: Error: `push2' is only supported in 64-bit mode
+.*:7: Error: `push2p' is only supported in 64-bit mode
+.*:8: Error: `pop2' is only supported in 64-bit mode
+.*:9: Error: `pop2p' is only supported in 64-bit mode
diff --git a/gas/testsuite/gas/i386/apx-push2pop2-inval.s b/gas/testsuite/gas/i386/apx-push2pop2-inval.s
new file mode 100644
index 00000000000..77166327ed1
--- /dev/null
+++ b/gas/testsuite/gas/i386/apx-push2pop2-inval.s
@@ -0,0 +1,9 @@ 
+# Check 32bit APX-PUSH2/POP2 instructions
+
+	.allow_index_reg
+	.text
+_start:
+	push2 %rax, %rbx
+	push2p %rax, %rbx
+	pop2 %rax, %rbx
+	pop2p %rax, %rbx
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index ee74bcd4615..75e1a4ca369 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -509,6 +509,7 @@  if [gas_32_check] then {
     run_dump_test "sm4"
     run_dump_test "sm4-intel"
     run_list_test "pbndkb-inval"
+    run_list_test "apx-push2pop2-inval"
     run_list_test "sg"
     run_dump_test "clzero"
     run_dump_test "invlpgb"
diff --git a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d
index 9060b697c0d..fe652977a54 100644
--- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d
+++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.d
@@ -31,5 +31,7 @@  Disassembly of section .text:
 [ 	]*[a-f0-9]+:[ 	]+00 ff[ 	]+add    %bh,%bh
 [ 	]*[a-f0-9]+:[ 	]+62 f4 ec[ 	]+\(bad\)
 [ 	]*[a-f0-9]+:[ 	]+08 ff[ 	]+or     %bh,%bh
-[ 	]*[a-f0-9]+:[ 	]+c0[ 	]+.byte 0xc0
-#pass
+[ 	]*[a-f0-9]+:[ 	]+c0 ff ff[ 	]+sar    \$0xff,%bh
+[ 	]*[a-f0-9]+:[ 	]+62 f4 64[ 	]+\(bad\)
+[ 	]*[a-f0-9]+:[ 	]+08 8f c0 ff ff ff[ 	]+or     %cl,-0x40\(%rdi\)
+[ 	]*[a-f0-9]+:[ 	]+62 f4 7c 18 8f c0[ 	]+pop2   %rax,\(bad\)
diff --git a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
index d4f4cb72e6e..e6f61a229f0 100644
--- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
+++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
@@ -30,3 +30,9 @@  _start:
         .byte 0xff
         #{evex} inc %rax EVEX.vvvv' > 0 (illegal value).
         .byte 0x62, 0xf4, 0xec, 0x08, 0xff, 0xc0
+        .byte 0xff, 0xff
+	# pop2 %rax, %rbx set EVEX.ND=0.
+        .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
+        .byte 0xff, 0xff, 0xff
+	# pop2 %rax, %rsp set EVEX.VVVV=0xf.
+        .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0
diff --git a/gas/testsuite/gas/i386/x86-64-apx-push2pop2-intel.d b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-intel.d
new file mode 100644
index 00000000000..46b21219582
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-intel.d
@@ -0,0 +1,42 @@ 
+#as: --64
+#objdump: -dw -Mintel
+#name: i386 APX-push2pop2 insns (Intel disassembly)
+#source: x86-64-apx-push2pop2.s
+
+.*: +file format .*
+
+Disassembly of section \.text:
+
+0+ <_start>:
+\s*[a-f0-9]+:\s*62 f4 7c 18 ff f3\s+push2\s+rax,rbx
+\s*[a-f0-9]+:\s*62 fc 3c 18 ff f1\s+push2\s+r8,r17
+\s*[a-f0-9]+:\s*62 d4 04 10 ff f1\s+push2\s+r31,r9
+\s*[a-f0-9]+:\s*62 dc 3c 10 ff f7\s+push2\s+r24,r31
+\s*[a-f0-9]+:\s*62 f4 fc 18 ff f3\s+push2p\s+rax,rbx
+\s*[a-f0-9]+:\s*62 fc bc 18 ff f1\s+push2p\s+r8,r17
+\s*[a-f0-9]+:\s*62 d4 84 10 ff f1\s+push2p\s+r31,r9
+\s*[a-f0-9]+:\s*62 dc bc 10 ff f7\s+push2p\s+r24,r31
+\s*[a-f0-9]+:\s*62 f4 64 18 8f c0\s+pop2\s+rbx,rax
+\s*[a-f0-9]+:\s*62 d4 74 10 8f c0\s+pop2\s+r17,r8
+\s*[a-f0-9]+:\s*62 dc 34 18 8f c7\s+pop2\s+r9,r31
+\s*[a-f0-9]+:\s*62 dc 04 10 8f c0\s+pop2\s+r31,r24
+\s*[a-f0-9]+:\s*62 f4 e4 18 8f c0\s+pop2p\s+rbx,rax
+\s*[a-f0-9]+:\s*62 d4 f4 10 8f c0\s+pop2p\s+r17,r8
+\s*[a-f0-9]+:\s*62 dc b4 18 8f c7\s+pop2p\s+r9,r31
+\s*[a-f0-9]+:\s*62 dc 84 10 8f c0\s+pop2p\s+r31,r24
+\s*[a-f0-9]+:\s*62 f4 7c 18 ff f3\s+push2\s+rax,rbx
+\s*[a-f0-9]+:\s*62 fc 3c 18 ff f1\s+push2\s+r8,r17
+\s*[a-f0-9]+:\s*62 d4 04 10 ff f1\s+push2\s+r31,r9
+\s*[a-f0-9]+:\s*62 dc 3c 10 ff f7\s+push2\s+r24,r31
+\s*[a-f0-9]+:\s*62 f4 fc 18 ff f3\s+push2p\s+rax,rbx
+\s*[a-f0-9]+:\s*62 fc bc 18 ff f1\s+push2p\s+r8,r17
+\s*[a-f0-9]+:\s*62 d4 84 10 ff f1\s+push2p\s+r31,r9
+\s*[a-f0-9]+:\s*62 dc bc 10 ff f7\s+push2p\s+r24,r31
+\s*[a-f0-9]+:\s*62 f4 64 18 8f c0\s+pop2\s+rbx,rax
+\s*[a-f0-9]+:\s*62 d4 74 10 8f c0\s+pop2\s+r17,r8
+\s*[a-f0-9]+:\s*62 dc 34 18 8f c7\s+pop2\s+r9,r31
+\s*[a-f0-9]+:\s*62 dc 04 10 8f c0\s+pop2\s+r31,r24
+\s*[a-f0-9]+:\s*62 f4 e4 18 8f c0\s+pop2p\s+rbx,rax
+\s*[a-f0-9]+:\s*62 d4 f4 10 8f c0\s+pop2p\s+r17,r8
+\s*[a-f0-9]+:\s*62 dc b4 18 8f c7\s+pop2p\s+r9,r31
+\s*[a-f0-9]+:\s*62 dc 84 10 8f c0\s+pop2p\s+r31,r24
diff --git a/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.l b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.l
new file mode 100644
index 00000000000..5eea811a047
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.l
@@ -0,0 +1,11 @@ 
+.* Assembler messages:
+.*:6: Error: operand size mismatch for `push2'
+.*:7: Error: unsupported rsp register for `push2'
+.*:8: Error: unsupported rsp register for `push2'
+.*:9: Error: operand size mismatch for `push2p'
+.*:10: Error: unsupported rsp register for `push2p'
+.*:11: Error: unsupported rsp register for `pop2'
+.*:12: Error: unsupported rsp register for `pop2'
+.*:13: Error: destination and source registers must be distinct for `pop2'
+.*:14: Error: unsupported rsp register for `pop2p'
+.*:15: Error: destination and source registers must be distinct for `pop2p'
diff --git a/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.s b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.s
new file mode 100644
index 00000000000..c0cd9c3ce89
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-apx-push2pop2-inval.s
@@ -0,0 +1,15 @@ 
+# Check illegal APX-Push2Pop2 instructions
+
+	.allow_index_reg
+	.text
+_start:
+	push2  %eax, %ebx
+	push2  %rsp, %r17
+	push2  %r17, %rsp
+	push2p %eax, %ebx
+	push2p %rsp, %r17
+	pop2   %rax, %rsp
+	pop2   %rsp, %rax
+	pop2   %r12, %r12
+	pop2p  %rax, %rsp
+	pop2p  %r12, %r12
diff --git a/gas/testsuite/gas/i386/x86-64-apx-push2pop2.d b/gas/testsuite/gas/i386/x86-64-apx-push2pop2.d
new file mode 100644
index 00000000000..54f22a7f94e
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-apx-push2pop2.d
@@ -0,0 +1,42 @@ 
+#as: --64
+#objdump: -dw
+#name: x86_64 APX-push2pop2 insns
+#source: x86-64-apx-push2pop2.s
+
+.*: +file format .*
+
+Disassembly of section \.text:
+
+0+ <_start>:
+\s*[a-f0-9]+:\s*62 f4 7c 18 ff f3\s+push2\s+%rbx,%rax
+\s*[a-f0-9]+:\s*62 fc 3c 18 ff f1\s+push2\s+%r17,%r8
+\s*[a-f0-9]+:\s*62 d4 04 10 ff f1\s+push2\s+%r9,%r31
+\s*[a-f0-9]+:\s*62 dc 3c 10 ff f7\s+push2\s+%r31,%r24
+\s*[a-f0-9]+:\s*62 f4 fc 18 ff f3\s+push2p\s+%rbx,%rax
+\s*[a-f0-9]+:\s*62 fc bc 18 ff f1\s+push2p\s+%r17,%r8
+\s*[a-f0-9]+:\s*62 d4 84 10 ff f1\s+push2p\s+%r9,%r31
+\s*[a-f0-9]+:\s*62 dc bc 10 ff f7\s+push2p\s+%r31,%r24
+\s*[a-f0-9]+:\s*62 f4 64 18 8f c0\s+pop2\s+%rax,%rbx
+\s*[a-f0-9]+:\s*62 d4 74 10 8f c0\s+pop2\s+%r8,%r17
+\s*[a-f0-9]+:\s*62 dc 34 18 8f c7\s+pop2\s+%r31,%r9
+\s*[a-f0-9]+:\s*62 dc 04 10 8f c0\s+pop2\s+%r24,%r31
+\s*[a-f0-9]+:\s*62 f4 e4 18 8f c0\s+pop2p\s+%rax,%rbx
+\s*[a-f0-9]+:\s*62 d4 f4 10 8f c0\s+pop2p\s+%r8,%r17
+\s*[a-f0-9]+:\s*62 dc b4 18 8f c7\s+pop2p\s+%r31,%r9
+\s*[a-f0-9]+:\s*62 dc 84 10 8f c0\s+pop2p\s+%r24,%r31
+\s*[a-f0-9]+:\s*62 f4 7c 18 ff f3\s+push2\s+%rbx,%rax
+\s*[a-f0-9]+:\s*62 fc 3c 18 ff f1\s+push2\s+%r17,%r8
+\s*[a-f0-9]+:\s*62 d4 04 10 ff f1\s+push2\s+%r9,%r31
+\s*[a-f0-9]+:\s*62 dc 3c 10 ff f7\s+push2\s+%r31,%r24
+\s*[a-f0-9]+:\s*62 f4 fc 18 ff f3\s+push2p\s+%rbx,%rax
+\s*[a-f0-9]+:\s*62 fc bc 18 ff f1\s+push2p\s+%r17,%r8
+\s*[a-f0-9]+:\s*62 d4 84 10 ff f1\s+push2p\s+%r9,%r31
+\s*[a-f0-9]+:\s*62 dc bc 10 ff f7\s+push2p\s+%r31,%r24
+\s*[a-f0-9]+:\s*62 f4 64 18 8f c0\s+pop2\s+%rax,%rbx
+\s*[a-f0-9]+:\s*62 d4 74 10 8f c0\s+pop2\s+%r8,%r17
+\s*[a-f0-9]+:\s*62 dc 34 18 8f c7\s+pop2\s+%r31,%r9
+\s*[a-f0-9]+:\s*62 dc 04 10 8f c0\s+pop2\s+%r24,%r31
+\s*[a-f0-9]+:\s*62 f4 e4 18 8f c0\s+pop2p\s+%rax,%rbx
+\s*[a-f0-9]+:\s*62 d4 f4 10 8f c0\s+pop2p\s+%r8,%r17
+\s*[a-f0-9]+:\s*62 dc b4 18 8f c7\s+pop2p\s+%r31,%r9
+\s*[a-f0-9]+:\s*62 dc 84 10 8f c0\s+pop2p\s+%r24,%r31
diff --git a/gas/testsuite/gas/i386/x86-64-apx-push2pop2.s b/gas/testsuite/gas/i386/x86-64-apx-push2pop2.s
new file mode 100644
index 00000000000..4cfc0a2185f
--- /dev/null
+++ b/gas/testsuite/gas/i386/x86-64-apx-push2pop2.s
@@ -0,0 +1,39 @@ 
+# Check 64bit APX-Push2Pop2 instructions
+
+	.allow_index_reg
+	.text
+_start:
+	push2 %rbx, %rax
+	push2 %r17, %r8
+	push2 %r9, %r31
+	push2 %r31, %r24
+	push2p %rbx, %rax
+	push2p %r17, %r8
+	push2p %r9, %r31
+	push2p %r31, %r24
+	pop2 %rax, %rbx
+	pop2 %r8, %r17
+	pop2 %r31, %r9
+	pop2 %r24, %r31
+	pop2p %rax, %rbx
+	pop2p %r8, %r17
+	pop2p %r31, %r9
+	pop2p %r24, %r31
+
+.intel_syntax noprefix
+	push2 rax, rbx
+	push2 r8, r17
+	push2 r31, r9
+	push2 r24, r31
+	push2p rax, rbx
+	push2p r8, r17
+	push2p r31, r9
+	push2p r24, r31
+	pop2 rbx, rax
+	pop2 r17, r8
+	pop2 r9, r31
+	pop2 r31, r24
+	pop2p rbx, rax
+	pop2p r17, r8
+	pop2p r9, r31
+	pop2p r31, r24
diff --git a/gas/testsuite/gas/i386/x86-64.exp b/gas/testsuite/gas/i386/x86-64.exp
index 07cb716d2a5..668b366a212 100644
--- a/gas/testsuite/gas/i386/x86-64.exp
+++ b/gas/testsuite/gas/i386/x86-64.exp
@@ -342,6 +342,9 @@  run_dump_test "x86-64-avx512dq-rcigrd-intel"
 run_dump_test "x86-64-avx512dq-rcigrd"
 run_dump_test "x86-64-avx512dq-rcigrne-intel"
 run_dump_test "x86-64-avx512dq-rcigrne"
+run_dump_test "x86-64-apx-push2pop2"
+run_dump_test "x86-64-apx-push2pop2-intel"
+run_list_test "x86-64-apx-push2pop2-inval"
 run_dump_test "x86-64-avx512dq-rcigru-intel"
 run_dump_test "x86-64-avx512dq-rcigru"
 run_dump_test "x86-64-avx512dq-rcigrz-intel"
diff --git a/opcodes/i386-dis-evex-mod.h b/opcodes/i386-dis-evex-mod.h
index a60c19add3c..515039b431c 100644
--- a/opcodes/i386-dis-evex-mod.h
+++ b/opcodes/i386-dis-evex-mod.h
@@ -1,4 +1,9 @@ 
 /* Nothing at present.  */
+  /* MOD_EVEX_MAP4_8F_R_0 */
+  {
+    { Bad_Opcode },
+    { PREFIX_TABLE (PREFIX_EVEX_MAP4_8F_R_0_M_1) },
+  },
   /* MOD_EVEX_MAP4_DA_PREFIX_1 */
   {
     { Bad_Opcode },
@@ -41,3 +46,8 @@ 
   {
     { "movdiri",	{ Edq, Gdq }, 0 },
   },
+  /* MOD_EVEX_MAP4_FF_R_6 */
+  {
+    { Bad_Opcode },
+    { PREFIX_TABLE (PREFIX_EVEX_MAP4_FF_R_6_M_1) },
+  },
diff --git a/opcodes/i386-dis-evex-prefix.h b/opcodes/i386-dis-evex-prefix.h
index 09d8c10bdfd..f8cc5b492c0 100644
--- a/opcodes/i386-dis-evex-prefix.h
+++ b/opcodes/i386-dis-evex-prefix.h
@@ -338,6 +338,10 @@ 
     { "vcmpp%XH", { MaskG, Vex, EXxh, EXxEVexS, CMP }, 0 },
     { "vcmps%XH", { MaskG, VexScalar, EXw, EXxEVexS, CMP }, 0 },
   },
+  /* PREFIX_EVEX_MAP4_8F_R_0_M_1 */
+  {
+    { VEX_W_TABLE (EVEX_W_MAP4_8F_R_0_M_1_P_0) }
+  },
   /* PREFIX_EVEX_MAP4_D8 */
   {
     { "sha1nexte", { XM, EXxmm }, 0 },
@@ -403,6 +407,10 @@ 
     { "aand",	{ Mdq, Gdq }, 0 },
     { "aor",	{ Mdq, Gdq }, 0 },
   },
+  /* PREFIX_EVEX_MAP4_FF_R_6_M_1 */
+  {
+    { VEX_W_TABLE (EVEX_W_MAP4_FF_R_6_M_1_P_0) },
+  },
   /* PREFIX_EVEX_MAP5_10 */
   {
     { Bad_Opcode },
diff --git a/opcodes/i386-dis-evex-reg.h b/opcodes/i386-dis-evex-reg.h
index b75558c40ca..6bc0c26116f 100644
--- a/opcodes/i386-dis-evex-reg.h
+++ b/opcodes/i386-dis-evex-reg.h
@@ -86,6 +86,10 @@ 
     { "subQ",	{ VexGv, Ev, sIb }, 0 },
     { "xorQ",	{ VexGv, Ev, sIb }, 0 },
   },
+  /* REG_EVEX_MAP4_8F */
+  {
+    { MOD_TABLE (MOD_EVEX_MAP4_8F_R_0) },
+  },
   /* REG_EVEX_MAP4_D8_PREFIX_1 */
   {
     { "aesencwide128kl",	{ M }, 0 },
@@ -116,4 +120,9 @@ 
   {
     { "incQ",   { VexGv ,Ev }, 0 },
     { "decQ",   { VexGv ,Ev }, 0 },
+    { Bad_Opcode },
+    { Bad_Opcode },
+    { Bad_Opcode },
+    { Bad_Opcode },
+    { MOD_TABLE (MOD_EVEX_MAP4_FF_R_6) },
   },
diff --git a/opcodes/i386-dis-evex-w.h b/opcodes/i386-dis-evex-w.h
index b828277d413..ad3db92888c 100644
--- a/opcodes/i386-dis-evex-w.h
+++ b/opcodes/i386-dis-evex-w.h
@@ -442,6 +442,16 @@ 
     { Bad_Opcode },
     { "vpshrdw",   { XM, Vex, EXx, Ib }, 0 },
   },
+  /* EVEX_W_MAP4_8F_R_0_M_1_P_0 */
+  {
+    { "pop2", { { PUSH2_POP2_Fixup, q_mode}, Eq }, 0 },
+    { "pop2p", { { PUSH2_POP2_Fixup, q_mode}, Eq }, 0 },
+  },
+  /* EVEX_W_MAP4_FF_R_6_M_1_P_0 */
+  {
+    { "push2", { { PUSH2_POP2_Fixup, q_mode}, Eq }, 0 },
+    { "push2p", { { PUSH2_POP2_Fixup, q_mode}, Eq }, 0 },
+  },
   /* EVEX_W_MAP5_5B_P_0 */
   {
     { "vcvtdq2ph%XY",	{ XMxmmq, EXx, EXxEVexR }, 0 },
diff --git a/opcodes/i386-dis-evex.h b/opcodes/i386-dis-evex.h
index ef752f417c5..7ad7b5b2cf8 100644
--- a/opcodes/i386-dis-evex.h
+++ b/opcodes/i386-dis-evex.h
@@ -1035,7 +1035,7 @@  static const struct dis386 evex_table[][256] = {
     { Bad_Opcode },
     { Bad_Opcode },
     { Bad_Opcode },
-    { Bad_Opcode },
+    { REG_TABLE (REG_EVEX_MAP4_8F) },
     /* 90 */
     { Bad_Opcode },
     { Bad_Opcode },
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 0de3959cf80..825b14ad0dd 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -105,6 +105,7 @@  static bool FXSAVE_Fixup (instr_info *, int, int);
 static bool MOVSXD_Fixup (instr_info *, int, int);
 static bool DistinctDest_Fixup (instr_info *, int, int);
 static bool PREFETCHI_Fixup (instr_info *, int, int);
+static bool PUSH2_POP2_Fixup (instr_info *, int, int);
 
 static void ATTRIBUTE_PRINTF_3 i386_dis_printf (const disassemble_info *,
 						enum disassembler_style,
@@ -890,6 +891,7 @@  enum
   REG_EVEX_MAP4_80,
   REG_EVEX_MAP4_81,
   REG_EVEX_MAP4_83,
+  REG_EVEX_MAP4_8F,
   REG_EVEX_MAP4_D8_PREFIX_1,
   REG_EVEX_MAP4_F6,
   REG_EVEX_MAP4_F7,
@@ -935,6 +937,7 @@  enum
 
   MOD_VEX_0F3849_X86_64_L_0_W_0,
 
+  MOD_EVEX_MAP4_8F_R_0,
   MOD_EVEX_MAP4_DA_PREFIX_1,
   MOD_EVEX_MAP4_DB_PREFIX_1,
   MOD_EVEX_MAP4_DC_PREFIX_1,
@@ -945,6 +948,7 @@  enum
   MOD_EVEX_MAP4_F8_PREFIX_2,
   MOD_EVEX_MAP4_F8_PREFIX_3,
   MOD_EVEX_MAP4_F9,
+  MOD_EVEX_MAP4_FF_R_6,
 };
 
 enum
@@ -1180,6 +1184,7 @@  enum
   PREFIX_EVEX_0F3A67,
   PREFIX_EVEX_0F3AC2,
 
+  PREFIX_EVEX_MAP4_8F_R_0_M_1,
   PREFIX_EVEX_MAP4_D8,
   PREFIX_EVEX_MAP4_DA,
   PREFIX_EVEX_MAP4_DB,
@@ -1192,6 +1197,7 @@  enum
   PREFIX_EVEX_MAP4_F2,
   PREFIX_EVEX_MAP4_F8,
   PREFIX_EVEX_MAP4_FC,
+  PREFIX_EVEX_MAP4_FF_R_6_M_1,
 
   PREFIX_EVEX_MAP5_10,
   PREFIX_EVEX_MAP5_11,
@@ -1752,6 +1758,9 @@  enum
   EVEX_W_0F3A70,
   EVEX_W_0F3A72,
 
+  EVEX_W_MAP4_8F_R_0_M_1_P_0,
+  EVEX_W_MAP4_FF_R_6_M_1_P_0,
+
   EVEX_W_MAP5_5B_P_0,
   EVEX_W_MAP5_7A_P_3,
 };
@@ -9011,6 +9020,8 @@  get_valid_dis386 (const struct dis386 *dp, instr_info *ins)
 	case 0x4:
 	  vex_table_index = EVEX_MAP4;
 	  ins->evex_type = evex_from_legacy;
+	  if (ins->address_mode != mode_64bit)
+	    return &bad_opcode;
 	  break;
 	case 0x5:
 	  vex_table_index = EVEX_MAP5;
@@ -9073,8 +9084,9 @@  get_valid_dis386 (const struct dis386 *dp, instr_info *ins)
 	{
 	  /* EVEX from legacy instructions, when the EVEX.ND bit is 0,
 	     all bits of EVEX.vvvv and EVEX.V' must be 1.  */
-	  if (!ins->vex.b && (ins->vex.register_specifier
-				  || !ins->vex.v))
+	  if (ins->vex.ll || (!ins->vex.b
+			      && (ins->vex.register_specifier
+				  || !ins->vex.v)))
 	    return &bad_opcode;
 	  ins->rex |= REX_OPCODE;
 	}
@@ -13437,6 +13449,9 @@  OP_VEX (instr_info *ins, int bytemode, int sizeflag ATTRIBUTE_UNUSED)
 	case b_mode:
 	  names = att_names8rex;
 	  break;
+	case q_mode:
+	  names = att_names64;
+	  break;
 	case mask_bd_mode:
 	case mask_mode:
 	  if (reg > 0x7)
@@ -13821,3 +13836,24 @@  PREFETCHI_Fixup (instr_info *ins, int bytemode, int sizeflag)
 
   return OP_M (ins, bytemode, sizeflag);
 }
+
+static bool
+PUSH2_POP2_Fixup (instr_info *ins, int bytemode, int sizeflag)
+{
+  unsigned int vvvv_reg = ins->vex.register_specifier
+    | !ins->vex.v << 4;
+  unsigned int rm_reg = ins->modrm.rm + (ins->rex & REX_B ? 8 : 0)
+    + (ins->rex2 & REX_B ? 16 : 0);
+
+  /* Here vex.b is treated as "EVEX.ND.  */
+  /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same registers.  */
+  if (!ins->vex.b || vvvv_reg == 0x4 || rm_reg == 0x4
+      || (!ins->modrm.reg
+	  && vvvv_reg == rm_reg))
+    {
+      oappend (ins, "(bad)");
+      return true;
+    }
+
+  return OP_VEX (ins, bytemode, sizeflag);
+}
diff --git a/opcodes/i386-gen.c b/opcodes/i386-gen.c
index 2e6ae807bbe..144ec129a32 100644
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -474,6 +474,7 @@  static bitfield opcode_modifiers[] =
   BITFIELD (ISA64),
   BITFIELD (NoEgpr),
   BITFIELD (NF),
+  BITFIELD (Push2Pop2),
 };
 
 #define CLASS(n) #n, n
diff --git a/opcodes/i386-opc.h b/opcodes/i386-opc.h
index bb826bbdb34..4b6d52d29cb 100644
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -755,6 +755,9 @@  enum
   /* No CSPAZO flags update indication.  */
   NF,
 
+  /* APX Push2Pop2 bit  */
+  Push2Pop2,
+
   /* The last bitfield in i386_opcode_modifier.  */
   Opcode_Modifier_Num
 };
@@ -804,6 +807,7 @@  typedef struct i386_opcode_modifier
   unsigned int isa64:2;
   unsigned int noegpr:1;
   unsigned int nf:1;
+  unsigned int push2pop2:1;
 } i386_opcode_modifier;
 
 /* Operand classes.  */
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index b1f7491e7d7..03ebef028f9 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -3494,3 +3494,10 @@  erets, 0xf20f01ca, FRED|x64, NoSuf, {}
 eretu, 0xf30f01ca, FRED|x64, NoSuf, {}
 
 // FRED instructions end.
+
+// APX Push2/Pop2 instruction.
+
+push2, 0xff/6, APX_F, Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
+push2p, 0xff/6, APX_F, Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
+pop2, 0x8f/0, APX_F, Modrm|VexW0|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
+pop2p, 0x8f/0, APX_F, Modrm|VexW1|EVex128|Push2Pop2|EVexMap4|VexVVVVSrc|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }