From: "H.J. Lu" <hjl.tools@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH] x86: fold special-operand insn attributes into a single enum
Date: Mon, 14 Nov 2022 07:58:58 -0800 [thread overview]
Message-ID: <CAMe9rOr_xL8rs2Pn32bqnC82aT9s7HcB8vTLQySJyhbsNfR41g@mail.gmail.com> (raw)
In-Reply-To: <21665493-a9f9-3429-c9ae-ea69bc7751e2@suse.com>
On Thu, Nov 10, 2022 at 5:45 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Attributes which aren't used together in any single insn template can be
> converted from individual booleans to a single enum, as was done for a few
> other attributes before. This is more space efficient. Collect together
> all attributes which express special operand constraints (and which fit
> the criteria for folding).
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -2071,7 +2071,7 @@ operand_size_match (const insn_template
> {
> if (i.types[j].bitfield.class != Reg
> && i.types[j].bitfield.class != RegSIMD
> - && t->opcode_modifier.anysize)
> + && t->opcode_modifier.operandconstraint == ANY_SIZE)
> continue;
>
> if (t->operand_types[j].bitfield.class == Reg
> @@ -4518,7 +4518,7 @@ load_insn_p (void)
> {
> /* Anysize insns: lea, invlpg, clflush, prefetch*, bndmk, bndcl, bndcu,
> bndcn, bndstx, bndldx, clflushopt, clwb, cldemote. */
> - if (i.tm.opcode_modifier.anysize)
> + if (i.tm.opcode_modifier.operandconstraint == ANY_SIZE)
> return 0;
>
> /* pop. */
> @@ -5107,7 +5107,7 @@ md_assemble (char *line)
> if (!process_operands ())
> return;
> }
> - else if (!quiet_warnings && i.tm.opcode_modifier.ugh)
> + else if (!quiet_warnings && i.tm.opcode_modifier.operandconstraint == UGH)
> {
> /* UnixWare fsub no args is alias for fsubp, fadd -> faddp, etc. */
> as_warn (_("translating to `%sp'"), i.tm.name);
> @@ -6020,7 +6020,7 @@ check_VecOperands (const insn_template *
> }
>
> /* Check if default mask is allowed. */
> - if (t->opcode_modifier.nodefmask
> + if (t->opcode_modifier.operandconstraint == NO_DEFAULT_MASK
> && (!i.mask.reg || i.mask.reg->reg_num == 0))
> {
> i.error = no_default_mask;
> @@ -6102,7 +6102,7 @@ check_VecOperands (const insn_template *
>
> /* For some special instructions require that destination must be distinct
> from source registers. */
> - if (t->opcode_modifier.distinctdest)
> + if (t->opcode_modifier.operandconstraint == DISTINCT_DEST)
> {
> unsigned int dest_reg = i.operands - 1;
>
> @@ -7047,7 +7047,7 @@ process_suffix (void)
> i.suffix = QWORD_MNEM_SUFFIX;
> else if (i.reg_operands
> && (i.operands > 1 || i.types[0].bitfield.class == Reg)
> - && !i.tm.opcode_modifier.addrprefixopreg)
> + && i.tm.opcode_modifier.operandconstraint != ADDR_PREFIX_OP_REG)
> {
> unsigned int numop = i.operands;
>
> @@ -7414,7 +7414,7 @@ process_suffix (void)
> break;
> }
>
> - if (i.tm.opcode_modifier.addrprefixopreg)
> + if (i.tm.opcode_modifier.operandconstraint == ADDR_PREFIX_OP_REG)
> {
> gas_assert (!i.suffix);
> gas_assert (i.reg_operands);
> @@ -7808,7 +7808,7 @@ process_operands (void)
> }
> }
> }
> - else if (i.tm.opcode_modifier.implicit1stxmm0)
> + else if (i.tm.opcode_modifier.operandconstraint == IMPLICIT_1ST_XMM0)
> {
> gas_assert ((MAX_OPERANDS - 1) > dupl
> && (i.tm.opcode_modifier.vexsources
> @@ -7876,7 +7876,7 @@ process_operands (void)
> i.reg_operands--;
> i.tm.operands--;
> }
> - else if (i.tm.opcode_modifier.implicitquadgroup)
> + else if (i.tm.opcode_modifier.operandconstraint == IMPLICIT_QUAD_GROUP)
> {
> unsigned int regnum, first_reg_in_group, last_reg_in_group;
>
> @@ -7893,7 +7893,7 @@ process_operands (void)
> register_prefix, i.op[1].regs->reg_name, last_reg_in_group,
> i.tm.name);
> }
> - else if (i.tm.opcode_modifier.regkludge)
> + else if (i.tm.opcode_modifier.operandconstraint == REG_KLUDGE)
> {
> /* The imul $imm, %reg instruction is converted into
> imul $imm, %reg, %reg, and the clr %reg instruction
> @@ -7963,7 +7963,7 @@ process_operands (void)
> i.tm.base_opcode |= i.op[op].regs->reg_num;
> if ((i.op[op].regs->reg_flags & RegRex) != 0)
> i.rex |= REX_B;
> - if (!quiet_warnings && i.tm.opcode_modifier.ugh)
> + if (!quiet_warnings && i.tm.opcode_modifier.operandconstraint == UGH)
> {
> /* Warn about some common errors, but press on regardless.
> The first case can be generated by gcc (<= 2.8.1). */
> @@ -8190,7 +8190,7 @@ build_modrm_byte (void)
> unsigned int vvvv;
>
> /* Swap two source operands if needed. */
> - if (i.tm.opcode_modifier.swapsources)
> + if (i.tm.opcode_modifier.operandconstraint == SWAP_SOURCES)
> {
> vvvv = source;
> source = dest;
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -729,9 +729,8 @@ static bitfield opcode_modifiers[] =
> BITFIELD (FloatR),
> BITFIELD (Size),
> BITFIELD (CheckRegSize),
> - BITFIELD (DistinctDest),
> + BITFIELD (OperandConstraint),
> BITFIELD (MnemonicSize),
> - BITFIELD (Anysize),
> BITFIELD (No_bSuf),
> BITFIELD (No_wSuf),
> BITFIELD (No_lSuf),
> @@ -742,14 +741,10 @@ static bitfield opcode_modifiers[] =
> BITFIELD (IsString),
> BITFIELD (RegMem),
> BITFIELD (BNDPrefixOk),
> - BITFIELD (RegKludge),
> - BITFIELD (Implicit1stXmm0),
> BITFIELD (PrefixOk),
> - BITFIELD (AddrPrefixOpReg),
> BITFIELD (IsPrefix),
> BITFIELD (ImmExt),
> BITFIELD (NoRex64),
> - BITFIELD (Ugh),
> BITFIELD (Vex),
> BITFIELD (VexVVVV),
> BITFIELD (VexW),
> @@ -764,9 +759,6 @@ static bitfield opcode_modifiers[] =
> BITFIELD (StaticRounding),
> BITFIELD (SAE),
> BITFIELD (Disp8MemShift),
> - BITFIELD (NoDefMask),
> - BITFIELD (ImplicitQuadGroup),
> - BITFIELD (SwapSources),
> BITFIELD (Optimize),
> BITFIELD (ATTMnemonic),
> BITFIELD (ATTSyntax),
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -495,17 +495,35 @@ enum
> Size,
> /* check register size. */
> CheckRegSize,
> + /* any memory size */
> +#define ANY_SIZE 1
> + /* fake an extra reg operand for clr, imul and special register
> + processing for some instructions. */
> +#define REG_KLUDGE 2
> + /* deprecated fp insn, gets a warning */
> +#define UGH 3
> + /* An implicit xmm0 as the first operand */
> +#define IMPLICIT_1ST_XMM0 4
> + /* The second operand must be a vector register, {x,y,z}mmN, where N is a multiple of 4.
> + It implicitly denotes the register group of {x,y,z}mmN - {x,y,z}mm(N + 3).
> + */
> +#define IMPLICIT_QUAD_GROUP 5
> + /* Two source operands are swapped. */
> +#define SWAP_SOURCES 6
> + /* Default mask isn't allowed. */
> +#define NO_DEFAULT_MASK 7
> + /* Address prefix changes register operand */
> +#define ADDR_PREFIX_OP_REG 8
> /* Instrucion requires that destination must be distinct from source
> registers. */
> - DistinctDest,
> +#define DISTINCT_DEST 9
> + OperandConstraint,
> /* instruction ignores operand size prefix and in Intel mode ignores
> mnemonic size suffix check. */
> #define IGNORESIZE 1
> /* default insn size depends on mode */
> #define DEFAULTSIZE 2
> MnemonicSize,
> - /* any memory size */
> - Anysize,
> /* b suffix on instruction illegal */
> No_bSuf,
> /* w suffix on instruction illegal */
> @@ -533,11 +551,6 @@ enum
> RegMem,
> /* quick test if branch instruction is MPX supported */
> BNDPrefixOk,
> - /* fake an extra reg operand for clr, imul and special register
> - processing for some instructions. */
> - RegKludge,
> - /* An implicit xmm0 as the first operand */
> - Implicit1stXmm0,
> #define PrefixNone 0
> #define PrefixRep 1
> #define PrefixHLERelease 2 /* Okay with an XRELEASE (0xf3) prefix. */
> @@ -548,16 +561,12 @@ enum
> #define PrefixHLELock 5 /* Okay with a LOCK prefix. */
> #define PrefixHLEAny 6 /* Okay with or without a LOCK prefix. */
> PrefixOk,
> - /* Address prefix changes register operand */
> - AddrPrefixOpReg,
> /* opcode is a prefix */
> IsPrefix,
> /* instruction has extension in 8 bit imm */
> ImmExt,
> /* instruction don't need Rex64 prefix. */
> NoRex64,
> - /* deprecated fp insn, gets a warning */
> - Ugh,
> /* insn has VEX prefix:
> 1: 128bit VEX prefix (or operand dependent).
> 2: 256bit VEX prefix.
> @@ -700,17 +709,6 @@ enum
> #define DISP8_SHIFT_VL 7
> Disp8MemShift,
>
> - /* Default mask isn't allowed. */
> - NoDefMask,
> -
> - /* The second operand must be a vector register, {x,y,z}mmN, where N is a multiple of 4.
> - It implicitly denotes the register group of {x,y,z}mmN - {x,y,z}mm(N + 3).
> - */
> - ImplicitQuadGroup,
> -
> - /* Two source operands are swapped. */
> - SwapSources,
> -
> /* Support encoding optimization. */
> Optimize,
>
> @@ -745,9 +743,8 @@ typedef struct i386_opcode_modifier
> unsigned int floatr:1;
> unsigned int size:2;
> unsigned int checkregsize:1;
> - unsigned int distinctdest:1;
> + unsigned int operandconstraint:4;
> unsigned int mnemonicsize:2;
> - unsigned int anysize:1;
> unsigned int no_bsuf:1;
> unsigned int no_wsuf:1;
> unsigned int no_lsuf:1;
> @@ -758,14 +755,10 @@ typedef struct i386_opcode_modifier
> unsigned int isstring:2;
> unsigned int regmem:1;
> unsigned int bndprefixok:1;
> - unsigned int regkludge:1;
> - unsigned int implicit1stxmm0:1;
> unsigned int prefixok:3;
> - unsigned int addrprefixopreg:1;
> unsigned int isprefix:1;
> unsigned int immext:1;
> unsigned int norex64:1;
> - unsigned int ugh:1;
> unsigned int vex:2;
> unsigned int vexvvvv:2;
> unsigned int vexw:2;
> @@ -780,9 +773,6 @@ typedef struct i386_opcode_modifier
> unsigned int staticrounding:1;
> unsigned int sae:1;
> unsigned int disp8memshift:3;
> - unsigned int nodefmask:1;
> - unsigned int implicitquadgroup:1;
> - unsigned int swapsources:1;
> unsigned int optimize:1;
> unsigned int attmnemonic:1;
> unsigned int attsyntax:1;
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -75,6 +75,17 @@
> #define Size32 Size=SIZE32
> #define Size64 Size=SIZE64
>
> +#define AddrPrefixOpReg OperandConstraint=ADDR_PREFIX_OP_REG | \
> + No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf
> +#define Anysize OperandConstraint=ANY_SIZE
> +#define DistinctDest OperandConstraint=DISTINCT_DEST
> +#define Implicit1stXmm0 OperandConstraint=IMPLICIT_1ST_XMM0
> +#define ImplicitQuadGroup OperandConstraint=IMPLICIT_QUAD_GROUP
> +#define NoDefMask OperandConstraint=NO_DEFAULT_MASK
> +#define RegKludge OperandConstraint=REG_KLUDGE
> +#define SwapSources OperandConstraint=SWAP_SOURCES
> +#define Ugh OperandConstraint=UGH
> +
> #define IgnoreSize MnemonicSize=IGNORESIZE
> #define DefaultSize MnemonicSize=DEFAULTSIZE
>
> @@ -91,8 +102,6 @@
> #define HLEPrefixRelease PrefixOk=PrefixHLERelease
> #define NoTrackPrefixOk PrefixOk=PrefixNoTrack
>
> -#define AddrPrefixOpReg AddrPrefixOpReg|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf
> -
> #define Space0F OpcodeSpace=SPACE_0F
> #define Space0F38 OpcodeSpace=SPACE_0F38
> #define Space0F3A OpcodeSpace=SPACE_0F3A
OK.
Thanks.
--
H.J.
prev parent reply other threads:[~2022-11-14 15:59 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-10 13:45 Jan Beulich
2022-11-10 17:38 ` H.J. Lu
2022-11-11 8:00 ` Jan Beulich
2022-11-11 8:22 ` Jan Beulich
2022-11-11 19:48 ` H.J. Lu
2022-11-14 1:38 ` Jiang, Haochen
2022-11-14 15:58 ` H.J. Lu [this message]
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=CAMe9rOr_xL8rs2Pn32bqnC82aT9s7HcB8vTLQySJyhbsNfR41g@mail.gmail.com \
--to=hjl.tools@gmail.com \
--cc=binutils@sourceware.org \
--cc=jbeulich@suse.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).