From: "Cui, Lili" <lili.cui@intel.com>
To: "Beulich, Jan" <JBeulich@suse.com>, Binutils <binutils@sourceware.org>
Cc: "H.J. Lu" <hjl.tools@gmail.com>
Subject: RE: [PATCH 1/4] x86: rename vec_encoding and vex_encoding_*
Date: Sun, 18 Feb 2024 05:59:56 +0000 [thread overview]
Message-ID: <SJ0PR11MB56004CAC05FA61460BF562379E522@SJ0PR11MB5600.namprd11.prod.outlook.com> (raw)
In-Reply-To: <c5ea7548-e171-43ba-94c5-ffdb75be04cc@suse.com>
> Even with just VEX these weren't limited to vector insns. With APX the set of
> non-vector ones covered has greatly increased. Drop the vec_ prefix. Also drop
> the vex_ ones off of the enumerators, as they weren't appropriate anyway:
> Should have been vec_ then, too.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -445,16 +445,16 @@ struct _i386_insn
> /* Disable instruction size optimization. */
> bool no_optimize;
>
> - /* How to encode vector instructions. */
> + /* How to encode instructions. */
> enum
> {
> - vex_encoding_default = 0,
> - vex_encoding_vex,
> - vex_encoding_vex3,
> - vex_encoding_evex,
> - vex_encoding_evex512,
> - vex_encoding_error
> - } vec_encoding;
> + encoding_default = 0,
> + encoding_vex,
> + encoding_vex3,
> + encoding_evex,
> + encoding_evex512,
"encoding_evex512" seems redundant, duplicating with encoding_evex, do you know the reason for keeping it? Could you add some comments for " encoding_evex512", thanks.
Lili.
> + encoding_error
> + } encoding;
>
> /* REP prefix. */
> const char *rep_prefix;
> @@ -1885,8 +1885,8 @@ static const i386_cpu_flags avx512 = CPU
>
> static INLINE bool need_evex_encoding (const insn_template *t) {
> - return i.vec_encoding == vex_encoding_evex
> - || i.vec_encoding == vex_encoding_evex512
> + return i.encoding == encoding_evex
> + || i.encoding == encoding_evex512
> || (t->opcode_modifier.vex && i.has_egpr)
> || i.mask.reg;
> }
> @@ -2489,7 +2489,7 @@ static INLINE int
> fits_in_imm4 (offsetT num)
> {
> /* Despite the name, check for imm3 if we're dealing with EVEX. */
> - return (num & (i.vec_encoding != vex_encoding_evex ? 0xf : 7)) == num;
> + return (num & (i.encoding != encoding_evex ? 0xf : 7)) == num;
> }
>
> static i386_operand_type
> @@ -3800,7 +3800,7 @@ build_vex_prefix (const insn_template *t
> /* Use 2-byte VEX prefix by swapping destination and source operand
> if there are more than 1 register operand. */
> if (i.reg_operands > 1
> - && i.vec_encoding != vex_encoding_vex3
> + && i.encoding != encoding_vex3
> && i.dir_encoding == dir_encoding_default
> && i.operands == i.reg_operands
> && operand_type_equal (&i.types[0], &i.types[i.operands - 1]) @@ -
> 3829,7 +3829,7 @@ build_vex_prefix (const insn_template *t
> /* Use 2-byte VEX prefix by swapping commutative source operands if there
> are no memory operands and at least 3 register ones. */
> if (i.reg_operands >= 3
> - && i.vec_encoding != vex_encoding_vex3
> + && i.encoding != encoding_vex3
> && i.reg_operands == i.operands - i.imm_operands
> && i.tm.opcode_modifier.vex
> && i.tm.opcode_modifier.commutative @@ -3892,7 +3892,7 @@
> build_vex_prefix (const insn_template *t
>
> /* Use 2-byte VEX prefix if possible. */
> if (w == 0
> - && i.vec_encoding != vex_encoding_vex3
> + && i.encoding != encoding_vex3
> && i.tm.opcode_space == SPACE_0F
> && (i.rex & (REX_W | REX_X | REX_B)) == 0)
> {
> @@ -4758,7 +4758,7 @@ optimize_encoding (void)
> && (i.tm.opcode_modifier.vex
> || ((!i.mask.reg || i.mask.zeroing)
> && i.tm.opcode_modifier.evex
> - && (i.vec_encoding != vex_encoding_evex
> + && (i.encoding != encoding_evex
> || cpu_arch_isa_flags.bitfield.cpuavx512vl
> || is_cpu (&i.tm, CpuAVX512VL)
> || (i.tm.operand_types[2].bitfield.zmmword
> @@ -4808,12 +4808,12 @@ optimize_encoding (void)
> */
> if (i.tm.opcode_modifier.evex)
> {
> - if (i.vec_encoding != vex_encoding_evex)
> + if (i.encoding != encoding_evex)
> {
> i.tm.opcode_modifier.vex = VEX128;
> i.tm.opcode_modifier.vexw = VEXW0;
> i.tm.opcode_modifier.evex = 0;
> - i.vec_encoding = vex_encoding_vex;
> + i.encoding = encoding_vex;
> i.mask.reg = NULL;
> }
> else if (optimize > 1)
> @@ -4836,7 +4836,7 @@ optimize_encoding (void)
> i.types[j].bitfield.ymmword = 0;
> }
> }
> - else if (i.vec_encoding != vex_encoding_evex
> + else if (i.encoding != encoding_evex
> && !i.types[0].bitfield.zmmword
> && !i.types[1].bitfield.zmmword
> && !i.mask.reg
> @@ -4978,7 +4978,7 @@ optimize_encoding (void)
> && i.tm.opcode_modifier.vex
> && !(i.op[0].regs->reg_flags & RegRex)
> && i.op[0].regs->reg_type.bitfield.xmmword
> - && i.vec_encoding != vex_encoding_vex3)
> + && i.encoding != encoding_vex3)
> {
> /* Optimize: -Os:
> vpbroadcastq %xmmN, %xmmM ->
> vpunpcklqdq %xmmN, %xmmN, %xmmM (N < 8) @@ -6839,10 +6839,10
> @@ md_assemble (char *line)
> if (optimize && !i.no_optimize && i.tm.opcode_modifier.optimize)
> optimize_encoding ();
>
> - /* Past optimization there's no need to distinguish vex_encoding_evex and
> - vex_encoding_evex512 anymore. */
> - if (i.vec_encoding == vex_encoding_evex512)
> - i.vec_encoding = vex_encoding_evex;
> + /* Past optimization there's no need to distinguish encoding_evex and
> + encoding_evex512 anymore. */
> + if (i.encoding == encoding_evex512)
> + i.encoding = encoding_evex;
>
> if (use_unaligned_vector_move)
> encode_with_unaligned_vector_move (); @@ -7136,15 +7136,15 @@
> parse_insn (const char *line, char *mnem
> break;
> case Prefix_VEX:
> /* {vex} */
> - i.vec_encoding = vex_encoding_vex;
> + i.encoding = encoding_vex;
> break;
> case Prefix_VEX3:
> /* {vex3} */
> - i.vec_encoding = vex_encoding_vex3;
> + i.encoding = encoding_vex3;
> break;
> case Prefix_EVEX:
> /* {evex} */
> - i.vec_encoding = vex_encoding_evex;
> + i.encoding = encoding_evex;
> break;
> case Prefix_REX:
> /* {rex} */
> @@ -8260,7 +8260,7 @@ check_VecOperands (const insn_template * static
> int VEX_check_encoding (const insn_template *t) {
> - if (i.vec_encoding == vex_encoding_error)
> + if (i.encoding == encoding_error)
> {
> i.error = unsupported;
> return 1;
> @@ -8277,8 +8277,8 @@ VEX_check_encoding (const insn_template
> return 1;
> }
>
> - if (i.vec_encoding == vex_encoding_evex
> - || i.vec_encoding == vex_encoding_evex512)
> + if (i.encoding == encoding_evex
> + || i.encoding == encoding_evex512)
> {
> /* This instruction must be encoded with EVEX prefix. */
> if (!t->opcode_modifier.evex)
> @@ -8292,7 +8292,7 @@ VEX_check_encoding (const insn_template
> if (!t->opcode_modifier.vex)
> {
> /* This instruction template doesn't have VEX prefix. */
> - if (i.vec_encoding != vex_encoding_default)
> + if (i.encoding != encoding_default)
> {
> i.error = no_vex_encoding;
> return 1;
> @@ -8967,7 +8967,7 @@ match_template (char mnem_suffix)
> Note that the semantics have not been changed. */
> if (optimize
> && !i.no_optimize
> - && i.vec_encoding != vex_encoding_evex
> + && i.encoding != encoding_evex
> && ((t + 1 < current_templates.end
> && !t[1].opcode_modifier.evex
> && t[1].opcode_space <= SPACE_0F38 @@ -10157,7 +10157,7
> @@ process_operands (void)
> if (dot_insn () && i.reg_operands == 2)
> {
> gas_assert (is_any_vex_encoding (&i.tm)
> - || i.vec_encoding != vex_encoding_default);
> + || i.encoding != encoding_default);
> i.vex.register_specifier = i.op[i.operands - 1].regs;
> }
> }
> @@ -10167,7 +10167,7 @@ process_operands (void)
> == InstanceNone)
> {
> gas_assert (is_any_vex_encoding (&i.tm)
> - || i.vec_encoding != vex_encoding_default);
> + || i.encoding != encoding_default);
> i.vex.register_specifier = i.op[i.operands - 1].regs;
> }
>
> @@ -10275,7 +10275,7 @@ build_modrm_byte (void)
> }
> exp->X_add_number |= register_number (i.op[reg_slot].regs)
> << (3 + !(i.tm.opcode_modifier.evex
> - || i.vec_encoding == vex_encoding_evex));
> + || i.encoding == encoding_evex));
> }
>
> if (i.tm.opcode_modifier.vexvvvv == VexVVVV_DST) @@ -12548,20
> +12548,20 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> }
>
> if (vex || xop
> - ? i.vec_encoding == vex_encoding_evex
> + ? i.encoding == encoding_evex
> : evex
> - ? i.vec_encoding == vex_encoding_vex
> - || i.vec_encoding == vex_encoding_vex3
> - : i.vec_encoding != vex_encoding_default)
> + ? i.encoding == encoding_vex
> + || i.encoding == encoding_vex3
> + : i.encoding != encoding_default)
> {
> as_bad (_("pseudo-prefix conflicts with encoding specifier"));
> goto bad;
> }
>
> - if (line > end && i.vec_encoding == vex_encoding_default)
> - i.vec_encoding = evex ? vex_encoding_evex : vex_encoding_vex;
> + if (line > end && i.encoding == encoding_default)
> + i.encoding = evex ? encoding_evex : encoding_vex;
>
> - if (i.vec_encoding != vex_encoding_default)
> + if (i.encoding != encoding_default)
> {
> /* Only address size and segment override prefixes are permitted with
> VEX/XOP/EVEX encodings. */
> @@ -12892,20 +12892,20 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> goto done;
> }
>
> - /* No need to distinguish vex_encoding_evex and vex_encoding_evex512.
> */
> - if (i.vec_encoding == vex_encoding_evex512)
> - i.vec_encoding = vex_encoding_evex;
> + /* No need to distinguish encoding_evex and encoding_evex512. */
> + if (i.encoding == encoding_evex512)
> + i.encoding = encoding_evex;
>
> /* Are we to emit ModR/M encoding? */
> if (!i.short_form
> && (i.mem_operands
> - || i.reg_operands > (i.vec_encoding != vex_encoding_default)
> + || i.reg_operands > (i.encoding != encoding_default)
> || i.tm.extension_opcode != None))
> i.tm.opcode_modifier.modrm = 1;
>
> if (!i.tm.opcode_modifier.modrm
> && (i.reg_operands
> - > i.short_form + 0U + (i.vec_encoding != vex_encoding_default)
> + > i.short_form + 0U + (i.encoding != encoding_default)
> || i.mem_operands))
> {
> as_bad (_("too many register/memory operands")); @@ -12944,7
> +12944,7 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> }
> /* Fall through. */
> case 3:
> - if (i.vec_encoding != vex_encoding_default)
> + if (i.encoding != encoding_default)
> {
> i.tm.opcode_modifier.vexvvvv = 1;
> break;
> @@ -13000,13 +13000,13 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> || i.index_reg->reg_type.bitfield.ymmword
> || i.index_reg->reg_type.bitfield.zmmword))
> {
> - if (i.vec_encoding == vex_encoding_default)
> + if (i.encoding == encoding_default)
> {
> as_bad (_("VSIB unavailable with legacy encoding"));
> goto done;
> }
>
> - if (i.vec_encoding == vex_encoding_evex
> + if (i.encoding == encoding_evex
> && i.reg_operands > 1)
> {
> /* We could allow two register operands, encoding the 2nd one in
> @@ -13026,7 +13026,7 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> for (j = i.imm_operands; j < i.operands; ++j)
> {
> /* Look for 8-bit operands that use old registers. */
> - if (i.vec_encoding != vex_encoding_default
> + if (i.encoding != encoding_default
> && flag_code == CODE_64BIT
> && i.types[j].bitfield.class == Reg
> && i.types[j].bitfield.byte
> @@ -13085,7 +13085,7 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> case 4: combined.bitfield.dword = 1; break;
> }
>
> - if (i.vec_encoding == vex_encoding_default)
> + if (i.encoding == encoding_default)
> {
> if (flag_code == CODE_64BIT && combined.bitfield.qword)
> i.rex |= REX_W;
> @@ -13162,7 +13162,7 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> if (i.memshift >= 32)
> i.memshift = 0;
> else if (!evex)
> - i.vec_encoding = vex_encoding_error;
> + i.encoding = encoding_error;
>
> if (i.disp_operands && !optimize_disp (&i.tm))
> goto done;
> @@ -13228,8 +13228,8 @@ s_insn (int dummy ATTRIBUTE_UNUSED)
> potential special casing there. */
> i.tm.base_opcode |= val;
>
> - if (i.vec_encoding == vex_encoding_error
> - || (i.vec_encoding != vex_encoding_evex
> + if (i.encoding == encoding_error
> + || (i.encoding != encoding_evex
> ? i.broadcast.type || i.broadcast.bytes
> || i.rounding.type != rc_none
> || i.mask.reg
> @@ -13340,10 +13340,10 @@ RC_SAE_specifier (const char *pstr)
> return NULL;
> }
>
> - if (i.vec_encoding == vex_encoding_default)
> - i.vec_encoding = vex_encoding_evex512;
> - else if (i.vec_encoding != vex_encoding_evex
> - && i.vec_encoding != vex_encoding_evex512)
> + if (i.encoding == encoding_default)
> + i.encoding = encoding_evex512;
> + else if (i.encoding != encoding_evex
> + && i.encoding != encoding_evex512)
> return NULL;
>
> i.rounding.type = RC_NamesTable[j].type; @@ -13405,10 +13405,10
> @@ check_VecOperations (char *op_string)
> }
> op_string++;
>
> - if (i.vec_encoding == vex_encoding_default)
> - i.vec_encoding = vex_encoding_evex;
> - else if (i.vec_encoding != vex_encoding_evex
> - && i.vec_encoding != vex_encoding_evex512)
> + if (i.encoding == encoding_default)
> + i.encoding = encoding_evex;
> + else if (i.encoding != encoding_evex
> + && i.encoding != encoding_evex512)
> goto unknown_vec_op;
>
> i.broadcast.type = bcst_type;
> @@ -15676,11 +15676,11 @@ static bool check_register (const reg_en
> if (vector_size < VSZ512)
> return false;
>
> - if (i.vec_encoding == vex_encoding_default)
> - i.vec_encoding = vex_encoding_evex512;
> - else if (i.vec_encoding != vex_encoding_evex
> - && i.vec_encoding != vex_encoding_evex512)
> - i.vec_encoding = vex_encoding_error;
> + if (i.encoding == encoding_default)
> + i.encoding = encoding_evex512;
> + else if (i.encoding != encoding_evex
> + && i.encoding != encoding_evex512)
> + i.encoding = encoding_error;
> }
>
> if (vector_size < VSZ256 && r->reg_type.bitfield.ymmword) @@ -15706,11
> +15706,11 @@ static bool check_register (const reg_en
> || flag_code != CODE_64BIT)
> return false;
>
> - if (i.vec_encoding == vex_encoding_default
> - || i.vec_encoding == vex_encoding_evex512)
> - i.vec_encoding = vex_encoding_evex;
> - else if (i.vec_encoding != vex_encoding_evex)
> - i.vec_encoding = vex_encoding_error;
> + if (i.encoding == encoding_default
> + || i.encoding == encoding_evex512)
> + i.encoding = encoding_evex;
> + else if (i.encoding != encoding_evex)
> + i.encoding = encoding_error;
> }
>
> if (r->reg_flags & RegRex2)
> --- a/gas/config/tc-i386-intel.c
> +++ b/gas/config/tc-i386-intel.c
> @@ -209,10 +209,10 @@ operatorT i386_operator (const char *nam
> || i386_types[j].sz[0] > 8
> || (i386_types[j].sz[0] & (i386_types[j].sz[0] - 1)))
> return O_illegal;
> - if (i.vec_encoding == vex_encoding_default)
> - i.vec_encoding = vex_encoding_evex;
> - else if (i.vec_encoding != vex_encoding_evex
> - && i.vec_encoding != vex_encoding_evex512)
> + if (i.encoding == encoding_default)
> + i.encoding = encoding_evex;
> + else if (i.encoding != encoding_evex
> + && i.encoding != encoding_evex512)
> return O_illegal;
> if (!i.broadcast.bytes && !i.broadcast.type)
> {
next prev parent reply other threads:[~2024-02-18 6:00 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-16 9:56 [PATCH 0/4] x86/APX: misc adjustments Jan Beulich
2024-02-16 9:57 ` [PATCH 1/4] x86: rename vec_encoding and vex_encoding_* Jan Beulich
2024-02-18 5:59 ` Cui, Lili [this message]
2024-02-19 7:54 ` Jan Beulich
2024-02-20 9:19 ` Cui, Lili
2024-02-16 9:58 ` [PATCH 2/4] x86/APX: respect {vex}/{vex3} Jan Beulich
2024-02-18 7:55 ` Cui, Lili
2024-02-19 8:00 ` Jan Beulich
2024-02-20 10:12 ` Cui, Lili
2024-02-20 10:30 ` Jan Beulich
2024-02-20 15:59 ` Michael Matz
2024-02-20 16:52 ` H.J. Lu
2024-02-16 9:58 ` [PATCH 3/4] x86/APX: correct .insn opcode space determination when REX2 is needed Jan Beulich
2024-02-16 9:59 ` [PATCH 4/4] x86/APX: optimize certain XOR and SUB forms Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SJ0PR11MB56004CAC05FA61460BF562379E522@SJ0PR11MB5600.namprd11.prod.outlook.com \
--to=lili.cui@intel.com \
--cc=JBeulich@suse.com \
--cc=binutils@sourceware.org \
--cc=hjl.tools@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).