[10/13] x86: fold EsSeg into IsString

Message ID bcc29b62-ada4-7951-7d11-441a66697bee@suse.com
State New
Headers show
Series
  • x86: further insn template compaction
Related show

Commit Message

Jan Beulich Oct. 30, 2019, 8:27 a.m.
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.

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.

Comments

H.J. Lu Oct. 31, 2019, 7:39 p.m. | #1
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.
Jan Beulich Nov. 4, 2019, 4:15 p.m. | #2
On 31.10.2019 20:39, H.J. Lu wrote:
> On Wed, Oct 30, 2019 at 1:27 AM Jan Beulich <jbeulich@suse.com> wrote:

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


Would IsStringEsOp0 or even IsStringEs0 also be okay with you?

Jan
H.J. Lu Nov. 4, 2019, 5:16 p.m. | #3
On Mon, Nov 4, 2019 at 8:15 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 31.10.2019 20:39, H.J. Lu wrote:

> > On Wed, Oct 30, 2019 at 1:27 AM Jan Beulich <jbeulich@suse.com> wrote:

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

>

> Would IsStringEsOp0 or even IsStringEs0 also be okay with you?

>

> Jan


IsStringEsOp0 looks good to me.

-- 
H.J.

Patch

--- 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 }
 ins, 0, 0x6c, None, 1, Cpu186, W|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-ins, 2, 0x6c, None, 1, Cpu186, W|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { InOutPortReg, Byte|Word|Dword|Unspecified|BaseIndex|EsSeg }
+ins, 2, 0x6c, None, 1, Cpu186, W|No_sSuf|No_qSuf|No_ldSuf|EsSegOp1|RepPrefixOk, { InOutPortReg, Byte|Word|Dword|Unspecified|BaseIndex }
 outs, 0, 0x6e, None, 1, Cpu186, W|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
 outs, 2, 0x6e, None, 1, Cpu186, W|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Unspecified|BaseIndex, InOutPortReg }
 lods, 0, 0xac, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
@@ -502,21 +505,21 @@  slod, 0, 0xac, None, 1, 0, W|No_sSuf|No_
 slod, 1, 0xac, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 slod, 2, 0xac, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Acc|Byte|Word|Dword|Qword }
 movs, 0, 0xa4, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-movs, 2, 0xa4, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
+movs, 2, 0xa4, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp1|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 smov, 0, 0xa4, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-smov, 2, 0xa4, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
+smov, 2, 0xa4, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp1|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 scas, 0, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-scas, 1, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
-scas, 2, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg, Acc|Byte|Word|Dword|Qword }
+scas, 1, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex }
+scas, 2, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Acc|Byte|Word|Dword|Qword }
 ssca, 0, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-ssca, 1, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
-ssca, 2, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg, Acc|Byte|Word|Dword|Qword }
+ssca, 1, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex }
+ssca, 2, 0xae, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex, Acc|Byte|Word|Dword|Qword }
 stos, 0, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-stos, 1, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
-stos, 2, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Acc|Byte|Word|Dword|Qword, Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
+stos, 1, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex }
+stos, 2, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp1|RepPrefixOk, { Acc|Byte|Word|Dword|Qword, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 ssto, 0, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-ssto, 1, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
-ssto, 2, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|IsString|RepPrefixOk, { Acc|Byte|Word|Dword|Qword, Byte|Word|Dword|Qword|Unspecified|BaseIndex|EsSeg }
+ssto, 1, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Byte|Word|Dword|Qword|Unspecified|BaseIndex }
+ssto, 2, 0xaa, None, 1, 0, W|No_sSuf|No_ldSuf|EsSegOp1|RepPrefixOk, { Acc|Byte|Word|Dword|Qword, Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 xlat, 0, 0xd7, None, 1, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString, { 0 }
 xlat, 1, 0xd7, None, 1, 0, No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString, { Byte|Unspecified|BaseIndex }
 
@@ -1408,8 +1411,8 @@  cmpunordsd, 2, 0xf20fc2, 0x3, 2, CpuSSE2
 cmppd, 3, 0x66c2, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexVVVV=1|VexW=1|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Imm8, RegXMM|Unspecified|BaseIndex, RegXMM }
 cmppd, 3, 0x660fc2, None, 2, CpuSSE2, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8, RegXMM|Unspecified|BaseIndex, RegXMM }
 // Intel mode string compare.
-cmpsd, 0, 0xa7, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-cmpsd, 2, 0xa7, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex|EsSeg, Dword|Unspecified|BaseIndex }
+cmpsd, 0, 0xa7, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
+cmpsd, 2, 0xa7, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|EsSegOp0|RepPrefixOk, { Dword|Unspecified|BaseIndex, Dword|Unspecified|BaseIndex }
 cmpsd, 3, 0xf2c2, None, 1, CpuAVX, Modrm|Vex=3|VexOpcode=0|VexVVVV=1|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Imm8, Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
 cmpsd, 3, 0xf20fc2, None, 2, CpuSSE2, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8, Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
 comisd, 2, 0x662f, None, 1, CpuAVX, Modrm|Vex=3|VexOpcode=0|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Qword|Unspecified|BaseIndex|RegXMM, RegXMM }
@@ -1444,8 +1447,8 @@  movmskpd, 2, 0x660f50, None, 2, CpuSSE2,
 movntpd, 2, 0x662b, None, 1, CpuAVX, Modrm|Vex|VexOpcode=0|VexW=1|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { RegXMM, Xmmword|Unspecified|BaseIndex }
 movntpd, 2, 0x660f2b, None, 2, CpuSSE2, Modrm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegXMM, Xmmword|Unspecified|BaseIndex }
 // Intel mode string move.
-movsd, 0, 0xa5, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
-movsd, 2, 0xa5, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { Dword|Unspecified|BaseIndex, Dword|Unspecified|BaseIndex|EsSeg }
+movsd, 0, 0xa5, None, 1, 0, Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString|RepPrefixOk, { 0 }
+movsd, 2, 0xa5, None, 1, 0, Size32|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|EsSegOp1|RepPrefixOk, { Dword|Unspecified|BaseIndex, Dword|Unspecified|BaseIndex }
 movsd, 2, 0xf210, None, 1, CpuAVX, D|Modrm|Vex=3|VexOpcode=0|VexW=1|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { Qword|Unspecified|BaseIndex, RegXMM }
 movsd, 2, 0xf210, None, 1, CpuAVX, D|Modrm|Vex=3|VexOpcode=0|VexVVVV=1|VexW=1|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|SSE2AVX, { RegXMM, RegXMM }
 movsd, 2, 0xf20f10, None, 2, CpuSSE2, D|Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Qword|Unspecified|BaseIndex|RegXMM, RegXMM }