[PATCH 2/8] Support APX GPR32 with extend evex prefix
Jan Beulich
jbeulich@suse.com
Fri Sep 22 10:50:35 GMT 2023
On 19.09.2023 17:25, Cui, Lili wrote:
> --- a/opcodes/i386-dis-evex-len.h
> +++ b/opcodes/i386-dis-evex-len.h
> @@ -62,6 +62,16 @@ static const struct dis386 evex_len_table[][3] = {
> { REG_TABLE (REG_EVEX_0F38C7_L_2) },
> },
>
> + /* EVEX_LEN_0F38F2 */
> + {
> + { "andnS", { Gdq, VexGdq, Edq }, 0 },
> + },
> +
> + /* EVEX_LEN_0F38F3 */
> + {
> + { REG_TABLE(REG_EVEX_0F38F3_L_0) },
> + },
> +
> /* EVEX_LEN_0F3A00 */
> {
> { Bad_Opcode },
> diff --git a/opcodes/i386-dis-evex-mod.h b/opcodes/i386-dis-evex-mod.h
> index f9f912c5094..5a1326a1b73 100644
> --- a/opcodes/i386-dis-evex-mod.h
> +++ b/opcodes/i386-dis-evex-mod.h
> @@ -1 +1,51 @@
> /* Nothing at present. */
> + /* MOD_EVEX_MAP4_65 */
> + {
> + { "wrussK", { M, Gdq }, PREFIX_DATA },
> + },
> + /* MOD_EVEX_MAP4_66_PREFIX_0 */
> + {
> + { "wrssK", { M, Gdq }, 0 },
> + },
Not very long ago I invested quite a bit of time to remove unnecessary
decoding through mod_table[]. Please don't introduce new instances.
Entries should be added here only when both branches are populated
(iow it looks as if this patch shouldn't touch this file at all).
> --- a/opcodes/i386-dis-evex-prefix.h
> +++ b/opcodes/i386-dis-evex-prefix.h
> @@ -338,6 +338,89 @@
> { "vcmpp%XH", { MaskG, Vex, EXxh, EXxEVexS, CMP }, 0 },
> { "vcmps%XH", { MaskG, VexScalar, EXw, EXxEVexS, CMP }, 0 },
> },
> + /* PREFIX_EVEX_MAP4_60 */
> + {
> + { "movbeS", { Gv, Ev }, 0 },
> + { Bad_Opcode },
> + { "movbeS", { Gv, Ev }, 0 },
> + },
> + /* PREFIX_EVEX_MAP4_61 */
> + {
> + { "movbeS", { Ev, Gv }, 0 },
> + { Bad_Opcode },
> + { "movbeS", { Ev, Gv }, 0 },
> + },
In cases like this (of which, aiui, there will be many more), where only
prefix 66 is valid, and only to modify operand size, it would be quite
desirable to have a new PREFIX_... identifier to use in the parent table
entry, such that this additional decode step can be avoided.
> --- /dev/null
> +++ b/opcodes/i386-dis-evex-x86.h
I'm puzzled by the name suffix: x86 is kind of redundant with i386. Main
question perhaps is: Do we really need a new file here? It's not a lot
that is put here right now, but of course I haven't peeked ahead.
> --- a/opcodes/i386-dis-evex.h
> +++ b/opcodes/i386-dis-evex.h
> @@ -164,10 +164,10 @@ static const struct dis386 evex_table[][256] = {
> { Bad_Opcode },
> { Bad_Opcode },
> /* 90 */
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F90) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F91) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F92) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F93) },
> { Bad_Opcode },
> { Bad_Opcode },
> { Bad_Opcode },
> @@ -375,9 +375,9 @@ static const struct dis386 evex_table[][256] = {
> { "vpsllv%DQ", { XM, Vex, EXx }, PREFIX_DATA },
> /* 48 */
> { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F3849) },
> { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F384B) },
> { "vrcp14p%XW", { XM, EXx }, PREFIX_DATA },
> { "vrcp14s%XW", { XMScalar, VexScalar, EXdq }, PREFIX_DATA },
> { "vrsqrt14p%XW", { XM, EXx }, 0 },
> @@ -545,32 +545,32 @@ static const struct dis386 evex_table[][256] = {
> { "%XEvaesdecY", { XM, Vex, EXx }, PREFIX_DATA },
> { "%XEvaesdeclastY", { XM, Vex, EXx }, PREFIX_DATA },
> /* E0 */
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E0) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E1) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E2) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E3) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E4) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E5) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E6) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E7) },
> /* E8 */
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E8) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38E9) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38EA) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38EB) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38EC) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38ED) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38EE) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38EF) },
> /* F0 */
> { Bad_Opcode },
> { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38F2) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38F3) },
> { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> - { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38F5) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38F6) },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F38F7) },
> /* F8 */
> { Bad_Opcode },
> { Bad_Opcode },
> @@ -854,7 +854,7 @@ static const struct dis386 evex_table[][256] = {
> { Bad_Opcode },
> { Bad_Opcode },
> /* F0 */
> - { Bad_Opcode },
> + { X86_64_EVEX_FROM_VEX_TABLE (X86_64_EVEX_0F3AF0) },
> { Bad_Opcode },
> { Bad_Opcode },
> { Bad_Opcode },
> @@ -872,7 +872,7 @@ static const struct dis386 evex_table[][256] = {
> { Bad_Opcode },
> { Bad_Opcode },
> },
> - /* EVEX_MAP5_ */
> + /* EVEX_MAP4_ */
While just an artifact from this, ...
> @@ -893,8 +893,8 @@ static const struct dis386 evex_table[][256] = {
> { Bad_Opcode },
> { Bad_Opcode },
> /* 10 */
> - { PREFIX_TABLE (PREFIX_EVEX_MAP5_10) },
> - { PREFIX_TABLE (PREFIX_EVEX_MAP5_11) },
> + { Bad_Opcode },
> + { Bad_Opcode },
> { Bad_Opcode },
> { Bad_Opcode },
> { Bad_Opcode },
> @@ -907,7 +907,7 @@ static const struct dis386 evex_table[][256] = {
> { Bad_Opcode },
> { Bad_Opcode },
> { Bad_Opcode },
> - { PREFIX_TABLE (PREFIX_EVEX_MAP5_1D) },
> + { Bad_Opcode },
> { Bad_Opcode },
> { Bad_Opcode },
> /* 20 */
... changes like these are extremely odd to read. Can you please try
to split this patch such that initially you simply introduce an empty
new sub-table, to avoid such anomalies (which will also affect "git
blame" then, I expect)?
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -132,6 +132,13 @@ enum x86_64_isa
> intel64
> };
>
> +enum evex_type
> +{
> + evex_default = 0,
> + evex_from_legacy,
> + evex_from_vex,
> +};
> +
> struct instr_info
> {
> enum address_mode address_mode;
> @@ -212,7 +219,6 @@ struct instr_info
> int ll;
> bool w;
> bool evex;
> - bool r;
The change to eliminate this field would certainly be nice to be separate
from the bulk of thw APX changes here.
Jan
More information about the Binutils
mailing list