[PATCH v2 1/4] x86: consolidate Disp<NN> handling a little

Jan Beulich jbeulich@suse.com
Mon Dec 23 11:02:00 GMT 2019


In memory operand addressing, which forms of displacement are permitted
besides Disp8 is pretty clearly limited
- outside of 64-bit mode, Disp16 or Disp32 only, depending on address
  size (MPX being special in not allowing Disp16),
- in 64-bit mode, Disp32s or Disp64 without address size override, and
  solely Disp32 with one.
Adjust assembler and i386-gen to match this, observing that templates
already get adjusted before trying to match them against input depending
on the presence of an address size prefix.

This adjustment logic gets extended to all cases, as certain DispNN
values should also be dropped when there's no such prefix. In fact
behavior of the assembler, perhaps besides the exact diagnostics wording,
should not differ between there being templates applicable to 64-bit and
non-64-bit at the same time, or there being fully separate sets of
templates, with their DispNN settings already reduced accordingly.

This adjustment logic further gets guarded such that there wouldn't be
and Disp<N> conversion based on address size prefix when this prefix
doesn't control the width of the displacement (on branches other than
absolute ones).

These adjustments then also allow folding two MOV templates, which had
been split between 64-bit and non-64-bits variants so far.

Once in this area also
- drop the bogus DispNN from JumpByte templates, leaving just the
  correct Disp8 there (compensated by i386_finalize_displacement()
  now setting Disp8 on their operands),
- add the missing Disp32S to XBEGIN.

Note that the changes make it necessary to temporarily mark a test as
XFAIL; this will get taken care of by a subsequent patch. The failing
parts are entirely bogus and will get replaced.

gas/
2019-12-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (i386_addressing_mode): Declare.
	(match_template): Don't transform displacement width flags for
	non-indirect branches. Re-write transformation logic.
	(i386_displacement): Also check BaseIndex when deciding whether
	an operand belongs to a direct branch. Restrict which DispNN get
	set.
	(i386_finalize_displacement): Set Disp8 for JumpByte templates.
	* config/tc-i386-intel.c (i386_intel_operand): Don't set Disp32
	for 64-bit addressing.
	* testsuite/gas/i386/i386.exp: XFail x86-64-branch-3.

opcodes/
2019-12-XX  Jan Beulich  <jbeulich@suse.com>

	* i386-gen.c (process_i386_operand_type): Don't set Disp32 for
	Cpu64 templates.
	* i386-opc.tbl (mov): Fold two templates.
	(jcxz, jecxz, jrcxz, loop, loope, loopne, loopnz, loopz): Drop
	Disp16, Disp32, and Disp32S.
	(xbegin): Add Disp32S.
	* i386-tbl.h: Re-generate.
---
v2: New, parts split off from later patch.

--- a/gas/config/tc-i386-intel.c
+++ b/gas/config/tc-i386-intel.c
@@ -939,12 +939,13 @@ i386_intel_operand (char *operand_string
 
 	  if (flag_code == CODE_64BIT)
 	    {
-	      i.types[this_operand].bitfield.disp32 = 1;
 	      if (!i.prefix[ADDR_PREFIX])
 		{
 		  i.types[this_operand].bitfield.disp64 = 1;
 		  i.types[this_operand].bitfield.disp32s = 1;
 		}
+	      else
+		i.types[this_operand].bitfield.disp32 = 1;
 	    }
 	  else if (!i.prefix[ADDR_PREFIX] ^ (flag_code == CODE_16BIT))
 	    i.types[this_operand].bitfield.disp32 = 1;
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -182,6 +182,7 @@ static char *parse_insn (char *, char *)
 static char *parse_operands (char *, const char *);
 static void swap_operands (void);
 static void swap_2_operands (int, int);
+static enum flag_code i386_addressing_mode (void);
 static void optimize_imm (void);
 static void optimize_disp (void);
 static const insn_template *match_template (char);
@@ -5883,51 +5884,50 @@ match_template (char mnem_suffix)
 	    break;
 	}
 
-      /* Address size prefix will turn Disp64/Disp32/Disp16 operand
-	 into Disp32/Disp16/Disp32 operand.  */
-      if (i.prefix[ADDR_PREFIX] != 0)
-	  {
-	    /* There should be only one Disp operand.  */
-	    switch (flag_code)
-	    {
-	    case CODE_16BIT:
-	      for (j = 0; j < MAX_OPERANDS; j++)
-		{
-		  if (operand_types[j].bitfield.disp16)
-		    {
-		      addr_prefix_disp = j;
-		      operand_types[j].bitfield.disp32 = 1;
-		      operand_types[j].bitfield.disp16 = 0;
-		      break;
-		    }
-		}
+      if (!t->opcode_modifier.jump
+	  || t->opcode_modifier.jump == JUMP_ABSOLUTE)
+	{
+	  /* There should be only one Disp operand.  */
+	  for (j = 0; j < MAX_OPERANDS; j++)
+	    if (operand_type_check (operand_types[j], disp))
 	      break;
-	    case CODE_32BIT:
-	      for (j = 0; j < MAX_OPERANDS; j++)
+	  if (j < MAX_OPERANDS)
+	    {
+	      bfd_boolean override = (i.prefix[ADDR_PREFIX] != 0);
+
+	      addr_prefix_disp = j;
+
+	      /* Address size prefix will turn Disp64/Disp32S/Disp32/Disp16
+		 operand into Disp32/Disp32/Disp16/Disp32 operand.  */
+	      switch (flag_code)
 		{
-		  if (operand_types[j].bitfield.disp32)
+		case CODE_16BIT:
+		  override = !override;
+		  /* Fall through.  */
+		case CODE_32BIT:
+		  if (operand_types[j].bitfield.disp32
+		      && operand_types[j].bitfield.disp16)
 		    {
-		      addr_prefix_disp = j;
-		      operand_types[j].bitfield.disp32 = 0;
-		      operand_types[j].bitfield.disp16 = 1;
-		      break;
+		      operand_types[j].bitfield.disp16 = override;
+		      operand_types[j].bitfield.disp32 = !override;
 		    }
-		}
-	      break;
-	    case CODE_64BIT:
-	      for (j = 0; j < MAX_OPERANDS; j++)
-		{
-		  if (operand_types[j].bitfield.disp64)
+		  operand_types[j].bitfield.disp32s = 0;
+		  operand_types[j].bitfield.disp64 = 0;
+		  break;
+
+		case CODE_64BIT:
+		  if (operand_types[j].bitfield.disp32s
+		      || operand_types[j].bitfield.disp64)
 		    {
-		      addr_prefix_disp = j;
-		      operand_types[j].bitfield.disp64 = 0;
-		      operand_types[j].bitfield.disp32 = 1;
-		      break;
+		      operand_types[j].bitfield.disp64 &= !override;
+		      operand_types[j].bitfield.disp32s &= !override;
+		      operand_types[j].bitfield.disp32 = override;
 		    }
+		  operand_types[j].bitfield.disp16 = 0;
+		  break;
 		}
-	      break;
 	    }
-	  }
+	}
 
       /* Force 0x8b encoding for "mov foo@GOT, %eax".  */
       if (i.reloc[0] == BFD_RELOC_386_GOT32 && t->base_opcode == 0xa0)
@@ -9937,10 +9937,11 @@ i386_displacement (char *disp_start, cha
 
   operand_type_set (&bigdisp, 0);
   if (i.jumpabsolute
+      || i.types[this_operand].bitfield.baseindex
       || (current_templates->start->opcode_modifier.jump != JUMP
 	  && current_templates->start->opcode_modifier.jump != JUMP_DWORD))
     {
-      bigdisp.bitfield.disp32 = 1;
+      i386_addressing_mode ();
       override = (i.prefix[ADDR_PREFIX] != 0);
       if (flag_code == CODE_64BIT)
 	{
@@ -9949,12 +9950,13 @@ i386_displacement (char *disp_start, cha
 	      bigdisp.bitfield.disp32s = 1;
 	      bigdisp.bitfield.disp64 = 1;
 	    }
+	  else
+	    bigdisp.bitfield.disp32 = 1;
 	}
       else if ((flag_code == CODE_16BIT) ^ override)
-	{
-	  bigdisp.bitfield.disp32 = 0;
 	  bigdisp.bitfield.disp16 = 1;
-	}
+      else
+	  bigdisp.bitfield.disp32 = 1;
     }
   else
     {
@@ -9966,10 +9968,7 @@ i386_displacement (char *disp_start, cha
 	  if (override || i.suffix == WORD_MNEM_SUFFIX)
 	    bigdisp.bitfield.disp16 = 1;
 	  else
-	    {
-	      bigdisp.bitfield.disp32 = 1;
-	      bigdisp.bitfield.disp32s = 1;
-	    }
+	    bigdisp.bitfield.disp32s = 1;
 	}
       else
 	{
@@ -10142,6 +10141,11 @@ i386_finalize_displacement (segT exp_seg
     }
 #endif
 
+  if (current_templates->start->opcode_modifier.jump == JUMP_BYTE
+      /* Constants get taken care of by optimize_disp().  */
+      && exp->X_op != O_constant)
+    i.types[this_operand].bitfield.disp8 = 1;
+
   /* Check if this is a displacement only operand.  */
   bigdisp = i.types[this_operand];
   bigdisp.bitfield.disp8 = 0;
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -1124,6 +1124,7 @@ if [expr ([istarget "i*86-*-*"] || [ista
 
 	run_dump_test "x86-64-jump"
 	run_dump_test "x86-64-branch-2"
+	setup_xfail "*-*-*"
 	run_list_test "x86-64-branch-3" "-al -mintel64"
 	run_list_test "x86-64-branch-4" "-al -mintel64"
 
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1236,7 +1236,8 @@ process_i386_operand_type (FILE *table,
 	  if (!active_cpu_flags.bitfield.cpu64
 	      && !active_cpu_flags.bitfield.cpumpx)
 	    set_bitfield("Disp16", types, 1, ARRAY_SIZE (types), lineno);
-	  set_bitfield("Disp32", types, 1, ARRAY_SIZE (types), lineno);
+	  if (!active_cpu_flags.bitfield.cpu64)
+	    set_bitfield("Disp32", types, 1, ARRAY_SIZE (types), lineno);
 	  if (!active_cpu_flags.bitfield.cpuno64)
 	    set_bitfield("Disp32S", types, 1, ARRAY_SIZE (types), lineno);
 	}
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -90,10 +90,7 @@
 ### MARKER ###
 
 // Move instructions.
-// We put the 64bit displacement first and we only mark constants
-// larger than 32bit as Disp64.
-mov, 2, 0xa0, None, 1, Cpu64, D|W|No_sSuf|No_ldSuf, { Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
-mov, 2, 0xa0, None, 1, CpuNo64, D|W|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Unspecified|Byte|Word|Dword, Acc|Byte|Word|Dword }
+mov, 2, 0xa0, None, 1, 0, D|W|No_sSuf|No_qSuf|No_ldSuf, { Disp16|Disp32|Disp64|Unspecified|Byte|Word|Dword|Qword, Acc|Byte|Word|Dword|Qword }
 mov, 2, 0x88, None, 1, 0, D|W|CheckRegSize|Modrm|No_sSuf|No_ldSuf|HLEPrefixOk=3, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }
 // In the 64bit mode the short form mov immediate is redefined to have
 // 64bit value.
@@ -447,24 +444,24 @@ jnle, 1, 0x7f, None, 1, 0, Jump|No_bSuf|
 jg, 1, 0x7f, None, 1, 0, Jump|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|BNDPrefixOk, { Disp8|Disp16|Disp32|Disp32S }
 
 // jcxz vs. jecxz is chosen on the basis of the address size prefix.
-jcxz, 1, 0xe3, None, 1, CpuNo64, JumpByte|Size16|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8|Disp16|Disp32 }
-jecxz, 1, 0xe3, None, 1, 0, JumpByte|Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8|Disp16|Disp32|Disp32S }
-jrcxz, 1, 0xe3, None, 1, Cpu64, JumpByte|Size64|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Disp8|Disp32|Disp32S }
+jcxz, 1, 0xe3, None, 1, CpuNo64, JumpByte|Size16|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
+jecxz, 1, 0xe3, None, 1, 0, JumpByte|Size32|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
+jrcxz, 1, 0xe3, None, 1, Cpu64, JumpByte|Size64|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Disp8 }
 
 // The loop instructions also use the address size prefix to select
 // %cx rather than %ecx for the loop count, so the `w' form of these
 // instructions emit an address size prefix rather than a data size
 //  prefix.
-loop, 1, 0xe2, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8|Disp16|Disp32 }
-loop, 1, 0xe2, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8|Disp32|Disp32S }
-loopz, 1, 0xe1, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8|Disp16|Disp32 }
-loopz, 1, 0xe1, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8|Disp32|Disp32S }
-loope, 1, 0xe1, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8|Disp16|Disp32 }
-loope, 1, 0xe1, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8|Disp32|Disp32S }
-loopnz, 1, 0xe0, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8|Disp16|Disp32 }
-loopnz, 1, 0xe0, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8|Disp32|Disp32S }
-loopne, 1, 0xe0, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8|Disp16|Disp32 }
-loopne, 1, 0xe0, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8|Disp32|Disp32S }
+loop, 1, 0xe2, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
+loop, 1, 0xe2, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8 }
+loopz, 1, 0xe1, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
+loopz, 1, 0xe1, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8 }
+loope, 1, 0xe1, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
+loope, 1, 0xe1, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8 }
+loopnz, 1, 0xe0, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
+loopnz, 1, 0xe0, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8 }
+loopne, 1, 0xe0, None, 1, CpuNo64, JumpByte|No_bSuf|No_sSuf|No_qSuf|No_ldSuf, { Disp8 }
+loopne, 1, 0xe0, None, 1, Cpu64, JumpByte|No_bSuf|No_wSuf|No_sSuf|No_ldSuf|NoRex64, { Disp8 }
 
 // Set byte on flag instructions.
 seto, 1, 0xf90, 0x0, 2, Cpu386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg8|Byte|Unspecified|BaseIndex }
@@ -2552,7 +2549,7 @@ xrelease, 0, 0xf3, None, 1, CpuHLE, No_b
 
 // RTM instructions
 xabort, 1, 0xc6f8, None, 2, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Imm8 }
-xbegin, 1, 0xc7f8, None, 2, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Disp16|Disp32 }
+xbegin, 1, 0xc7f8, None, 2, CpuRTM,  JumpDword|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64, { Disp16|Disp32|Disp32S }
 xend, 0, 0xf01d5, None, 3, CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { 0 }
 xtest, 0, 0xf01d6, None, 3, CpuHLE|CpuRTM, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { 0 }
 



More information about the Binutils mailing list