[PATCH 10/13] x86: fold EsSeg into IsString

H.J. Lu hjl.tools@gmail.com
Thu Oct 31 19:39:00 GMT 2019


On Wed, Oct 30, 2019 at 1:27 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> EsSeg (a per-operand bit) is used with IsString (a per-insn attribute)
> only. Extend the attribute to 2 bits, thus allowing to encode
> - not a string insn,
> - string insn with neither operand requiring use of %es:,
> - string insn with 1st operand requiring use of %es:,
> - string insn with 2nd operand requiring use of %es:,
> which covers all possible cases, allowing to drop EsSeg.
>
> The (transient) need to comment out the OTUnused #define did uncover an
> oversight in the earlier OTMax -> OTNum conversion, which is being taken
> care of here.
>
> Take the opportunity and remove IgnoreSize again from the operand-less
> forms of CMPSD and MOVSD - commit d241b91073 added them by mistake.

Please commit this part as a separate patch.

> gas/
> 2019-10-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (type_names): Remove OPERAND_TYPE_ESSEG
>         entry.
>         (md_assemble): Adjust isstring field use. Add assertion.
>         (check_string): Mostly re-write.
>         (i386_index_check): Adjust isstring field use and related code.
>
> opcodes/
> 2019-10-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * i386-gen.c (operand_type_init): Remove OPERAND_TYPE_ESSEG
>         entry.
>         (operand_types): Remove EsSeg entry.
>         (main): Replace stale use of OTMax.
>         * i386-opc.h (IS_STRING_ES_OP0, IS_STRING_ES_OP1): Define.
>         (struct i386_opcode_modifier): Expand isstring field to 2 bits.
>         (EsSeg): Delete.
>         (OTUnused): Comment out.
>         (union i386_operand_type): Remove esseg field.
>         * i386-opc.tbl (EsSegOp0, EsSegOp1): Define.
>         (cmps, scmp, scas, ssca, cmpsd): Add EsSegOp0.
>         (ins, movs, smov, movsd): Add EsSegOp1.
>         (stos, ssto): Add EsSegOp0/EsSegOp1.
>         * i386-init.h, i386-tbl.h: Re-generate.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -3160,7 +3160,6 @@ const type_names[] =
>    { OPERAND_TYPE_REGYMM, "rYMM" },
>    { OPERAND_TYPE_REGZMM, "rZMM" },
>    { OPERAND_TYPE_REGMASK, "Mask reg" },
> -  { OPERAND_TYPE_ESSEG, "es" },
>  };
>
>  static void
> @@ -4368,8 +4367,9 @@ md_assemble (char *line)
>      }
>
>    /* Check string instruction segment overrides.  */
> -  if (i.tm.opcode_modifier.isstring && i.mem_operands != 0)
> +  if (i.tm.opcode_modifier.isstring >= IS_STRING_ES_OP0)
>      {
> +      gas_assert (i.mem_operands);
>        if (!check_string ())
>         return;
>        i.disp_operands = 0;
> @@ -6158,35 +6158,24 @@ check_reverse:
>  static int
>  check_string (void)
>  {
> -  unsigned int mem_op = i.flags[0] & Operand_Mem ? 0 : 1;
> +  unsigned int es_op = i.tm.opcode_modifier.isstring - IS_STRING_ES_OP0;
> +  unsigned int op = i.tm.operand_types[0].bitfield.baseindex ? es_op : 0;
>
> -  if (i.tm.operand_types[mem_op].bitfield.esseg)
> +  if (i.seg[op] != NULL && i.seg[op] != &es)
>      {
> -      if (i.seg[0] != NULL && i.seg[0] != &es)
> -       {
> -         as_bad (_("`%s' operand %d must use `%ses' segment"),
> -                 i.tm.name,
> -                 intel_syntax ? i.tm.operands - mem_op : mem_op + 1,
> -                 register_prefix);
> -         return 0;
> -       }
> -      /* There's only ever one segment override allowed per instruction.
> -        This instruction possibly has a legal segment override on the
> -        second operand, so copy the segment to where non-string
> -        instructions store it, allowing common code.  */
> -      i.seg[0] = i.seg[1];
> -    }
> -  else if (i.tm.operand_types[mem_op + 1].bitfield.esseg)
> -    {
> -      if (i.seg[1] != NULL && i.seg[1] != &es)
> -       {
> -         as_bad (_("`%s' operand %d must use `%ses' segment"),
> -                 i.tm.name,
> -                 intel_syntax ? i.tm.operands - mem_op - 1 : mem_op + 2,
> -                 register_prefix);
> -         return 0;
> -       }
> +      as_bad (_("`%s' operand %u must use `%ses' segment"),
> +             i.tm.name,
> +             intel_syntax ? i.tm.operands - es_op : es_op + 1,
> +             register_prefix);
> +      return 0;
>      }
> +
> +  /* There's only ever one segment override allowed per instruction.
> +     This instruction possibly has a legal segment override on the
> +     second operand, so copy the segment to where non-string
> +     instructions store it, allowing common code.  */
> +  i.seg[op] = i.seg[1];
> +
>    return 1;
>  }
>
> @@ -9874,16 +9863,16 @@ i386_index_check (const char *operand_st
>
>        if (current_templates->start->opcode_modifier.repprefixok)
>         {
> -         i386_operand_type type = current_templates->end[-1].operand_types[0];
> +         int es_op = current_templates->end[-1].opcode_modifier.isstring
> +                     - IS_STRING_ES_OP0;
> +         int op = 0;
>
> -         if (!type.bitfield.baseindex
> +         if (!current_templates->end[-1].operand_types[0].bitfield.baseindex
>               || ((!i.mem_operands != !intel_syntax)
>                   && current_templates->end[-1].operand_types[1]
>                      .bitfield.baseindex))
> -           type = current_templates->end[-1].operand_types[1];
> -         expected_reg = hash_find (reg_hash,
> -                                   di_si[addr_mode][type.bitfield.esseg]);
> -
> +           op = 1;
> +         expected_reg = hash_find (reg_hash, di_si[addr_mode][op == es_op]);
>         }
>        else
>         expected_reg = hash_find (reg_hash, bx[addr_mode]);
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -447,8 +447,6 @@ static initializer operand_type_init[] =
>      "Class=RegMask" },
>    { "OPERAND_TYPE_REGBND",
>      "Class=RegBND" },
> -  { "OPERAND_TYPE_ESSEG",
> -    "EsSeg" },
>    { "OPERAND_TYPE_ACC8",
>      "Instance=Accum|Byte" },
>    { "OPERAND_TYPE_ACC16",
> @@ -718,7 +716,6 @@ static bitfield operand_types[] =
>    BITFIELD (Disp32S),
>    BITFIELD (Disp64),
>    BITFIELD (JumpAbsolute),
> -  BITFIELD (EsSeg),
>    BITFIELD (Byte),
>    BITFIELD (Word),
>    BITFIELD (Dword),
> @@ -1740,7 +1737,7 @@ main (int argc, char **argv)
>    static_assert (ARRAY_SIZE (operand_types) + CLASS_WIDTH + INSTANCE_WIDTH
>                  == OTNum);
>
> -  c = OTNumOfBits - OTMax - 1;
> +  c = OTNumOfBits - OTNum;
>    if (c)
>      fail (_("%d unused bits in i386_operand_type.\n"), c);
>  #endif
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -435,7 +435,11 @@ enum
>    No_ldSuf,
>    /* instruction needs FWAIT */
>    FWait,
> -  /* quick test for string instructions */
> +  /* IsString provides for a quick test for string instructions, and
> +     its actual value also indicates which of the operands (if any)
> +     requires use of the %es segment.  */
> +#define IS_STRING_ES_OP0 2
> +#define IS_STRING_ES_OP1 3
>    IsString,
>    /* RegMem is for instructions with a modrm byte where the register
>       destination operand should be encoded in the mod and regmem fields.
> @@ -652,7 +656,7 @@ typedef struct i386_opcode_modifier
>    unsigned int no_qsuf:1;
>    unsigned int no_ldsuf:1;
>    unsigned int fwait:1;
> -  unsigned int isstring:1;
> +  unsigned int isstring:2;
>    unsigned int regmem:1;
>    unsigned int bndprefixok:1;
>    unsigned int notrackprefixok:1;
> @@ -763,8 +767,6 @@ enum
>    BaseIndex,
>    /* Absolute address for jump.  */
>    JumpAbsolute,
> -  /* String insn operand with fixed es segment */
> -  EsSeg,
>    /* BYTE size. */
>    Byte,
>    /* WORD size. 2 byte */
> @@ -798,8 +800,9 @@ enum
>    (OTNumOfUints * sizeof (unsigned int) * CHAR_BIT)
>
>  /* If you get a compiler error for zero width of the unused field,
> -   comment it out.  */
> +   comment it out.
>  #define OTUnused               OTNum
> +*/
>
>  typedef union i386_operand_type
>  {
> @@ -821,7 +824,6 @@ typedef union i386_operand_type
>        unsigned int disp64:1;
>        unsigned int baseindex:1;
>        unsigned int jumpabsolute:1;
> -      unsigned int esseg:1;
>        unsigned int byte:1;
>        unsigned int word:1;
>        unsigned int dword:1;
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -60,6 +60,9 @@
>  // RegMem implies a ModR/M byte
>  #define RegMem Modrm|RegMem
>
> +#define EsSegOp0 IsString=IS_STRING_ES_OP0
> +#define EsSegOp1 IsString=IS_STRING_ES_OP1
> +
>  #define VexW0 VexW=VEXW0
>  #define VexW1 VexW=VEXW1
>  #define VexWIG VexW=VEXWIG
> @@ -488,11 +491,11 @@ setg, 1, 0xf9f, 0x0, 2, Cpu386, Modrm|No
>
>  // String manipulation.
>  cmps, 0, 0xa6, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
> -cmps, 2, 0xa6, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
> +cmps, 2, 0xa6, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
>  scmp, 0, 0xa6, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
> -scmp, 2, 0xa6, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
> +scmp, 2, 0xa6, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Byte|Word|Dword|Qword|Unspecified|BaseIndex }

Please use

#define IsStringEsSegOp0 IsString=IS_STRING_ES_OP0
#define IsStringEsSegOp1 IsString=IS_STRING_ES_OP1

so that we can tell it a string instruction.

OK with this change.

Thanks.

-- 
H.J.



More information about the Binutils mailing list