On 24.11.2023 08:02, Cui, Lili wrote:
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -248,6 +248,7 @@ enum i386_error
> invalid_vector_register_set,
> invalid_tmm_register_set,
> invalid_dest_and_src_register_set,
> + invalid_src_register_set,
> invalid_pseudo_prefix,
> unsupported_vector_index_register,
> unsupported_broadcast,
> @@ -256,6 +257,7 @@ enum i386_error
> mask_not_on_destination,
> no_default_mask,
> unsupported_rc_sae,
> + unsupported_rsp_register,
> invalid_register_operand,
> internal_error,
> };
> @@ -5398,6 +5400,9 @@ md_assemble (char *line)
> case invalid_dest_and_src_register_set:
> err_msg = _("destination and source registers must be distinct");
> break;
> + case invalid_src_register_set:
Did you mean invalid_dest_register_set and ...
> + err_msg = _("two source registers must be distinct");
... "two destination ..."? This is for POP2, after all, which has no source
register at all.
> @@ -5422,6 +5427,9 @@ md_assemble (char *line)
> case unsupported_rc_sae:
> err_msg = _("unsupported static rounding/sae");
> break;
> + case unsupported_rsp_register:
> + err_msg = _("cannot be used with %rsp register");
> + break;
While this wording looks okay as visible here, please consider it in the
context it is used in: "cannot be used with %rsp register for `push2'"
is, I'm sorry to say that, clumsy at best. If you want to stick to setting
err_msg, how about "%rsp register cannot be used"? Personally I'd prefer a
resulting output of "%rsp register cannot be used with `push2'", but I
wouldn't insist on you going that route if you don't like that.
> @@ -7113,6 +7121,33 @@ check_EgprOperands (const insn_template *t)
> return 0;
> }
>
> +/* Check if APX operands are valid for the instruction. */
> +static int
Please can functions returning boolean indicators have a return type of
"bool" (and perhaps use "true" as the success indicator, not "false")?
> +check_APX_operands (const insn_template *t)
> +{
> + /* Push2* and Pop2* cannot use RSP and Pop2* cannot pop two same registers.
> + */
> + if (t->mnem_off == MN_push2 || t->mnem_off == MN_push2p
> + || t->mnem_off == MN_pop2 || t->mnem_off == MN_pop2p)
Considering (perhaps just theoretical) further additions here, did you
consider using switch()? Even without further additions this would imo
be more legible (due to there being slightly less redundancy).
> --- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> @@ -28,3 +28,9 @@ _start:
> .byte 0xff
> #{evex} inc %rax %rbx EVEX.vvvv' != 1111 && EVEX.ND = 0.
> .insn EVEX.L0.NP.M4.W1 0xff, %rax, %rbx
> + .byte 0xff
> + # pop2 %rax, %rbx set EVEX.ND=0.
> + .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
> + .byte 0xff, 0xff, 0xff
> + # pop2 %rax set EVEX.vvvv' = 1111.
Another instance of the unclear EVEX.vvvv' (i.e. the questionable nature
if ' here). Yet then - what is the test below checking? EVEX.vvvv encodes
one of the two operands, so all values are valid? Isn't this about both
operands being the same? That would better be said then explicitly, e.g.
simply
# pop2 %rax, %rax (twice same destination)
> + .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0
Also again both new tests use .byte instead of .insn: Is there a particular
reason? Here are a couple of examples that I have readily available (Intel
syntax again, ftaod):
.insn EVEX.L0.M4.W0 0x8f/0, r8, rax{sae} ; pop2 r8, rax
.insn EVEX.L0.M4.W0 0x8f/0, xmm16, rax{sae} ; pop2 r16, rax
.insn EVEX.L0.M4.W0 0x8f/0, rax, r8{sae} ; pop2 rax, r8
.insn EVEX.L0.M12.W0 0x8f/0, rax, rax{sae} ; pop2 rax, r16
.insn EVEX.L0.M4.W1 0x8f/0, rax, rcx{sae} ; pop2.x rax, rcx
I'm sure you can derive from them what you're actually after.
> --- /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
Nit: Un-indented directive again.
> --- 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,
> @@ -225,6 +226,9 @@ struct instr_info
> }
> vex;
>
> +/* For APX EVEX-promoted prefix, EVEX.ND shares the same bit as vex.b. */
> +#define nd b
Can this be moved ahead to patch 4, such that it can be used there (instead
of vex.b) as well? IOW ...
> @@ -9125,7 +9133,7 @@ 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->evex_type == evex_from_legacy && !ins->vex.b
> + if (ins->evex_type == evex_from_legacy && !ins->vex.nd
> && (ins->vex.register_specifier || !ins->vex.v))
> return &bad_opcode;
... neither this nor ...
> @@ -13388,11 +13396,10 @@ OP_VEX (instr_info *ins, int bytemode, int sizeflag ATTRIBUTE_UNUSED)
> if (!ins->need_vex)
> return true;
>
> - /* Here vex.b is treated as "EVEX.ND". */
> if (ins->evex_type == evex_from_legacy)
> {
> ins->evex_used |= EVEX_b_used;
> - if (!ins->vex.b)
> + if (!ins->vex.nd)
> return true;
> }
... this should require touching here.
> @@ -13884,3 +13894,26 @@ 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)
> +{
> + if (ins->modrm.mod != 3 || !ins->vex.b)
Did you mean vex.nd? Plus, considering the vex.nd check further down, why
is this checked both here and there?
> + return true;
Doesn't this result in silently bogus/wrong output? Shouldn't you print
"(bad)" like you do further down? At which point it may make sense to
simply fold both if()s?
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -807,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;
Still a new modifier despite my earlier request to avoid adding one when
you easily can? Here OperandConstraint is actually fully applicable to
use, as what you want to enforce is a constraint on operands.
Jan
> On 24.11.2023 08:02, Cui, Lili wrote:
> > --- a/gas/config/tc-i386.c
> > +++ b/gas/config/tc-i386.c
> > @@ -248,6 +248,7 @@ enum i386_error
> > invalid_vector_register_set,
> > invalid_tmm_register_set,
> > invalid_dest_and_src_register_set,
> > + invalid_src_register_set,
> > invalid_pseudo_prefix,
> > unsupported_vector_index_register,
> > unsupported_broadcast,
> > @@ -256,6 +257,7 @@ enum i386_error
> > mask_not_on_destination,
> > no_default_mask,
> > unsupported_rc_sae,
> > + unsupported_rsp_register,
> > invalid_register_operand,
> > internal_error,
> > };
> > @@ -5398,6 +5400,9 @@ md_assemble (char *line)
> > case invalid_dest_and_src_register_set:
> > err_msg = _("destination and source registers must be distinct");
> > break;
> > + case invalid_src_register_set:
>
> Did you mean invalid_dest_register_set and ...
>
> > + err_msg = _("two source registers must be distinct");
>
> ... "two destination ..."? This is for POP2, after all, which has no source register
> at all.
>
Done.
> > @@ -5422,6 +5427,9 @@ md_assemble (char *line)
> > case unsupported_rc_sae:
> > err_msg = _("unsupported static rounding/sae");
> > break;
> > + case unsupported_rsp_register:
> > + err_msg = _("cannot be used with %rsp register");
> > + break;
>
> While this wording looks okay as visible here, please consider it in the context
> it is used in: "cannot be used with %rsp register for `push2'"
> is, I'm sorry to say that, clumsy at best. If you want to stick to setting err_msg,
> how about "%rsp register cannot be used"? Personally I'd prefer a resulting
> output of "%rsp register cannot be used with `push2'", but I wouldn't insist on
> you going that route if you don't like that.
>
"%rsp register cannot be used" ,this is much better, thanks.
> > @@ -7113,6 +7121,33 @@ check_EgprOperands (const insn_template *t)
> > return 0;
> > }
> >
> > +/* Check if APX operands are valid for the instruction. */ static
> > +int
>
> Please can functions returning boolean indicators have a return type of "bool"
> (and perhaps use "true" as the success indicator, not "false")?
>
Done.
> > +check_APX_operands (const insn_template *t) {
> > + /* Push2* and Pop2* cannot use RSP and Pop2* cannot pop two same
> registers.
> > + */
> > + if (t->mnem_off == MN_push2 || t->mnem_off == MN_push2p
> > + || t->mnem_off == MN_pop2 || t->mnem_off == MN_pop2p)
>
> Considering (perhaps just theoretical) further additions here, did you consider
> using switch()? Even without further additions this would imo be more legible
> (due to there being slightly less redundancy).
>
Done.
/* Push2* and Pop2* cannot use RSP and Pop2* cannot pop two same registers.
*/
switch (t->mnem_off)
{
case MN_pop2:
case MN_pop2p:
if (register_number (i.op[0].regs) == register_number (i.op[1].regs))
{
i.error = invalid_dest_register_set;
return 1;
}
case MN_push2:
case MN_push2p:
if (register_number (i.op[0].regs) == 4
|| register_number (i.op[1].regs) == 4)
{
i.error = unsupported_rsp_register;
return 1;
}
}
> > --- a/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> > +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> > @@ -28,3 +28,9 @@ _start:
> > .byte 0xff
> > #{evex} inc %rax %rbx EVEX.vvvv' != 1111 && EVEX.ND = 0.
> > .insn EVEX.L0.NP.M4.W1 0xff, %rax, %rbx
> > + .byte 0xff
> > + # pop2 %rax, %rbx set EVEX.ND=0.
> > + .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
> > + .byte 0xff, 0xff, 0xff
> > + # pop2 %rax set EVEX.vvvv' = 1111.
>
> Another instance of the unclear EVEX.vvvv' (i.e. the questionable nature if '
> here). Yet then - what is the test below checking? EVEX.vvvv encodes one of
> the two operands, so all values are valid? Isn't this about both operands being
> the same? That would better be said then explicitly, e.g.
> simply
>
> # pop2 %rax, %rax (twice same destination)
>
> > + .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0
>
> Also again both new tests use .byte instead of .insn: Is there a particular
> reason? Here are a couple of examples that I have readily available (Intel
> syntax again, ftaod):
>
> .insn EVEX.L0.M4.W0 0x8f/0, r8, rax{sae} ; pop2 r8, rax
> .insn EVEX.L0.M4.W0 0x8f/0, xmm16, rax{sae} ; pop2 r16, rax
> .insn EVEX.L0.M4.W0 0x8f/0, rax, r8{sae} ; pop2 rax, r8
> .insn EVEX.L0.M12.W0 0x8f/0, rax, rax{sae} ; pop2 rax, r16
> .insn EVEX.L0.M4.W1 0x8f/0, rax, rcx{sae} ; pop2.x rax, rcx
>
> I'm sure you can derive from them what you're actually after.
>
Thanks!
# pop2 %rax, %r8 set EVEX.ND=0.
.insn EVEX.L0.M4.W0 0x8f/0, %rax, %r8
.byte 0xff, 0xff, 0xff
# pop2 %rax, %r8 set EVEX.vvvv = 1111.
.insn EVEX.L0.M4.W0 0x8f, %rax, {rn-sae},%r8
# pop2 %r8, %r8.
.insn EVEX.L0.M4.W0 0x8f/0, %r8,{rn-sae}, %r8
> > --- /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
>
> Nit: Un-indented directive again.
Done.
>
> > --- 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,
> > @@ -225,6 +226,9 @@ struct instr_info
> > }
> > vex;
> >
> > +/* For APX EVEX-promoted prefix, EVEX.ND shares the same bit as
> > +vex.b. */ #define nd b
>
> Can this be moved ahead to patch 4, such that it can be used there (instead of
> vex.b) as well? IOW ...
>
> > @@ -9125,7 +9133,7 @@ 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->evex_type == evex_from_legacy && !ins->vex.b
> > + if (ins->evex_type == evex_from_legacy && !ins->vex.nd
> > && (ins->vex.register_specifier || !ins->vex.v))
> > return &bad_opcode;
>
> ... neither this nor ...
>
> > @@ -13388,11 +13396,10 @@ OP_VEX (instr_info *ins, int bytemode, int
> sizeflag ATTRIBUTE_UNUSED)
> > if (!ins->need_vex)
> > return true;
> >
> > - /* Here vex.b is treated as "EVEX.ND". */
> > if (ins->evex_type == evex_from_legacy)
> > {
> > ins->evex_used |= EVEX_b_used;
> > - if (!ins->vex.b)
> > + if (!ins->vex.nd)
> > return true;
> > }
>
> ... this should require touching here.
>
I moved them into NDD patch, , which adds these checks.
> > @@ -13884,3 +13894,26 @@ 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) {
> > + if (ins->modrm.mod != 3 || !ins->vex.b)
>
> Did you mean vex.nd? Plus, considering the vex.nd check further down, why is
> this checked both here and there?
>
Dropped.
> > + return true;
>
> Doesn't this result in silently bogus/wrong output? Shouldn't you print
> "(bad)" like you do further down? At which point it may make sense to simply
> fold both if()s?
>
> > --- a/opcodes/i386-opc.h
> > +++ b/opcodes/i386-opc.h
> > @@ -807,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;
>
> Still a new modifier despite my earlier request to avoid adding one when you
> easily can? Here OperandConstraint is actually fully applicable to use, as what
> you want to enforce is a constraint on operands.
>
I also found this issue and removed it locally.
Thanks,
Lili.
@@ -248,6 +248,7 @@ enum i386_error
invalid_vector_register_set,
invalid_tmm_register_set,
invalid_dest_and_src_register_set,
+ invalid_src_register_set,
invalid_pseudo_prefix,
unsupported_vector_index_register,
unsupported_broadcast,
@@ -256,6 +257,7 @@ enum i386_error
mask_not_on_destination,
no_default_mask,
unsupported_rc_sae,
+ unsupported_rsp_register,
invalid_register_operand,
internal_error,
};
@@ -5398,6 +5400,9 @@ md_assemble (char *line)
case invalid_dest_and_src_register_set:
err_msg = _("destination and source registers must be distinct");
break;
+ case invalid_src_register_set:
+ err_msg = _("two source registers must be distinct");
+ break;
case invalid_pseudo_prefix:
err_msg = _("rex2 pseudo prefix cannot be used here");
break;
@@ -5422,6 +5427,9 @@ md_assemble (char *line)
case unsupported_rc_sae:
err_msg = _("unsupported static rounding/sae");
break;
+ case unsupported_rsp_register:
+ err_msg = _("cannot be used with %rsp register");
+ break;
case invalid_register_operand:
err_msg = _("invalid register operand");
break;
@@ -7113,6 +7121,33 @@ check_EgprOperands (const insn_template *t)
return 0;
}
+/* Check if APX operands are valid for the instruction. */
+static int
+check_APX_operands (const insn_template *t)
+{
+ /* Push2* and Pop2* cannot use RSP and Pop2* cannot pop two same registers.
+ */
+ if (t->mnem_off == MN_push2 || t->mnem_off == MN_push2p
+ || t->mnem_off == MN_pop2 || t->mnem_off == MN_pop2p)
+ {
+ 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_src_register_set;
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/* Helper function for the progress() macro in match_template(). */
static INLINE enum i386_error progress (enum i386_error new,
enum i386_error last,
@@ -7606,6 +7641,13 @@ match_template (char mnem_suffix)
continue;
}
+ /* Check if APX operands are valid. */
+ if (check_APX_operands (t))
+ {
+ specific_error = progress (i.error);
+ continue;
+ }
+
/* Check whether to use the shorter VEX encoding for certain insns where
the EVEX enconding comes first in the table. This requires the respective
AVX-* feature to be explicitly enabled. */
new file mode 100644
@@ -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
new file mode 100644
@@ -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
@@ -511,6 +511,7 @@ if [gas_32_check] then {
run_dump_test "sm4-intel"
run_list_test "pbndkb-inval"
run_list_test "user_msr-inval"
+ run_list_test "apx-push2pop2-inval"
run_list_test "sg"
run_dump_test "clzero"
run_dump_test "invlpgb"
@@ -28,5 +28,7 @@ Disassembly of section .text:
[ ]*[a-f0-9]+:[ ]+67 62 f2 7c 18 f5[ ]+addr32 \(bad\)
[ ]*[a-f0-9]+:[ ]+0b ff[ ]+or %edi,%edi
[ ]*[a-f0-9]+:[ ]+62 f4 fc 08 ff[ ]+\(bad\)
-[ ]*[a-f0-9]+:[ ]+d8[ ]+.byte 0xd8
-#pass
+[ ]*[a-f0-9]+:[ ]+d8 ff[ ]+fdivr %st\(7\),%st
+[ ]*[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\)
@@ -28,3 +28,9 @@ _start:
.byte 0xff
#{evex} inc %rax %rbx EVEX.vvvv' != 1111 && EVEX.ND = 0.
.insn EVEX.L0.NP.M4.W1 0xff, %rax, %rbx
+ .byte 0xff
+ # pop2 %rax, %rbx set EVEX.ND=0.
+ .byte 0x62,0xf4,0x64,0x08,0x8f,0xc0
+ .byte 0xff, 0xff, 0xff
+ # pop2 %rax set EVEX.vvvv' = 1111.
+ .byte 0x62,0xf4,0x7c,0x18,0x8f,0xc0
new file mode 100644
@@ -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
new file mode 100644
@@ -0,0 +1,13 @@
+.* Assembler messages:
+.*:6: Error: operand size mismatch for `push2'
+.*:7: Error: operand size mismatch for `push2'
+.*:8: Error: cannot be used with %rsp register for `push2'
+.*:9: Error: cannot be used with %rsp register for `push2'
+.*:10: Error: operand size mismatch for `push2p'
+.*:11: Error: cannot be used with %rsp register for `push2p'
+.*:12: Error: operand size mismatch for `pop2'
+.*:13: Error: cannot be used with %rsp register for `pop2'
+.*:14: Error: cannot be used with %rsp register for `pop2'
+.*:15: Error: two source registers must be distinct for `pop2'
+.*:16: Error: cannot be used with %rsp register for `pop2p'
+.*:17: Error: two source registers must be distinct for `pop2p'
new file mode 100644
@@ -0,0 +1,17 @@
+# Check illegal APX-Push2Pop2 instructions
+
+ .allow_index_reg
+ .text
+_start:
+ push2 %ax, %bx
+ push2 %eax, %ebx
+ push2 %rsp, %r17
+ push2 %r17, %rsp
+ push2p %eax, %ebx
+ push2p %rsp, %r17
+ pop2 %ax, %bx
+ pop2 %rax, %rsp
+ pop2 %rsp, %rax
+ pop2 %r12, %r12
+ pop2p %rax, %rsp
+ pop2p %r12, %r12
new file mode 100644
@@ -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
new file mode 100644
@@ -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
@@ -345,6 +345,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"
@@ -86,6 +86,10 @@
{ "subQ", { VexGv, Ev, sIb }, PREFIX_NP_OR_DATA },
{ "xorQ", { VexGv, Ev, sIb }, PREFIX_NP_OR_DATA },
},
+ /* REG_EVEX_MAP4_8F */
+ {
+ { VEX_W_TABLE (EVEX_W_MAP4_8F_R_0) },
+ },
/* REG_EVEX_MAP4_F6 */
{
{ Bad_Opcode },
@@ -109,5 +113,10 @@
{
{ "incQ", { VexGv, Ev }, PREFIX_NP_OR_DATA },
{ "decQ", { VexGv, Ev }, PREFIX_NP_OR_DATA },
+ { Bad_Opcode },
+ { Bad_Opcode },
+ { Bad_Opcode },
+ { Bad_Opcode },
+ { VEX_W_TABLE (EVEX_W_MAP4_FF_R_6) },
},
@@ -442,6 +442,16 @@
{ Bad_Opcode },
{ "vpshrdw", { XM, Vex, EXx, Ib }, 0 },
},
+ /* EVEX_W_MAP4_8F_R_0 */
+ {
+ { "pop2", { { PUSH2_POP2_Fixup, q_mode}, Eq }, NO_PREFIX },
+ { "pop2p", { { PUSH2_POP2_Fixup, q_mode}, Eq }, NO_PREFIX },
+ },
+ /* EVEX_W_MAP4_FF_R_6 */
+ {
+ { "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 },
@@ -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 },
@@ -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,
@@ -225,6 +226,9 @@ struct instr_info
}
vex;
+/* For APX EVEX-promoted prefix, EVEX.ND shares the same bit as vex.b. */
+#define nd b
+
enum evex_type evex_type;
/* Remember if the current op is a jump instruction. */
@@ -899,6 +903,7 @@ enum
REG_EVEX_MAP4_80,
REG_EVEX_MAP4_81,
REG_EVEX_MAP4_83,
+ REG_EVEX_MAP4_8F,
REG_EVEX_MAP4_F6,
REG_EVEX_MAP4_F7,
REG_EVEX_MAP4_FE,
@@ -1742,6 +1747,9 @@ enum
EVEX_W_0F3A70,
EVEX_W_0F3A72,
+ EVEX_W_MAP4_8F_R_0,
+ EVEX_W_MAP4_FF_R_6,
+
EVEX_W_MAP5_5B_P_0,
EVEX_W_MAP5_7A_P_3,
};
@@ -9125,7 +9133,7 @@ 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->evex_type == evex_from_legacy && !ins->vex.b
+ if (ins->evex_type == evex_from_legacy && !ins->vex.nd
&& (ins->vex.register_specifier || !ins->vex.v))
return &bad_opcode;
@@ -13388,11 +13396,10 @@ OP_VEX (instr_info *ins, int bytemode, int sizeflag ATTRIBUTE_UNUSED)
if (!ins->need_vex)
return true;
- /* Here vex.b is treated as "EVEX.ND". */
if (ins->evex_type == evex_from_legacy)
{
ins->evex_used |= EVEX_b_used;
- if (!ins->vex.b)
+ if (!ins->vex.nd)
return true;
}
@@ -13500,6 +13507,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)
@@ -13884,3 +13894,26 @@ 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)
+{
+ if (ins->modrm.mod != 3 || !ins->vex.b)
+ return true;
+
+ 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);
+
+ /* Push2/Pop2 cannot use RSP and Pop2 cannot pop two same registers. */
+ if (!ins->vex.nd || vvvv_reg == 0x4 || rm_reg == 0x4
+ || (!ins->modrm.reg
+ && vvvv_reg == rm_reg))
+ {
+ oappend (ins, "(bad)");
+ return true;
+ }
+
+ return OP_VEX (ins, bytemode, sizeflag);
+}
@@ -807,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. */
@@ -3486,3 +3486,12 @@ uwrmsr, 0xf30f38f8, USER_MSR, Modrm|NoSuf|NoRex64, { Reg64, Reg64 }
uwrmsr, 0xf3f8/0, USER_MSR, Modrm|Vex128|VexMap7|VexW0|NoSuf, { Imm32, Reg64 }
// USER_MSR instructions end.
+
+// APX Push2/Pop2 instructions.
+
+push2, 0xff/6, APX_F, Modrm|VexW0|EVex128|EVexMap4|VexVVVV|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
+push2p, 0xff/6, APX_F, Modrm|VexW1|EVex128|EVexMap4|VexVVVV|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
+pop2, 0x8f/0, APX_F, Modrm|VexW0|EVex128|EVexMap4|VexVVVV|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
+pop2p, 0x8f/0, APX_F, Modrm|VexW1|EVex128|EVexMap4|VexVVVV|No_bSuf|No_wSuf|No_lSuf|No_sSuf, { Reg64, Reg64 }
+
+// APX Push2/Pop2 instructions end.