Re: [PATCH] x86: fold special-operand insn attributes into a single enum

["H.J. Lu" <hjl.tools@gmail.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).

These assumptions may not be all true for future new instructions.

> --- 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



-- 
H.J.