public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/8] x86: work towards further opcode table compaction
@ 2021-03-22 16:40 Jan Beulich
  2021-03-22 16:42 ` [PATCH 1/8] x86: unbreak certain MPX insn operand forms Jan Beulich
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Jan Beulich @ 2021-03-22 16:40 UTC (permalink / raw)
  To: Binutils

This is partial work for now only, because I'm not sure how controversial
some of the later the changes might be, and hence I wouldn't want to
continue down this road without at least rough consensus. The bugs fixed
by the first patch were noticed simply in the course of doing this rework.

1: unbreak certain MPX insn operand forms
2: don't open-code PREFIX_NONE
3: don't use opcode_length to identify pseudo prefixes
4: split opcode prefix and opcode space representation
5: re-order two fields of struct insn_template
6: re-number PREFIX_0X<nn>
7: derive mandatory prefix attribute from base opcode
8: derive opcode length from opcode value

Jan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/8] x86: unbreak certain MPX insn operand forms
  2021-03-22 16:40 [PATCH 0/8] x86: work towards further opcode table compaction Jan Beulich
@ 2021-03-22 16:42 ` Jan Beulich
  2021-03-22 17:58   ` H.J. Lu
  2021-03-22 16:42 ` [PATCH 2/8] x86: don't open-code PREFIX_NONE Jan Beulich
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-22 16:42 UTC (permalink / raw)
  To: Binutils

Commit 8b65b8953af2 ("x86: Remove the prefix byte from non-VEX/EVEX
base_opcode") dropped the mandatory prefix bytes from legacy encoded
insn templates, but failed to also adjust affected MPX-specific checks
in two places.

For the expressions to remain halfway readable, introduce local
variables to hold current_templates->start.

gas/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (i386_index_check): New local variable t.
	Correct MPX insn check.
	* config/tc-i386-intel.c (i386_intel_simplify_register): Correct
	MPX insn check.
	* testsuite/gas/i386/x86-64-mpx.s: Add RIP-relative cases. Test
	index scaling by other than 1.
	* testsuite/gas/i386/x86-64-mpx.d: Adjust expectations.

--- a/gas/config/tc-i386-intel.c
+++ b/gas/config/tc-i386-intel.c
@@ -314,9 +314,13 @@ i386_intel_simplify_register (expression
     intel_state.base = i386_regtab + reg_num;
   else if (!intel_state.index)
     {
+      const insn_template *t = current_templates->start;
+
       if (intel_state.in_scale
-	  || current_templates->start->base_opcode == 0xf30f1b /* bndmk */
-	  || (current_templates->start->base_opcode & ~1) == 0x0f1a /* bnd{ld,st}x */
+	  || (t->opcode_modifier.opcodeprefix == PREFIX_0XF3
+	      && t->base_opcode == 0x0f1b /* bndmk */)
+	  || (t->opcode_modifier.opcodeprefix == PREFIX_NONE
+	      && (t->base_opcode & ~1) == 0x0f1a /* bnd{ld,st}x */)
 	  || i386_regtab[reg_num].reg_type.bitfield.baseindex)
 	intel_state.index = i386_regtab + reg_num;
       else
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -11031,9 +11031,10 @@ i386_index_check (const char *operand_st
 {
   const char *kind = "base/index";
   enum flag_code addr_mode = i386_addressing_mode ();
+  const insn_template *t = current_templates->start;
 
-  if (current_templates->start->opcode_modifier.isstring
-      && !current_templates->start->cpu_flags.bitfield.cpupadlock
+  if (t->opcode_modifier.isstring
+      && !t->cpu_flags.bitfield.cpupadlock
       && (current_templates->end[-1].opcode_modifier.isstring
 	  || i.mem_operands))
     {
@@ -11050,7 +11051,7 @@ i386_index_check (const char *operand_st
 
       kind = "string address";
 
-      if (current_templates->start->opcode_modifier.prefixok == PrefixRep)
+      if (t->opcode_modifier.prefixok == PrefixRep)
 	{
 	  int es_op = current_templates->end[-1].opcode_modifier.isstring
 		      - IS_STRING_ES_OP0;
@@ -11130,9 +11131,11 @@ i386_index_check (const char *operand_st
 	    goto bad_address;
 
 	  /* bndmk, bndldx, bndstx and mandatory non-vector SIB have special restrictions. */
-	  if (current_templates->start->base_opcode == 0xf30f1b
-	      || (current_templates->start->base_opcode & ~1) == 0x0f1a
-	      || current_templates->start->opcode_modifier.sib == SIBMEM)
+	  if ((t->opcode_modifier.opcodeprefix == PREFIX_0XF3
+	       && t->base_opcode == 0x0f1b)
+	      || (t->opcode_modifier.opcodeprefix == PREFIX_NONE
+		  && (t->base_opcode & ~1) == 0x0f1a)
+	      || t->opcode_modifier.sib == SIBMEM)
 	    {
 	      /* They cannot use RIP-relative addressing. */
 	      if (i.base_reg && i.base_reg->reg_num == RegIP)
@@ -11142,7 +11145,8 @@ i386_index_check (const char *operand_st
 		}
 
 	      /* bndldx and bndstx ignore their scale factor. */
-	      if ((current_templates->start->base_opcode & ~1) == 0x0f1a
+	      if (t->opcode_modifier.opcodeprefix == PREFIX_NONE
+		  && (t->base_opcode & ~1) == 0x0f1a
 		  && i.log2_scale_factor)
 		as_warn (_("register scaling is being ignored here"));
 	    }
--- a/gas/testsuite/gas/i386/x86-64-mpx.d
+++ b/gas/testsuite/gas/i386/x86-64-mpx.d
@@ -14,16 +14,17 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	f3 0f 1b 48 03       	bndmk  0x3\(%rax\),%bnd1
 [ 	]*[a-f0-9]+:	f3 42 0f 1b 0c 25 03 00 00 00 	bndmk  0x3\(,%r12,1\),%bnd1
 [ 	]*[a-f0-9]+:	f3 0f 1b 0c 08       	bndmk  \(%rax,%rcx,1\),%bnd1
-[ 	]*[a-f0-9]+:	f3 41 0f 1b 4c 03 03 	bndmk  0x3\(%r11,%rax,1\),%bnd1
+[ 	]*[a-f0-9]+:	f3 41 0f 1b 4c 43 03 	bndmk  0x3\(%r11,%rax,2\),%bnd1
 [ 	]*[a-f0-9]+:	f3 42 0f 1b 4c 0b 03 	bndmk  0x3\(%rbx,%r9,1\),%bnd1
 [ 	]*[a-f0-9]+:	66 41 0f 1a 0b       	bndmov \(%r11\),%bnd1
 [ 	]*[a-f0-9]+:	66 0f 1a 08          	bndmov \(%rax\),%bnd1
 [ 	]*[a-f0-9]+:	66 0f 1a 0c 25 99 03 00 00 	bndmov 0x399,%bnd1
 [ 	]*[a-f0-9]+:	66 41 0f 1a 51 03    	bndmov 0x3\(%r9\),%bnd2
 [ 	]*[a-f0-9]+:	66 0f 1a 50 03       	bndmov 0x3\(%rax\),%bnd2
+[ 	]*[a-f0-9]+:	66 0f 1a 15 33 33 00 00 	bndmov 0x3333\(%rip\),%bnd2 ?.*
 [ 	]*[a-f0-9]+:	66 42 0f 1a 04 25 03 00 00 00 	bndmov 0x3\(,%r12,1\),%bnd0
 [ 	]*[a-f0-9]+:	66 0f 1a 14 10       	bndmov \(%rax,%rdx,1\),%bnd2
-[ 	]*[a-f0-9]+:	66 41 0f 1a 4c 03 03 	bndmov 0x3\(%r11,%rax,1\),%bnd1
+[ 	]*[a-f0-9]+:	66 41 0f 1a 4c 43 03 	bndmov 0x3\(%r11,%rax,2\),%bnd1
 [ 	]*[a-f0-9]+:	66 42 0f 1a 4c 0b 03 	bndmov 0x3\(%rbx,%r9,1\),%bnd1
 [ 	]*[a-f0-9]+:	66 0f 1a c2          	bndmov %bnd2,%bnd0
 [ 	]*[a-f0-9]+:	66 41 0f 1b 0b       	bndmov %bnd1,\(%r11\)
@@ -31,9 +32,10 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	66 0f 1b 0c 25 99 03 00 00 	bndmov %bnd1,0x399
 [ 	]*[a-f0-9]+:	66 41 0f 1b 51 03    	bndmov %bnd2,0x3\(%r9\)
 [ 	]*[a-f0-9]+:	66 0f 1b 50 03       	bndmov %bnd2,0x3\(%rax\)
+[ 	]*[a-f0-9]+:	66 0f 1b 15 33 33 00 00 	bndmov %bnd2,0x3333\(%rip\) ?.*
 [ 	]*[a-f0-9]+:	66 42 0f 1b 04 25 03 00 00 00 	bndmov %bnd0,0x3\(,%r12,1\)
 [ 	]*[a-f0-9]+:	66 0f 1b 14 10       	bndmov %bnd2,\(%rax,%rdx,1\)
-[ 	]*[a-f0-9]+:	66 41 0f 1b 4c 03 03 	bndmov %bnd1,0x3\(%r11,%rax,1\)
+[ 	]*[a-f0-9]+:	66 41 0f 1b 4c 43 03 	bndmov %bnd1,0x3\(%r11,%rax,2\)
 [ 	]*[a-f0-9]+:	66 42 0f 1b 4c 0b 03 	bndmov %bnd1,0x3\(%rbx,%r9,1\)
 [ 	]*[a-f0-9]+:	66 0f 1a d0          	bndmov %bnd0,%bnd2
 [ 	]*[a-f0-9]+:	f3 41 0f 1a 0b       	bndcl  \(%r11\),%bnd1
@@ -43,9 +45,10 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	f3 0f 1a 0c 25 99 03 00 00 	bndcl  0x399,%bnd1
 [ 	]*[a-f0-9]+:	f3 41 0f 1a 51 03    	bndcl  0x3\(%r9\),%bnd2
 [ 	]*[a-f0-9]+:	f3 0f 1a 50 03       	bndcl  0x3\(%rax\),%bnd2
+[ 	]*[a-f0-9]+:	f3 0f 1a 15 33 33 00 00 	bndcl  0x3333\(%rip\),%bnd2 ?.*
 [ 	]*[a-f0-9]+:	f3 42 0f 1a 04 25 03 00 00 00 	bndcl  0x3\(,%r12,1\),%bnd0
 [ 	]*[a-f0-9]+:	f3 0f 1a 14 10       	bndcl  \(%rax,%rdx,1\),%bnd2
-[ 	]*[a-f0-9]+:	f3 41 0f 1a 4c 03 03 	bndcl  0x3\(%r11,%rax,1\),%bnd1
+[ 	]*[a-f0-9]+:	f3 41 0f 1a 4c 43 03 	bndcl  0x3\(%r11,%rax,2\),%bnd1
 [ 	]*[a-f0-9]+:	f3 42 0f 1a 4c 0b 03 	bndcl  0x3\(%rbx,%r9,1\),%bnd1
 [ 	]*[a-f0-9]+:	f2 41 0f 1a 0b       	bndcu  \(%r11\),%bnd1
 [ 	]*[a-f0-9]+:	f2 0f 1a 08          	bndcu  \(%rax\),%bnd1
@@ -54,9 +57,10 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	f2 0f 1a 0c 25 99 03 00 00 	bndcu  0x399,%bnd1
 [ 	]*[a-f0-9]+:	f2 41 0f 1a 51 03    	bndcu  0x3\(%r9\),%bnd2
 [ 	]*[a-f0-9]+:	f2 0f 1a 50 03       	bndcu  0x3\(%rax\),%bnd2
+[ 	]*[a-f0-9]+:	f2 0f 1a 15 33 33 00 00 	bndcu  0x3333\(%rip\),%bnd2 ?.*
 [ 	]*[a-f0-9]+:	f2 42 0f 1a 04 25 03 00 00 00 	bndcu  0x3\(,%r12,1\),%bnd0
 [ 	]*[a-f0-9]+:	f2 0f 1a 14 10       	bndcu  \(%rax,%rdx,1\),%bnd2
-[ 	]*[a-f0-9]+:	f2 41 0f 1a 4c 03 03 	bndcu  0x3\(%r11,%rax,1\),%bnd1
+[ 	]*[a-f0-9]+:	f2 41 0f 1a 4c 43 03 	bndcu  0x3\(%r11,%rax,2\),%bnd1
 [ 	]*[a-f0-9]+:	f2 42 0f 1a 4c 0b 03 	bndcu  0x3\(%rbx,%r9,1\),%bnd1
 [ 	]*[a-f0-9]+:	f2 41 0f 1b 0b       	bndcn  \(%r11\),%bnd1
 [ 	]*[a-f0-9]+:	f2 0f 1b 08          	bndcn  \(%rax\),%bnd1
@@ -65,9 +69,10 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	f2 0f 1b 0c 25 99 03 00 00 	bndcn  0x399,%bnd1
 [ 	]*[a-f0-9]+:	f2 41 0f 1b 51 03    	bndcn  0x3\(%r9\),%bnd2
 [ 	]*[a-f0-9]+:	f2 0f 1b 50 03       	bndcn  0x3\(%rax\),%bnd2
+[ 	]*[a-f0-9]+:	f2 0f 1b 15 33 33 00 00 	bndcn  0x3333\(%rip\),%bnd2 ?.*
 [ 	]*[a-f0-9]+:	f2 42 0f 1b 04 25 03 00 00 00 	bndcn  0x3\(,%r12,1\),%bnd0
 [ 	]*[a-f0-9]+:	f2 0f 1b 14 10       	bndcn  \(%rax,%rdx,1\),%bnd2
-[ 	]*[a-f0-9]+:	f2 41 0f 1b 4c 03 03 	bndcn  0x3\(%r11,%rax,1\),%bnd1
+[ 	]*[a-f0-9]+:	f2 41 0f 1b 4c 43 03 	bndcn  0x3\(%r11,%rax,2\),%bnd1
 [ 	]*[a-f0-9]+:	f2 42 0f 1b 4c 0b 03 	bndcn  0x3\(%rbx,%r9,1\),%bnd1
 [ 	]*[a-f0-9]+:	0f 1b 44 18 03       	bndstx %bnd0,0x3\(%rax,%rbx,1\)
 [ 	]*[a-f0-9]+:	0f 1b 54 13 03       	bndstx %bnd2,0x3\(%rbx,%rdx,1\)
@@ -85,11 +90,11 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	0f 1a 14 1d 03 00 00 00 	bndldx 0x3\(,%rbx,1\),%bnd2
 [ 	]*[a-f0-9]+:	42 0f 1a 14 25 03 00 00 00 	bndldx 0x3\(,%r12,1\),%bnd2
 [ 	]*[a-f0-9]+:	0f 1a 0a             	bndldx \(%rdx\),%bnd1
-[ 	]*[a-f0-9]+:	f2 e8 25 02 00 00    	bnd call 416 <foo>
+[ 	]*[a-f0-9]+:	f2 e8 25 02 00 00    	bnd call [0-9a-f]+ <foo>
 [ 	]*[a-f0-9]+:	f2 ff 10             	bnd call \*\(%rax\)
 [ 	]*[a-f0-9]+:	f2 41 ff 13          	bnd call \*\(%r11\)
-[ 	]*[a-f0-9]+:	f2 0f 84 17 02 00 00 	bnd je 416 <foo>
-[ 	]*[a-f0-9]+:	f2 e9 11 02 00 00    	bnd jmp 416 <foo>
+[ 	]*[a-f0-9]+:	f2 0f 84 17 02 00 00 	bnd je [0-9a-f]+ <foo>
+[ 	]*[a-f0-9]+:	f2 e9 11 02 00 00    	bnd jmp [0-9a-f]+ <foo>
 [ 	]*[a-f0-9]+:	f2 ff 21             	bnd jmp \*\(%rcx\)
 [ 	]*[a-f0-9]+:	f2 41 ff 24 24       	bnd jmp \*\(%r12\)
 [ 	]*[a-f0-9]+:	f2 c3                	bnd ret *
@@ -171,11 +176,11 @@ Disassembly of section .text:
 [ 	]*[a-f0-9]+:	0f 1a 14 1d 03 00 00 00 	bndldx 0x3\(,%rbx,1\),%bnd2
 [ 	]*[a-f0-9]+:	42 0f 1a 14 25 03 00 00 00 	bndldx 0x3\(,%r12,1\),%bnd2
 [ 	]*[a-f0-9]+:	0f 1a 0a             	bndldx \(%rdx\),%bnd1
-[ 	]*[a-f0-9]+:	f2 e8 16 00 00 00    	bnd call 416 <foo>
+[ 	]*[a-f0-9]+:	f2 e8 16 00 00 00    	bnd call [0-9a-f]+ <foo>
 [ 	]*[a-f0-9]+:	f2 ff d0             	bnd call \*%rax
 [ 	]*[a-f0-9]+:	f2 41 ff d3          	bnd call \*%r11
-[ 	]*[a-f0-9]+:	f2 74 0c             	bnd je 416 <foo>
-[ 	]*[a-f0-9]+:	f2 eb 09             	bnd jmp 416 <foo>
+[ 	]*[a-f0-9]+:	f2 74 0c             	bnd je [0-9a-f]+ <foo>
+[ 	]*[a-f0-9]+:	f2 eb 09             	bnd jmp [0-9a-f]+ <foo>
 [ 	]*[a-f0-9]+:	f2 ff e1             	bnd jmp \*%rcx
 [ 	]*[a-f0-9]+:	f2 41 ff e4          	bnd jmp \*%r12
 [ 	]*[a-f0-9]+:	f2 c3                	bnd ret *
--- a/gas/testsuite/gas/i386/x86-64-mpx.s
+++ b/gas/testsuite/gas/i386/x86-64-mpx.s
@@ -10,7 +10,7 @@ start:
 	bndmk 0x3(%rax), %bnd1
 	bndmk 0x3(,%r12,1), %bnd1
 	bndmk (%rax,%rcx), %bnd1
-	bndmk 0x3(%r11,%rax,1), %bnd1
+	bndmk 0x3(%r11,%rax,2), %bnd1
 	bndmk 0x3(%rbx,%r9,1), %bnd1
 
 	### bndmov
@@ -19,9 +19,10 @@ start:
 	bndmov (0x399), %bnd1
 	bndmov 0x3(%r9), %bnd2
 	bndmov 0x3(%rax), %bnd2
+	bndmov 0x3333(%rip), %bnd2
 	bndmov 0x3(,%r12,1), %bnd0
 	bndmov (%rax,%rdx), %bnd2
-	bndmov 0x3(%r11,%rax,1), %bnd1
+	bndmov 0x3(%r11,%rax,2), %bnd1
 	bndmov 0x3(%rbx,%r9,1), %bnd1
 	bndmov %bnd2, %bnd0
 
@@ -30,9 +31,10 @@ start:
 	bndmov %bnd1, (0x399)
 	bndmov %bnd2, 0x3(%r9)
 	bndmov %bnd2, 0x3(%rax)
+	bndmov %bnd2, 0x3333(%rip)
 	bndmov %bnd0, 0x3(,%r12,1)
 	bndmov %bnd2, (%rax,%rdx)
-	bndmov %bnd1, 0x3(%r11,%rax,1)
+	bndmov %bnd1, 0x3(%r11,%rax,2)
 	bndmov %bnd1, 0x3(%rbx,%r9,1)
 	bndmov %bnd0, %bnd2
 
@@ -44,9 +46,10 @@ start:
 	bndcl (0x399), %bnd1
 	bndcl 0x3(%r9), %bnd2
 	bndcl 0x3(%rax), %bnd2
+	bndcl 0x3333(%rip), %bnd2
 	bndcl 0x3(,%r12,1), %bnd0
 	bndcl (%rax,%rdx), %bnd2
-	bndcl 0x3(%r11,%rax,1), %bnd1
+	bndcl 0x3(%r11,%rax,2), %bnd1
 	bndcl 0x3(%rbx,%r9,1), %bnd1
 
 	### bndcu
@@ -57,9 +60,10 @@ start:
 	bndcu (0x399), %bnd1
 	bndcu 0x3(%r9), %bnd2
 	bndcu 0x3(%rax), %bnd2
+	bndcu 0x3333(%rip), %bnd2
 	bndcu 0x3(,%r12,1), %bnd0
 	bndcu (%rax,%rdx), %bnd2
-	bndcu 0x3(%r11,%rax,1), %bnd1
+	bndcu 0x3(%r11,%rax,2), %bnd1
 	bndcu 0x3(%rbx,%r9,1), %bnd1
 
 	### bndcn
@@ -70,9 +74,10 @@ start:
 	bndcn (0x399), %bnd1
 	bndcn 0x3(%r9), %bnd2
 	bndcn 0x3(%rax), %bnd2
+	bndcn 0x3333(%rip), %bnd2
 	bndcn 0x3(,%r12,1), %bnd0
 	bndcn (%rax,%rdx), %bnd2
-	bndcn 0x3(%r11,%rax,1), %bnd1
+	bndcn 0x3(%r11,%rax,2), %bnd1
 	bndcn 0x3(%rbx,%r9,1), %bnd1
 
 	### bndstx


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/8] x86: don't open-code PREFIX_NONE
  2021-03-22 16:40 [PATCH 0/8] x86: work towards further opcode table compaction Jan Beulich
  2021-03-22 16:42 ` [PATCH 1/8] x86: unbreak certain MPX insn operand forms Jan Beulich
@ 2021-03-22 16:42 ` Jan Beulich
  2021-03-22 17:58   ` H.J. Lu
  2021-03-22 16:43 ` [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes Jan Beulich
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-22 16:42 UTC (permalink / raw)
  To: Binutils

Use the constant rather than literal zero. While at it fold two
conditionals (using the same base opcode and prefix) in load_insn_p().

gas/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (load_insn_p): Use PREFIX_NONE. Fold two
	if()-s.
	(match_template, output_insn): Use PREFIX_NONE.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -4446,10 +4446,11 @@ load_insn_p (void)
 	  && i.tm.extension_opcode != 6)
 	return 1;
 
-      /* cmpxchg8b, cmpxchg16b, xrstors.  */
+      /* cmpxchg8b, cmpxchg16b, xrstors, vmptrld.  */
       if (i.tm.base_opcode == 0xfc7
-	  && i.tm.opcode_modifier.opcodeprefix == 0
-	  && (i.tm.extension_opcode == 1 || i.tm.extension_opcode == 3))
+	  && i.tm.opcode_modifier.opcodeprefix == PREFIX_NONE
+	  && (i.tm.extension_opcode == 1 || i.tm.extension_opcode == 3
+	      || i.tm.extension_opcode == 6))
 	return 1;
 
       /* fxrstor, ldmxcsr, xrstor.  */
@@ -4466,12 +4467,6 @@ load_insn_p (void)
 	      || i.tm.extension_opcode == 6))
 	return 1;
 
-      /* vmptrld */
-      if (i.tm.base_opcode == 0xfc7
-	  && i.tm.opcode_modifier.opcodeprefix == 0
-	  && i.tm.extension_opcode == 6)
-	return 1;
-
       /* Check for x87 instructions.  */
       if (i.tm.base_opcode >= 0xd8 && i.tm.base_opcode <= 0xdf)
 	{
@@ -6337,7 +6332,7 @@ match_template (char mnem_suffix)
       if (((i.suffix == QWORD_MNEM_SUFFIX
 	    && flag_code != CODE_64BIT
 	    && !(t->base_opcode == 0xfc7
-		 && i.tm.opcode_modifier.opcodeprefix == 0
+		 && i.tm.opcode_modifier.opcodeprefix == PREFIX_NONE
 		 && t->extension_opcode == 1) /* cmpxchg8b */)
 	   || (i.suffix == LONG_MNEM_SUFFIX
 	       && !cpu_arch_flags.bitfield.cpui386))
@@ -9259,7 +9254,7 @@ output_insn (void)
 	  || i.tm.cpu_flags.bitfield.cpucmov
 	  || i.tm.cpu_flags.bitfield.cpusyscall
 	  || (i.tm.base_opcode == 0xfc7
-	      && i.tm.opcode_modifier.opcodeprefix == 0
+	      && i.tm.opcode_modifier.opcodeprefix == PREFIX_NONE
 	      && i.tm.extension_opcode == 1) /* cmpxchg8b */)
 	x86_isa_1_used |= GNU_PROPERTY_X86_ISA_1_BASELINE;
       if (i.tm.cpu_flags.bitfield.cpusse3


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes
  2021-03-22 16:40 [PATCH 0/8] x86: work towards further opcode table compaction Jan Beulich
  2021-03-22 16:42 ` [PATCH 1/8] x86: unbreak certain MPX insn operand forms Jan Beulich
  2021-03-22 16:42 ` [PATCH 2/8] x86: don't open-code PREFIX_NONE Jan Beulich
@ 2021-03-22 16:43 ` Jan Beulich
  2021-03-22 17:55   ` H.J. Lu
  2021-03-22 16:44 ` [PATCH 5/8] x86: re-order two fields of struct insn_template Jan Beulich
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-22 16:43 UTC (permalink / raw)
  To: Binutils

This is in preparation of opcode_length going away as a field in the
templates. Identify pseudo prefixes by a base opcode of zero instead:
No real prefix has an opcode of zero. This at the same time allow
dropping a curious special case from i386-gen.

gas/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (parse_insn): Recognize pseudo prefixes by
	base_opcode and extension_opcode.

opcodes/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* i386-gen.c (process_i386_opcode_modifier): Drop IsPrefix
	check.
	* i386-opc.h (Prefix_*): Move #define-s.
	* i386-opc.tbl: Move pseudo prefix enumerator values to
	extension opcode field.
	* i386-tbl.h: Re-generate.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5123,10 +5123,11 @@ parse_insn (char *line, char *mnemonic)
 		      current_templates->start->name);
 	      return NULL;
 	    }
-	  if (current_templates->start->opcode_length == 0)
+
+	  if (current_templates->start->base_opcode == 0)
 	    {
 	      /* Handle pseudo prefixes.  */
-	      switch (current_templates->start->base_opcode)
+	      switch (current_templates->start->extension_opcode)
 		{
 		case Prefix_Disp8:
 		  /* {disp8} */
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1208,8 +1208,7 @@ process_i386_opcode_modifier (FILE *tabl
 		       || strncasecmp(str, "EVex=", 5) == 0
 		       || strncasecmp(str, "Disp8MemShift=", 14) == 0
 		       || strncasecmp(str, "Masking=", 8) == 0
-		       || strcasecmp(str, "SAE") == 0
-		       || strcasecmp(str, "IsPrefix") == 0)
+		       || strcasecmp(str, "SAE") == 0)
 		regular_encoding = 0;
 
 	      set_bitfield (str, modifiers, val, ARRAY_SIZE (modifiers),
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -920,6 +920,14 @@ typedef struct insn_template
 #define Opcode_SIMD_FloatD 0x1 /* Direction bit for SIMD fp insns. */
 #define Opcode_SIMD_IntD 0x10 /* Direction bit for SIMD int insns. */
 
+  /* extension_opcode is the 3 bit extension for group <n> insns.
+     This field is also used to store the 8-bit opcode suffix for the
+     AMD 3DNow! instructions.
+     If this template has no extension opcode (the usual case) use None
+     Instructions */
+  unsigned short extension_opcode;
+#define None 0xffff		/* If no extension_opcode is possible.  */
+
 /* Pseudo prefixes.  */
 #define Prefix_Disp8		0	/* {disp8} */
 #define Prefix_Disp16		1	/* {disp16} */
@@ -932,14 +940,6 @@ typedef struct insn_template
 #define Prefix_REX		8	/* {rex} */
 #define Prefix_NoOptimize	9	/* {nooptimize} */
 
-  /* extension_opcode is the 3 bit extension for group <n> insns.
-     This field is also used to store the 8-bit opcode suffix for the
-     AMD 3DNow! instructions.
-     If this template has no extension opcode (the usual case) use None
-     Instructions */
-  unsigned short extension_opcode;
-#define None 0xffff		/* If no extension_opcode is possible.  */
-
   /* Opcode length.  */
   unsigned char opcode_length;
 
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -849,19 +849,19 @@ rex.wrb, 0x4d, None, 1, Cpu64, No_bSuf|N
 rex.wrx, 0x4e, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
 rex.wrxb, 0x4f, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
 
-// Pseudo prefixes (opcode_length == 0)
+// Pseudo prefixes (base_opcode == 0)
 
-{disp8}, Prefix_Disp8, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{disp16}, Prefix_Disp16, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{disp32}, Prefix_Disp32, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{load}, Prefix_Load, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{store}, Prefix_Store, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{vex}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{vex2}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{vex3}, Prefix_VEX3, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{evex}, Prefix_EVEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{rex}, Prefix_REX, None, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{nooptimize}, Prefix_NoOptimize, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{disp8}, 0, Prefix_Disp8, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{disp16}, 0, Prefix_Disp16, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{disp32}, 0, Prefix_Disp32, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{load}, 0, Prefix_Load, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{store}, 0, Prefix_Store, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{vex}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{vex2}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{vex3}, 0, Prefix_VEX3, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{evex}, 0, Prefix_EVEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{rex}, 0, Prefix_REX, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+{nooptimize}, 0, Prefix_NoOptimize, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
 
 // 486 extensions.
 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 5/8] x86: re-order two fields of struct insn_template
  2021-03-22 16:40 [PATCH 0/8] x86: work towards further opcode table compaction Jan Beulich
                   ` (2 preceding siblings ...)
  2021-03-22 16:43 ` [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes Jan Beulich
@ 2021-03-22 16:44 ` Jan Beulich
  2021-03-22 18:03   ` H.J. Lu
  2021-03-22 16:45 ` [PATCH 6/8] x86: re-number PREFIX_0X<nn> Jan Beulich
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-22 16:44 UTC (permalink / raw)
  To: Binutils

To facilitate a subsequent table parser change, re-order CPU flags and
opcode modifier fields. No functional change intended.

gas/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (output_i386_opcode): Invoke
	process_i386_cpu_flag() slightly later.
	(process_i386_opcodes): Likewise.

opcodes/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* i386-opc.h (struct insn_template): Move cpu_flags field past
	opcode_modifier one.
	* i386-tbl.h: Re-generate.

--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1410,8 +1410,6 @@ output_i386_opcode (FILE *table, const c
   fprintf (table, "  { \"%s\", %s, %s, %s, %u,\n",
 	   name, base_opcode, extension_opcode, opcode_length, i);
 
-  process_i386_cpu_flag (table, cpu_flags, 0, ",", "    ", lineno);
-
   if (process_i386_opcode_modifier (table, opcode_modifier,
 				    operand_types, lineno))
     {
@@ -1449,6 +1447,8 @@ output_i386_opcode (FILE *table, const c
 	}
     }
 
+  process_i386_cpu_flag (table, cpu_flags, 0, ",", "    ", lineno);
+
   fprintf (table, "    { ");
 
   for (i = 0; i < ARRAY_SIZE (operand_types); i++)
@@ -1836,10 +1836,10 @@ process_i386_opcodes (FILE *table)
 
   fprintf (table, "  { NULL, 0, 0, 0, 0,\n");
 
-  process_i386_cpu_flag (table, "0", 0, ",", "    ", -1);
-
   process_i386_opcode_modifier (table, "0", NULL, -1);
 
+  process_i386_cpu_flag (table, "0", 0, ",", "    ", -1);
+
   fprintf (table, "    { ");
   process_i386_operand_type (table, "0", stage_opcodes, "\t  ", -1);
   fprintf (table, " } }\n");
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -951,14 +951,14 @@ typedef struct insn_template
   /* how many operands */
   unsigned char operands;
 
-  /* cpu feature flags */
-  i386_cpu_flags cpu_flags;
-
   /* the bits in opcode_modifier are used to generate the final opcode from
      the base_opcode.  These bits also are used to detect alternate forms of
      the same instruction */
   i386_opcode_modifier opcode_modifier;
 
+  /* cpu feature flags */
+  i386_cpu_flags cpu_flags;
+
   /* operand_types[i] describes the type of operand i.  This is made
      by OR'ing together all of the possible type masks.  (e.g.
      'operand_types[i] = Reg|Imm' specifies that operand i can be


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 6/8] x86: re-number PREFIX_0X<nn>
  2021-03-22 16:40 [PATCH 0/8] x86: work towards further opcode table compaction Jan Beulich
                   ` (3 preceding siblings ...)
  2021-03-22 16:44 ` [PATCH 5/8] x86: re-order two fields of struct insn_template Jan Beulich
@ 2021-03-22 16:45 ` Jan Beulich
  2021-03-22 18:03   ` H.J. Lu
  2021-03-22 16:46 ` [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode Jan Beulich
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-22 16:45 UTC (permalink / raw)
  To: Binutils

In preparation to use PREFIX_0X<nn> attributes also in VEX/XOP/EVEX
encoding templates, renumber the pseudo-enumerators such that their
values can then also be used directly in the respective prefix bit
fields.

gas/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (pte): Re-order opc_pfx[] entries.

opcodes/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* i386-opc.h (PREFIX_0XF2, PREFIX_0XF3): Excahnge values. Extend
	comment.
	* i386-tbl.h: Re-generate.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -3239,7 +3239,7 @@ pi (const char *line, i386_insn *x)
 static void
 pte (insn_template *t)
 {
-  static const unsigned char opc_pfx[] = { 0, 0x66, 0xf2, 0xf3 };
+  static const unsigned char opc_pfx[] = { 0, 0x66, 0xf3, 0xf2 };
   static const char *const opc_spc[] = {
     NULL, "0f", "0f38", "0f3a", NULL, NULL, NULL, NULL,
     "XOP08", "XOP09", "XOP0A",
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -593,16 +593,17 @@ enum
 #define SPACE_XOP09	9
 #define SPACE_XOP0A	0xA
   OpcodeSpace,
-  /* Opcode prefix:
+  /* Opcode prefix (values chosen to be usable directly in
+     VEX/XOP/EVEX pp fields):
      0: None
      1: Add 0x66 opcode prefix.
-     2: Add 0xf2 opcode prefix.
-     3: Add 0xf3 opcode prefix.
+     2: Add 0xf3 opcode prefix.
+     3: Add 0xf2 opcode prefix.
    */
 #define PREFIX_NONE	0
 #define PREFIX_0X66	1
-#define PREFIX_0XF2	2
-#define PREFIX_0XF3	3
+#define PREFIX_0XF3	2
+#define PREFIX_0XF2	3
   OpcodePrefix,
   /* number of VEX source operands:
      0: <= 2 source operands.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode
  2021-03-22 16:40 [PATCH 0/8] x86: work towards further opcode table compaction Jan Beulich
                   ` (4 preceding siblings ...)
  2021-03-22 16:45 ` [PATCH 6/8] x86: re-number PREFIX_0X<nn> Jan Beulich
@ 2021-03-22 16:46 ` Jan Beulich
  2021-03-22 18:03   ` H.J. Lu
       [not found] ` <65b17a6a-cc6a-a706-5e95-a7284c45beb1@suse.com>
       [not found] ` <f116024a-077d-c64e-f37c-1da8299272f0@suse.com>
  7 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-22 16:46 UTC (permalink / raw)
  To: Binutils

Just like is already done for legacy encoded insns, record the mandatory
prefix information in the respective opcode modifier field. Do this
without changing the source table, but rather by deriving the values from
their existing source representation.

gas/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (md_begin): Add assertion.
	(build_vex_prefix): Drop implied prefix calculation.
	(build_evex_prefix): Likewise.
	(optimize_encoding): Adjust opcode checks.
	(load_insn_p): Also check opcodeprefix.
	(match_template): Also check opcodespace.
	(process_suffix): Likewise.
	(process_operands): Likewise.
	(output_insn): Likewise. Also check isprefix when discaring
	standalone LOCK.
	* config/tc-i386-intel.c (i386_intel_operand): Also check
	opcodespace.

opcodes/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* i386-gen.c (process_i386_opcode_modifier): Return void. New
	parameter "prefix". Drop local variable "regular_encoding".
	Record prefix setting / check for consistency.
	(output_i386_opcode): Parse opcode_length and base_opcode
	earlier. Derive prefix encoding. Drop no longer applicable
	consistency checking. Adjust process_i386_opcode_modifier()
	invocation.
	(process_i386_opcodes): Adjust process_i386_opcode_modifier()
	invocation.
	* i386-tbl.h: Re-generate.
---
Things are being done this way because I generally think we may want to
revert that aspect of the earlier change introducing the opcode modifier
representation. This is because I believe the opcode table source is
easier to read and maintain that way. I'm similarly considering to put
the encoding space into the base opcode as well for VEX/XOP/EVEX
templates, such that the templates overall (and the SSE2AVX ones in
particular) would become more similar with one another.

--- a/gas/config/tc-i386-intel.c
+++ b/gas/config/tc-i386-intel.c
@@ -642,7 +642,8 @@ i386_intel_operand (char *operand_string
     return 0;
 
   if (intel_state.op_modifier != O_absent
-      && current_templates->start->base_opcode != 0x8d /* lea */)
+      && (current_templates->start->opcode_modifier.opcodespace != SPACE_BASE
+          || current_templates->start->base_opcode != 0x8d /* lea */))
     {
       i.types[this_operand].bitfield.unspecified = 0;
 
@@ -666,7 +667,8 @@ i386_intel_operand (char *operand_string
 	  if ((current_templates->start->name[0] == 'l'
 	       && current_templates->start->name[2] == 's'
 	       && current_templates->start->name[3] == 0)
-	      || current_templates->start->base_opcode == 0x62 /* bound */)
+	      || (current_templates->start->opcode_modifier.opcodespace == SPACE_BASE
+		  && current_templates->start->base_opcode == 0x62 /* bound */))
 	    suffix = WORD_MNEM_SUFFIX;
 	  else if (flag_code != CODE_32BIT
 		   && (current_templates->start->opcode_modifier.jump == JUMP
@@ -696,7 +698,8 @@ i386_intel_operand (char *operand_string
 
 	case O_qword_ptr: /* O_mmword_ptr */
 	  i.types[this_operand].bitfield.qword = 1;
-	  if (current_templates->start->base_opcode == 0x62 /* bound */
+	  if ((current_templates->start->opcode_modifier.opcodespace == SPACE_BASE
+	       && current_templates->start->base_opcode == 0x62 /* bound */)
 	      || got_a_float == 1)	/* "f..." */
 	    suffix = LONG_MNEM_SUFFIX;
 	  else
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -3065,6 +3065,8 @@ md_begin (void)
 
     while (1)
       {
+	gas_assert (optab->opcode_length == 4
+		    || !(optab->base_opcode >> (8 * optab->opcode_length)));
 	++optab;
 	if (optab->name == NULL
 	    || strcmp (optab->name, (optab - 1)->name) != 0)
@@ -3588,7 +3590,6 @@ static void
 build_vex_prefix (const insn_template *t)
 {
   unsigned int register_specifier;
-  unsigned int implied_prefix;
   unsigned int vector_length;
   unsigned int w;
 
@@ -3696,24 +3697,6 @@ build_vex_prefix (const insn_template *t
 	  }
     }
 
-  switch ((i.tm.base_opcode >> (i.tm.opcode_length << 3)) & 0xff)
-    {
-    case 0:
-      implied_prefix = 0;
-      break;
-    case DATA_PREFIX_OPCODE:
-      implied_prefix = 1;
-      break;
-    case REPE_PREFIX_OPCODE:
-      implied_prefix = 2;
-      break;
-    case REPNE_PREFIX_OPCODE:
-      implied_prefix = 3;
-      break;
-    default:
-      abort ();
-    }
-
   /* Check the REX.W bit and VEXW.  */
   if (i.tm.opcode_modifier.vexw == VEXWIG)
     w = (vexwig == vexw1 || (i.rex & REX_W)) ? 1 : 0;
@@ -3739,7 +3722,7 @@ build_vex_prefix (const insn_template *t
       i.vex.bytes[1] = (r << 7
 			| register_specifier << 3
 			| vector_length << 2
-			| implied_prefix);
+			| i.tm.opcode_modifier.opcodeprefix);
     }
   else
     {
@@ -3769,7 +3752,7 @@ build_vex_prefix (const insn_template *t
       i.vex.bytes[2] = (w << 7
 			| register_specifier << 3
 			| vector_length << 2
-			| implied_prefix);
+			| i.tm.opcode_modifier.opcodeprefix);
     }
 }
 
@@ -3792,8 +3775,7 @@ is_any_vex_encoding (const insn_template
 static void
 build_evex_prefix (void)
 {
-  unsigned int register_specifier;
-  unsigned int implied_prefix, w;
+  unsigned int register_specifier, w;
   rex_byte vrex_used = 0;
 
   /* Check register specifier.  */
@@ -3822,24 +3804,6 @@ build_evex_prefix (void)
 	vrex_used |= REX_X;
     }
 
-  switch ((i.tm.base_opcode >> 8) & 0xff)
-    {
-    case 0:
-      implied_prefix = 0;
-      break;
-    case DATA_PREFIX_OPCODE:
-      implied_prefix = 1;
-      break;
-    case REPE_PREFIX_OPCODE:
-      implied_prefix = 2;
-      break;
-    case REPNE_PREFIX_OPCODE:
-      implied_prefix = 3;
-      break;
-    default:
-      abort ();
-    }
-
   /* 4 byte EVEX prefix.  */
   i.vex.length = 4;
   i.vex.bytes[0] = 0x62;
@@ -3882,11 +3846,11 @@ build_evex_prefix (void)
   else
     w = (flag_code == CODE_64BIT ? i.rex & REX_W : evexwig == evexw1) ? 1 : 0;
 
-  /* Encode the U bit.  */
-  implied_prefix |= 0x4;
-
   /* The third byte of the EVEX prefix.  */
-  i.vex.bytes[2] = (w << 7 | register_specifier << 3 | implied_prefix);
+  i.vex.bytes[2] = ((w << 7)
+		    | (register_specifier << 3)
+		    | 4 /* Encode the U bit.  */
+		    | i.tm.opcode_modifier.opcodeprefix);
 
   /* The fourth byte of the EVEX prefix.  */
   /* The zeroing-masking bit.  */
@@ -4192,19 +4156,15 @@ optimize_encoding (void)
 		       || (i.tm.operand_types[2].bitfield.zmmword
 			   && i.types[2].bitfield.ymmword))))
 	   && ((i.tm.base_opcode == 0x55
-		|| i.tm.base_opcode == 0x6655
-		|| i.tm.base_opcode == 0x66df
 		|| i.tm.base_opcode == 0x57
-		|| i.tm.base_opcode == 0x6657
-		|| i.tm.base_opcode == 0x66ef
-		|| i.tm.base_opcode == 0x66f8
-		|| i.tm.base_opcode == 0x66f9
-		|| i.tm.base_opcode == 0x66fa
-		|| i.tm.base_opcode == 0x66fb
+		|| i.tm.base_opcode == 0xdf
+		|| i.tm.base_opcode == 0xef
+		|| i.tm.base_opcode == 0xf8
+		|| i.tm.base_opcode == 0xf9
+		|| i.tm.base_opcode == 0xfa
+		|| i.tm.base_opcode == 0xfb
 		|| i.tm.base_opcode == 0x42
-		|| i.tm.base_opcode == 0x6642
-		|| i.tm.base_opcode == 0x47
-		|| i.tm.base_opcode == 0x6647)
+		|| i.tm.base_opcode == 0x47)
 	       && i.tm.extension_opcode == None))
     {
       /* Optimize: -O1:
@@ -4257,7 +4217,7 @@ optimize_encoding (void)
 	}
       else if (i.tm.operand_types[0].bitfield.class == RegMask)
 	{
-	  i.tm.base_opcode &= 0xff;
+	  i.tm.opcode_modifier.opcodeprefix = PREFIX_NONE;
 	  i.tm.opcode_modifier.vexw = VEXW0;
 	}
       else
@@ -4276,11 +4236,9 @@ optimize_encoding (void)
 	   && !i.mask
 	   && !i.broadcast
 	   && is_evex_encoding (&i.tm)
-	   && ((i.tm.base_opcode & ~Opcode_SIMD_IntD) == 0x666f
-	       || (i.tm.base_opcode & ~Opcode_SIMD_IntD) == 0xf36f
-	       || (i.tm.base_opcode & ~Opcode_SIMD_IntD) == 0xf26f
-	       || (i.tm.base_opcode & ~4) == 0x66db
-	       || (i.tm.base_opcode & ~4) == 0x66eb)
+	   && ((i.tm.base_opcode & ~Opcode_SIMD_IntD) == 0x6f
+	       || (i.tm.base_opcode & ~4) == 0xdb
+	       || (i.tm.base_opcode & ~4) == 0xeb)
 	   && i.tm.extension_opcode == None)
     {
       /* Optimize: -O1:
@@ -4331,13 +4289,14 @@ optimize_encoding (void)
 	    i.types[j].bitfield.disp8 = vex_disp8;
 	    break;
 	  }
-      if ((i.tm.base_opcode & ~Opcode_SIMD_IntD) == 0xf26f)
-	i.tm.base_opcode ^= 0xf36f ^ 0xf26f;
+      if ((i.tm.base_opcode & ~Opcode_SIMD_IntD) == 0x6f
+	  && i.tm.opcode_modifier.opcodeprefix == PREFIX_0XF2)
+	i.tm.opcode_modifier.opcodeprefix = PREFIX_0XF3;
       i.tm.opcode_modifier.vex
 	= i.types[0].bitfield.ymmword ? VEX256 : VEX128;
       i.tm.opcode_modifier.vexw = VEXW0;
       /* VPAND, VPOR, and VPXOR are commutative.  */
-      if (i.reg_operands == 3 && i.tm.base_opcode != 0x66df)
+      if (i.reg_operands == 3 && i.tm.base_opcode != 0xdf)
 	i.tm.opcode_modifier.commutative = 1;
       i.tm.opcode_modifier.evex = 0;
       i.tm.opcode_modifier.masking = 0;
@@ -4395,6 +4354,7 @@ load_insn_p (void)
       if (i.tm.base_opcode == 0xae
 	  && i.tm.opcode_modifier.vex
 	  && i.tm.opcode_modifier.opcodespace == SPACE_0F
+	  && i.tm.opcode_modifier.opcodeprefix == PREFIX_NONE
 	  && i.tm.extension_opcode == 2)
 	return 1;
     }
@@ -6388,7 +6348,9 @@ match_template (char mnem_suffix)
 	}
 
       /* Force 0x8b encoding for "mov foo@GOT, %eax".  */
-      if (i.reloc[0] == BFD_RELOC_386_GOT32 && t->base_opcode == 0xa0)
+      if (i.reloc[0] == BFD_RELOC_386_GOT32
+	  && t->base_opcode == 0xa0
+	  && t->opcode_modifier.opcodespace == SPACE_BASE)
 	continue;
 
       /* We check register size if needed.  */
@@ -6415,6 +6377,7 @@ match_template (char mnem_suffix)
 	     zero-extend %eax to %rax.  */
 	  if (flag_code == CODE_64BIT
 	      && t->base_opcode == 0x90
+	      && t->opcode_modifier.opcodespace == SPACE_BASE
 	      && i.types[0].bitfield.instance == Accum
 	      && i.types[0].bitfield.dword
 	      && i.types[1].bitfield.instance == Accum
@@ -6425,6 +6388,7 @@ match_template (char mnem_suffix)
 	  if (flag_code != CODE_64BIT
 	      && i.hle_prefix
 	      && t->base_opcode == 0xa0
+	      && t->opcode_modifier.opcodespace == SPACE_BASE
 	      && i.types[0].bitfield.instance == Accum
 	      && (i.flags[1] & Operand_Mem))
 	    continue;
@@ -6780,7 +6744,9 @@ process_suffix (void)
 	 ambiguity checking below.  The suffix will be replaced afterwards
 	 to represent the destination (register).  */
       if (((i.tm.base_opcode | 8) == 0xfbe && i.tm.opcode_modifier.w)
-	  || (i.tm.base_opcode == 0x63 && i.tm.cpu_flags.bitfield.cpu64))
+	  || (i.tm.base_opcode == 0x63
+	      && i.tm.opcode_modifier.opcodespace == SPACE_BASE
+	      && i.tm.cpu_flags.bitfield.cpu64))
 	--i.operands;
 
       /* crc32 needs REX.W set regardless of suffix / source operand size.  */
@@ -7028,6 +6994,7 @@ process_suffix (void)
 	    i.suffix = SHORT_MNEM_SUFFIX;
 	  else if ((i.tm.base_opcode | 8) == 0xfbe
 		   || (i.tm.base_opcode == 0x63
+		       && i.tm.opcode_modifier.opcodespace == SPACE_BASE
 		       && i.tm.cpu_flags.bitfield.cpu64))
 	    /* handled below */;
 	  else if (evex)
@@ -7042,7 +7009,9 @@ process_suffix (void)
     }
 
   if ((i.tm.base_opcode | 8) == 0xfbe
-      || (i.tm.base_opcode == 0x63 && i.tm.cpu_flags.bitfield.cpu64))
+      || (i.tm.base_opcode == 0x63
+	  && i.tm.opcode_modifier.opcodespace == SPACE_BASE
+	  && i.tm.cpu_flags.bitfield.cpu64))
     {
       /* In Intel syntax, movsx/movzx must have a "suffix" (checked above).
 	 In AT&T syntax, if there is no suffix (warned about above), the default
@@ -7741,6 +7710,7 @@ process_operands (void)
 
   if ((i.seg[0] || i.prefix[SEG_PREFIX])
       && i.tm.base_opcode == 0x8d /* lea */
+      && i.tm.opcode_modifier.opcodespace == SPACE_BASE
       && !is_any_vex_encoding(&i.tm))
     {
       if (!quiet_warnings)
@@ -9244,7 +9214,8 @@ output_insn (void)
 	  || i.tm.cpu_flags.bitfield.cpupopcnt
 	  /* LAHF-SAHF insns in 64-bit mode.  */
 	  || (flag_code == CODE_64BIT
-	      && (i.tm.base_opcode | 1) == 0x9f))
+	      && (i.tm.base_opcode | 1) == 0x9f
+	      && i.tm.opcode_modifier.opcodespace == SPACE_BASE))
 	x86_isa_1_used |= GNU_PROPERTY_X86_ISA_1_V2;
       if (i.tm.cpu_flags.bitfield.cpuavx
 	  || i.tm.cpu_flags.bitfield.cpuavx2
@@ -9354,7 +9325,8 @@ output_insn (void)
 	 assembler ignore LOCK prefix and serves as a workaround.  */
       if (omit_lock_prefix)
 	{
-	  if (i.tm.base_opcode == LOCK_PREFIX_OPCODE)
+	  if (i.tm.base_opcode == LOCK_PREFIX_OPCODE
+	      && i.tm.opcode_modifier.isprefix)
 	    return;
 	  i.prefix[LOCK_PREFIX] = 0;
 	}
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1175,12 +1175,12 @@ adjust_broadcast_modifier (char **opnd)
   return bcst_type;
 }
 
-static int
-process_i386_opcode_modifier (FILE *table, char *mod, char **opnd, int lineno)
+static void
+process_i386_opcode_modifier (FILE *table, char *mod, unsigned int prefix,
+			      char **opnd, int lineno)
 {
   char *str, *next, *last;
   bitfield modifiers [ARRAY_SIZE (opcode_modifiers)];
-  unsigned int regular_encoding = 1;
 
   active_isstring = 0;
 
@@ -1199,18 +1199,7 @@ process_i386_opcode_modifier (FILE *tabl
 	    {
 	      int val = 1;
 	      if (strcasecmp(str, "Broadcast") == 0)
-		{
-		  val = adjust_broadcast_modifier (opnd);
-		  regular_encoding = 0;
-		}
-	      else if (strcasecmp(str, "Vex") == 0
-		       || strncasecmp(str, "Vex=", 4) == 0
-		       || strcasecmp(str, "EVex") == 0
-		       || strncasecmp(str, "EVex=", 5) == 0
-		       || strncasecmp(str, "Disp8MemShift=", 14) == 0
-		       || strncasecmp(str, "Masking=", 8) == 0
-		       || strcasecmp(str, "SAE") == 0)
-		regular_encoding = 0;
+		val = adjust_broadcast_modifier (opnd);
 
 	      set_bitfield (str, modifiers, val, ARRAY_SIZE (modifiers),
 			    lineno);
@@ -1231,6 +1220,19 @@ process_i386_opcode_modifier (FILE *tabl
 	    }
 	}
 
+      if (prefix)
+	{
+	  if (!modifiers[OpcodePrefix].value)
+	    modifiers[OpcodePrefix].value = prefix;
+	  else if (modifiers[OpcodePrefix].value != prefix)
+	    fail (_("%s:%d: Conflicting prefix specifications\n"),
+		  filename, lineno);
+	  else
+	    fprintf (stderr,
+		     _("%s:%d: Warning: redundant prefix specification\n"),
+		     filename, lineno);
+	}
+
       if (have_w && !bwlq_suf)
 	fail ("%s: %d: stray W modifier\n", filename, lineno);
       if (have_w && !(bwlq_suf & 1))
@@ -1242,8 +1244,6 @@ process_i386_opcode_modifier (FILE *tabl
 		 filename, lineno);
     }
   output_opcode_modifier (table, modifiers, ARRAY_SIZE (modifiers));
-
-  return regular_encoding;
 }
 
 enum stage {
@@ -1355,9 +1355,10 @@ static void
 output_i386_opcode (FILE *table, const char *name, char *str,
 		    char *last, int lineno)
 {
-  unsigned int i;
-  char *base_opcode, *extension_opcode, *opcode_length;
+  unsigned int i, prefix = 0;
+  char *base_opcode, *extension_opcode, *opcode_length, *end;
   char *cpu_flags, *opcode_modifier, *operand_types [MAX_OPERANDS];
+  unsigned long int opcode, length;
 
   /* Find base_opcode.  */
   base_opcode = next_field (str, ',', &str, last);
@@ -1407,46 +1408,33 @@ output_i386_opcode (FILE *table, const c
 	}
     }
 
-  fprintf (table, "  { \"%s\", %s, %s, %s, %u,\n",
-	   name, base_opcode, extension_opcode, opcode_length, i);
+  length = strtoul (opcode_length, &end, 0);
+  opcode = strtoul (base_opcode, &end, 0);
 
-  if (process_i386_opcode_modifier (table, opcode_modifier,
-				    operand_types, lineno))
+  /* Transform prefixes encoded in the opcode into opcode modifier
+     representation.  */
+  if (length < 4)
     {
-      char *end;
-      unsigned long int length = strtoul (opcode_length, &end, 0);
-      unsigned long int opcode = strtoul (base_opcode, &end, 0);
-      switch (length)
+      switch (opcode >> (8 * length))
 	{
-	case 4:
-	  break;
-	case 3:
-	  if ((opcode >> 24) != 0)
-	    fail (_("%s: %s: (base_opcode >> 24) != 0: %s\n"),
-		  filename, name, base_opcode);
-	  break;
-	case 2:
-	  if ((opcode >> 16) != 0)
-	    fail (_("%s: %s: (base_opcode >> 16) != 0: %s\n"),
-		  filename, name, base_opcode);
-	  break;
-	case 1:
-	  if ((opcode >> 8) != 0)
-	    fail (_("%s: %s: (base_opcode >> 8) != 0: %s\n"),
-		  filename, name, base_opcode);
-	  break;
-	case 0:
-	  if (opcode != 0)
-	    fail (_("%s: %s: base_opcode != 0: %s\n"),
-		  filename, name, base_opcode);
-	  break;
+	case 0: break;
+	case 0x66: prefix = PREFIX_0X66; break;
+	case 0xF3: prefix = PREFIX_0XF3; break;
+	case 0xF2: prefix = PREFIX_0XF2; break;
 	default:
-	  fail (_("%s: %s: invalid opcode length: %s\n"),
-		filename, name, opcode_length);
-	  break;
+	  fail (_("%s:%d: %s: Unexpected opcode prefix %02lx\n"),
+		filename, lineno, name, opcode >> (8 * length));
 	}
+
+      opcode &= (1UL << (8 * length)) - 1;
     }
 
+  fprintf (table, "  { \"%s\", 0x%0*lx%s, %s, %lu, %u,\n",
+	   name, 2 * (int)length, opcode, end, extension_opcode, length, i);
+
+  process_i386_opcode_modifier (table, opcode_modifier, prefix,
+				operand_types, lineno);
+
   process_i386_cpu_flag (table, cpu_flags, 0, ",", "    ", lineno);
 
   fprintf (table, "    { ");
@@ -1836,7 +1824,7 @@ process_i386_opcodes (FILE *table)
 
   fprintf (table, "  { NULL, 0, 0, 0, 0,\n");
 
-  process_i386_opcode_modifier (table, "0", NULL, -1);
+  process_i386_opcode_modifier (table, "0", 0, NULL, -1);
 
   process_i386_cpu_flag (table, "0", 0, ",", "    ", -1);
 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes
  2021-03-22 16:43 ` [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes Jan Beulich
@ 2021-03-22 17:55   ` H.J. Lu
  2021-03-23  7:36     ` Jan Beulich
  0 siblings, 1 reply; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 17:55 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Mar 22, 2021 at 9:43 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> This is in preparation of opcode_length going away as a field in the
> templates. Identify pseudo prefixes by a base opcode of zero instead:
> No real prefix has an opcode of zero. This at the same time allow
> dropping a curious special case from i386-gen.
>
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (parse_insn): Recognize pseudo prefixes by
>         base_opcode and extension_opcode.
>
> opcodes/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * i386-gen.c (process_i386_opcode_modifier): Drop IsPrefix
>         check.
>         * i386-opc.h (Prefix_*): Move #define-s.
>         * i386-opc.tbl: Move pseudo prefix enumerator values to
>         extension opcode field.
>         * i386-tbl.h: Re-generate.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -5123,10 +5123,11 @@ parse_insn (char *line, char *mnemonic)
>                       current_templates->start->name);
>               return NULL;
>             }
> -         if (current_templates->start->opcode_length == 0)
> +
> +         if (current_templates->start->base_opcode == 0)
>             {
>               /* Handle pseudo prefixes.  */
> -             switch (current_templates->start->base_opcode)
> +             switch (current_templates->start->extension_opcode)
>                 {
>                 case Prefix_Disp8:
>                   /* {disp8} */
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -1208,8 +1208,7 @@ process_i386_opcode_modifier (FILE *tabl
>                        || strncasecmp(str, "EVex=", 5) == 0
>                        || strncasecmp(str, "Disp8MemShift=", 14) == 0
>                        || strncasecmp(str, "Masking=", 8) == 0
> -                      || strcasecmp(str, "SAE") == 0
> -                      || strcasecmp(str, "IsPrefix") == 0)
> +                      || strcasecmp(str, "SAE") == 0)
>                 regular_encoding = 0;
>
>               set_bitfield (str, modifiers, val, ARRAY_SIZE (modifiers),
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -920,6 +920,14 @@ typedef struct insn_template
>  #define Opcode_SIMD_FloatD 0x1 /* Direction bit for SIMD fp insns. */
>  #define Opcode_SIMD_IntD 0x10 /* Direction bit for SIMD int insns. */
>
> +  /* extension_opcode is the 3 bit extension for group <n> insns.
> +     This field is also used to store the 8-bit opcode suffix for the
> +     AMD 3DNow! instructions.
> +     If this template has no extension opcode (the usual case) use None
> +     Instructions */
> +  unsigned short extension_opcode;
> +#define None 0xffff            /* If no extension_opcode is possible.  */
> +
>  /* Pseudo prefixes.  */
>  #define Prefix_Disp8           0       /* {disp8} */
>  #define Prefix_Disp16          1       /* {disp16} */
> @@ -932,14 +940,6 @@ typedef struct insn_template
>  #define Prefix_REX             8       /* {rex} */
>  #define Prefix_NoOptimize      9       /* {nooptimize} */
>
> -  /* extension_opcode is the 3 bit extension for group <n> insns.
> -     This field is also used to store the 8-bit opcode suffix for the
> -     AMD 3DNow! instructions.
> -     If this template has no extension opcode (the usual case) use None
> -     Instructions */
> -  unsigned short extension_opcode;
> -#define None 0xffff            /* If no extension_opcode is possible.  */
> -
>    /* Opcode length.  */
>    unsigned char opcode_length;
>
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -849,19 +849,19 @@ rex.wrb, 0x4d, None, 1, Cpu64, No_bSuf|N
>  rex.wrx, 0x4e, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>  rex.wrxb, 0x4f, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>
> -// Pseudo prefixes (opcode_length == 0)
> +// Pseudo prefixes (base_opcode == 0)
>
> -{disp8}, Prefix_Disp8, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{disp16}, Prefix_Disp16, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{disp32}, Prefix_Disp32, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{load}, Prefix_Load, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{store}, Prefix_Store, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{vex}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{vex2}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{vex3}, Prefix_VEX3, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{evex}, Prefix_EVEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{rex}, Prefix_REX, None, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> -{nooptimize}, Prefix_NoOptimize, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{disp8}, 0, Prefix_Disp8, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{disp16}, 0, Prefix_Disp16, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{disp32}, 0, Prefix_Disp32, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{load}, 0, Prefix_Load, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{store}, 0, Prefix_Store, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{vex}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{vex2}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{vex3}, 0, Prefix_VEX3, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{evex}, 0, Prefix_EVEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{rex}, 0, Prefix_REX, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> +{nooptimize}, 0, Prefix_NoOptimize, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}

Please define an opcode macro for pseudo prefix, something like

#define PSEUDO_PREFIX 0

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/8] x86: unbreak certain MPX insn operand forms
  2021-03-22 16:42 ` [PATCH 1/8] x86: unbreak certain MPX insn operand forms Jan Beulich
@ 2021-03-22 17:58   ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 17:58 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Mar 22, 2021 at 9:42 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Commit 8b65b8953af2 ("x86: Remove the prefix byte from non-VEX/EVEX
> base_opcode") dropped the mandatory prefix bytes from legacy encoded
> insn templates, but failed to also adjust affected MPX-specific checks
> in two places.
>
> For the expressions to remain halfway readable, introduce local
> variables to hold current_templates->start.
>
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (i386_index_check): New local variable t.
>         Correct MPX insn check.
>         * config/tc-i386-intel.c (i386_intel_simplify_register): Correct
>         MPX insn check.
>         * testsuite/gas/i386/x86-64-mpx.s: Add RIP-relative cases. Test
>         index scaling by other than 1.
>         * testsuite/gas/i386/x86-64-mpx.d: Adjust expectations.
>

OK.  Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/8] x86: don't open-code PREFIX_NONE
  2021-03-22 16:42 ` [PATCH 2/8] x86: don't open-code PREFIX_NONE Jan Beulich
@ 2021-03-22 17:58   ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 17:58 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Mar 22, 2021 at 9:42 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Use the constant rather than literal zero. While at it fold two
> conditionals (using the same base opcode and prefix) in load_insn_p().
>
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (load_insn_p): Use PREFIX_NONE. Fold two
>         if()-s.
>         (match_template, output_insn): Use PREFIX_NONE.

OK.  Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 8/8] x86: derive opcode length from opcode value
       [not found] ` <65b17a6a-cc6a-a706-5e95-a7284c45beb1@suse.com>
@ 2021-03-22 18:02   ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 18:02 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Mar 22, 2021 at 05:47:11PM +0100, Jan Beulich wrote:
> In the majority of cases we can easily determine the length from the
> encoding, irrespective of whether a prefix is specified there as well.
> We further don't even need to record the value in the table entries, as
> it's easy enough to determine it (without any guesswork, unless an insn
> with major opcode 00 appeared that requires a 2nd opcode byte to be
> specified explicitly) when installing the chosen template for further
> processing.
> 
> Should an encoding appear which
> - has a major opcode byte of 66, F3, or F2,
> - requires a 2nd opcode byte to be specified explicitly,
> - doesn't have a mandatory prefix
> we'd need to convert all templates presently encoding a mandatory prefix
> this way to the Prefix_0X<nn> model to eliminate the respective guessing
> i386-gen does.
> 
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* config/tc-i386.c (struct _i386_insn): New field
> 	opcode_length.
> 	(md_begin): Drop assertion.
> 	(install_template): New.
> 	(build_vex_prefix): Call install_template.
> 	(match_template): Likewise.
> 	(process_operands): Use new opcode_length field.
> 	(output_jump): Likewise.
> 	(output_insn): Likewise. Adjust psedo prefix check.
> 
> opcodes/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* i386-gen.c (output_i386_opcode): Drop processing of
> 	opcode_length. Calculate length from base_opcode. Adjust prefix
> 	encoding determination.
> 	(process_i386_opcodes): Drop output of fake opcode_length.
> 	* i386-opc.h (struct insn_template): Drop opcode_length field.
> 	* i386-opc.tbl: Drop opcode length field from all templates.
> 	* i386-tbl.h: Re-generate.
> 

OK.  Thanks.

H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode
  2021-03-22 16:46 ` [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode Jan Beulich
@ 2021-03-22 18:03   ` H.J. Lu
  2021-03-23 16:36     ` Jan Beulich
  0 siblings, 1 reply; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 18:03 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils, H.J. Lu

On Mon, Mar 22, 2021 at 05:46:14PM +0100, Jan Beulich wrote:
> Just like is already done for legacy encoded insns, record the mandatory
> prefix information in the respective opcode modifier field. Do this
> without changing the source table, but rather by deriving the values from
> their existing source representation.
> 
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* config/tc-i386.c (md_begin): Add assertion.
> 	(build_vex_prefix): Drop implied prefix calculation.
> 	(build_evex_prefix): Likewise.
> 	(optimize_encoding): Adjust opcode checks.
> 	(load_insn_p): Also check opcodeprefix.
> 	(match_template): Also check opcodespace.
> 	(process_suffix): Likewise.
> 	(process_operands): Likewise.
> 	(output_insn): Likewise. Also check isprefix when discaring
> 	standalone LOCK.
> 	* config/tc-i386-intel.c (i386_intel_operand): Also check
> 	opcodespace.
> 
> opcodes/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* i386-gen.c (process_i386_opcode_modifier): Return void. New
> 	parameter "prefix". Drop local variable "regular_encoding".
> 	Record prefix setting / check for consistency.
> 	(output_i386_opcode): Parse opcode_length and base_opcode
> 	earlier. Derive prefix encoding. Drop no longer applicable
> 	consistency checking. Adjust process_i386_opcode_modifier()
> 	invocation.
> 	(process_i386_opcodes): Adjust process_i386_opcode_modifier()
> 	invocation.
> 	* i386-tbl.h: Re-generate.

OK.  Thanks.

H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/8] x86: re-number PREFIX_0X<nn>
  2021-03-22 16:45 ` [PATCH 6/8] x86: re-number PREFIX_0X<nn> Jan Beulich
@ 2021-03-22 18:03   ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 18:03 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils, H.J. Lu

On Mon, Mar 22, 2021 at 05:45:30PM +0100, Jan Beulich wrote:
> In preparation to use PREFIX_0X<nn> attributes also in VEX/XOP/EVEX
> encoding templates, renumber the pseudo-enumerators such that their
> values can then also be used directly in the respective prefix bit
> fields.
> 
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* config/tc-i386.c (pte): Re-order opc_pfx[] entries.
> 
> opcodes/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* i386-opc.h (PREFIX_0XF2, PREFIX_0XF3): Excahnge values. Extend
> 	comment.
> 	* i386-tbl.h: Re-generate.
> 

OK.  Thanks.

H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 5/8] x86: re-order two fields of struct insn_template
  2021-03-22 16:44 ` [PATCH 5/8] x86: re-order two fields of struct insn_template Jan Beulich
@ 2021-03-22 18:03   ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 18:03 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils, H.J. Lu

On Mon, Mar 22, 2021 at 05:44:55PM +0100, Jan Beulich wrote:
> To facilitate a subsequent table parser change, re-order CPU flags and
> opcode modifier fields. No functional change intended.
> 
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* config/tc-i386.c (output_i386_opcode): Invoke
> 	process_i386_cpu_flag() slightly later.
> 	(process_i386_opcodes): Likewise.
> 
> opcodes/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* i386-opc.h (struct insn_template): Move cpu_flags field past
> 	opcode_modifier one.
> 	* i386-tbl.h: Re-generate.
> 

OK.  Thanks.

H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/8] x86: split opcode prefix and opcode space representation
       [not found] ` <f116024a-077d-c64e-f37c-1da8299272f0@suse.com>
@ 2021-03-22 18:04   ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-22 18:04 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils, H.J. Lu

On Mon, Mar 22, 2021 at 05:44:19PM +0100, Jan Beulich wrote:
> Commit 8b65b8953af2 ("x86: Remove the prefix byte from non-VEX/EVEX
> base_opcode") used the opcodeprefix field for two distinct purposes. In
> preparation of having VEX/XOP/EVEX and non-VEX templates become similar
> in the representatioon of both encoding space and opcode prefixes, split
> the field to have a separate one holding an insn's opcode space.
> 
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* config/tc-i386.c (pte): Print prefix and encoding space.
> 	(build_vex_prefix): Check opcodespace instead of opcodeprefix.
> 	(build_evex_prefix): Likewise.
> 	(load_insn_p): Likewise.
> 
> opcodes/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> 
> 	* i386-gen.c (opcode_modifiers): New OpcodeSpace element.
> 	* i386-opc.h (OpcodeSpace): New enumerator.
> 	(VEX0F, VEX0F38, VEX0F3A, XOP08, XOP09, XOP0A): Rename to ...
> 	(SPACE_BASE, SPACE_0F, SPACE_0F38, SPACE_0F3A, SPACE_XOP08,
> 	SPACE_XOP09, SPACE_XOP0A): ... respectively.
> 	(struct i386_opcode_modifier): New field opcodespace. Shrink
> 	opcodeprefix field.
> 	i386-opc.tbl (Space0F, Space0F38, Space0F3A, SpaceXOP08,
> 	SpaceXOP09, SpaceXOP0A): Define. Use them to replace
> 	OpcodePrefix uses.
> 	* i386-tbl.h: Re-generate.
> 

OK.  Thanks.

H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes
  2021-03-22 17:55   ` H.J. Lu
@ 2021-03-23  7:36     ` Jan Beulich
  2021-03-23 12:05       ` H.J. Lu
  0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-23  7:36 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 22.03.2021 18:55, H.J. Lu wrote:
> On Mon, Mar 22, 2021 at 9:43 AM Jan Beulich <jbeulich@suse.com> wrote:
>> --- a/opcodes/i386-opc.tbl
>> +++ b/opcodes/i386-opc.tbl
>> @@ -849,19 +849,19 @@ rex.wrb, 0x4d, None, 1, Cpu64, No_bSuf|N
>>  rex.wrx, 0x4e, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>>  rex.wrxb, 0x4f, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>>
>> -// Pseudo prefixes (opcode_length == 0)
>> +// Pseudo prefixes (base_opcode == 0)
>>
>> -{disp8}, Prefix_Disp8, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{disp16}, Prefix_Disp16, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{disp32}, Prefix_Disp32, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{load}, Prefix_Load, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{store}, Prefix_Store, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{vex}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{vex2}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{vex3}, Prefix_VEX3, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{evex}, Prefix_EVEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{rex}, Prefix_REX, None, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> -{nooptimize}, Prefix_NoOptimize, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{disp8}, 0, Prefix_Disp8, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{disp16}, 0, Prefix_Disp16, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{disp32}, 0, Prefix_Disp32, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{load}, 0, Prefix_Load, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{store}, 0, Prefix_Store, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{vex}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{vex2}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{vex3}, 0, Prefix_VEX3, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{evex}, 0, Prefix_EVEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{rex}, 0, Prefix_REX, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
>> +{nooptimize}, 0, Prefix_NoOptimize, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> 
> Please define an opcode macro for pseudo prefix, something like
> 
> #define PSEUDO_PREFIX 0

Sure. Seeing your request I'm actually considering to go further and
introduce a pseudo-prefix template. Would you be okay with this?

Jan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes
  2021-03-23  7:36     ` Jan Beulich
@ 2021-03-23 12:05       ` H.J. Lu
  2021-03-23 16:42         ` [PATCH v2] " Jan Beulich
  0 siblings, 1 reply; 23+ messages in thread
From: H.J. Lu @ 2021-03-23 12:05 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Tue, Mar 23, 2021 at 12:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.03.2021 18:55, H.J. Lu wrote:
> > On Mon, Mar 22, 2021 at 9:43 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> --- a/opcodes/i386-opc.tbl
> >> +++ b/opcodes/i386-opc.tbl
> >> @@ -849,19 +849,19 @@ rex.wrb, 0x4d, None, 1, Cpu64, No_bSuf|N
> >>  rex.wrx, 0x4e, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >>  rex.wrxb, 0x4f, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >>
> >> -// Pseudo prefixes (opcode_length == 0)
> >> +// Pseudo prefixes (base_opcode == 0)
> >>
> >> -{disp8}, Prefix_Disp8, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{disp16}, Prefix_Disp16, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{disp32}, Prefix_Disp32, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{load}, Prefix_Load, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{store}, Prefix_Store, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{vex}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{vex2}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{vex3}, Prefix_VEX3, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{evex}, Prefix_EVEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{rex}, Prefix_REX, None, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> -{nooptimize}, Prefix_NoOptimize, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{disp8}, 0, Prefix_Disp8, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{disp16}, 0, Prefix_Disp16, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{disp32}, 0, Prefix_Disp32, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{load}, 0, Prefix_Load, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{store}, 0, Prefix_Store, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{vex}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{vex2}, 0, Prefix_VEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{vex3}, 0, Prefix_VEX3, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{evex}, 0, Prefix_EVEX, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{rex}, 0, Prefix_REX, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >> +{nooptimize}, 0, Prefix_NoOptimize, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
> >
> > Please define an opcode macro for pseudo prefix, something like
> >
> > #define PSEUDO_PREFIX 0
>
> Sure. Seeing your request I'm actually considering to go further and
> introduce a pseudo-prefix template. Would you be okay with this?
>

Sounds good to me.  Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode
  2021-03-22 18:03   ` H.J. Lu
@ 2021-03-23 16:36     ` Jan Beulich
  2021-03-23 18:34       ` H.J. Lu
  0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-23 16:36 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 22.03.2021 19:03, H.J. Lu wrote:
> On Mon, Mar 22, 2021 at 05:46:14PM +0100, Jan Beulich wrote:
>> Just like is already done for legacy encoded insns, record the mandatory
>> prefix information in the respective opcode modifier field. Do this
>> without changing the source table, but rather by deriving the values from
>> their existing source representation.
>>
>> gas/
>> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>>
>> 	* config/tc-i386.c (md_begin): Add assertion.
>> 	(build_vex_prefix): Drop implied prefix calculation.
>> 	(build_evex_prefix): Likewise.
>> 	(optimize_encoding): Adjust opcode checks.
>> 	(load_insn_p): Also check opcodeprefix.
>> 	(match_template): Also check opcodespace.
>> 	(process_suffix): Likewise.
>> 	(process_operands): Likewise.
>> 	(output_insn): Likewise. Also check isprefix when discaring
>> 	standalone LOCK.
>> 	* config/tc-i386-intel.c (i386_intel_operand): Also check
>> 	opcodespace.
>>
>> opcodes/
>> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>>
>> 	* i386-gen.c (process_i386_opcode_modifier): Return void. New
>> 	parameter "prefix". Drop local variable "regular_encoding".
>> 	Record prefix setting / check for consistency.
>> 	(output_i386_opcode): Parse opcode_length and base_opcode
>> 	earlier. Derive prefix encoding. Drop no longer applicable
>> 	consistency checking. Adjust process_i386_opcode_modifier()
>> 	invocation.
>> 	(process_i386_opcodes): Adjust process_i386_opcode_modifier()
>> 	invocation.
>> 	* i386-tbl.h: Re-generate.
> 
> OK.  Thanks.

Thanks. Just to confirm - you being okay with the approach here, are
also okay with the outlined (in a post commit message remark) further
planned course of action?

Jan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2] x86: don't use opcode_length to identify pseudo prefixes
  2021-03-23 12:05       ` H.J. Lu
@ 2021-03-23 16:42         ` Jan Beulich
  2021-03-23 18:32           ` H.J. Lu
  0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-23 16:42 UTC (permalink / raw)
  To: Binutils

This is in preparation of opcode_length going away as a field in the
templates. Identify pseudo prefixes by a base opcode of zero instead:
No real prefix has an opcode of zero. This at the same time allows
dropping a curious special case from i386-gen.

Since most attributes are identical for all pseudo prefixes, take the
opportunity and also template them.

gas/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (parse_insn): Recognize pseudo prefixes by
	base_opcode and extension_opcode.

opcodes/
2021-03-XX  Jan Beulich  <jbeulich@suse.com>

	* i386-gen.c (process_i386_opcode_modifier): Drop IsPrefix
	check.
	* i386-opc.h (Prefix_*): Move #define-s.
	* i386-opc.tbl: Move pseudo prefix enumerator values to
	extension opcode field. Introduce pseudopfx template.
	* i386-tbl.h: Re-generate.
---
v2: Introduce PSEUDO_PREFIX and pseudopfx template.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5101,10 +5101,11 @@ parse_insn (char *line, char *mnemonic)
 		      current_templates->start->name);
 	      return NULL;
 	    }
-	  if (current_templates->start->opcode_length == 0)
+
+	  if (current_templates->start->base_opcode == PSEUDO_PREFIX)
 	    {
 	      /* Handle pseudo prefixes.  */
-	      switch (current_templates->start->base_opcode)
+	      switch (current_templates->start->extension_opcode)
 		{
 		case Prefix_Disp8:
 		  /* {disp8} */
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1209,8 +1209,7 @@ process_i386_opcode_modifier (FILE *tabl
 		       || strncasecmp(str, "EVex=", 5) == 0
 		       || strncasecmp(str, "Disp8MemShift=", 14) == 0
 		       || strncasecmp(str, "Masking=", 8) == 0
-		       || strcasecmp(str, "SAE") == 0
-		       || strcasecmp(str, "IsPrefix") == 0)
+		       || strcasecmp(str, "SAE") == 0)
 		regular_encoding = 0;
 
 	      set_bitfield (str, modifiers, val, ARRAY_SIZE (modifiers),
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -926,6 +926,17 @@ typedef struct insn_template
 #define Opcode_SIMD_FloatD 0x1 /* Direction bit for SIMD fp insns. */
 #define Opcode_SIMD_IntD 0x10 /* Direction bit for SIMD int insns. */
 
+/* (Fake) base opcode value for pseudo prefixes.  */
+#define PSEUDO_PREFIX 0
+
+  /* extension_opcode is the 3 bit extension for group <n> insns.
+     This field is also used to store the 8-bit opcode suffix for the
+     AMD 3DNow! instructions.
+     If this template has no extension opcode (the usual case) use None
+     Instructions */
+  unsigned short extension_opcode;
+#define None 0xffff		/* If no extension_opcode is possible.  */
+
 /* Pseudo prefixes.  */
 #define Prefix_Disp8		0	/* {disp8} */
 #define Prefix_Disp16		1	/* {disp16} */
@@ -938,14 +949,6 @@ typedef struct insn_template
 #define Prefix_REX		8	/* {rex} */
 #define Prefix_NoOptimize	9	/* {nooptimize} */
 
-  /* extension_opcode is the 3 bit extension for group <n> insns.
-     This field is also used to store the 8-bit opcode suffix for the
-     AMD 3DNow! instructions.
-     If this template has no extension opcode (the usual case) use None
-     Instructions */
-  unsigned short extension_opcode;
-#define None 0xffff		/* If no extension_opcode is possible.  */
-
   /* Opcode length.  */
   unsigned char opcode_length;
 
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -856,19 +856,14 @@ rex.wrb, 0x4d, None, 1, Cpu64, No_bSuf|N
 rex.wrx, 0x4e, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
 rex.wrxb, 0x4f, None, 1, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
 
-// Pseudo prefixes (opcode_length == 0)
+// Pseudo prefixes (base_opcode == PSEUDO_PREFIX)
 
-{disp8}, Prefix_Disp8, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{disp16}, Prefix_Disp16, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{disp32}, Prefix_Disp32, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{load}, Prefix_Load, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{store}, Prefix_Store, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{vex}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{vex2}, Prefix_VEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{vex3}, Prefix_VEX3, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{evex}, Prefix_EVEX, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{rex}, Prefix_REX, None, 0, Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
-{nooptimize}, Prefix_NoOptimize, None, 0, 0, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
+<pseudopfx:ident:cpu, disp8:Disp8:0, disp16:Disp16:0, disp32:Disp32:0, \
+                      load:Load:0, store:Store:0, \
+                      vex:VEX:0, vex2:VEX:0, vex3:VEX3:0, evex:EVEX:0, \
+                      rex:REX:Cpu64, nooptimize:NoOptimize:0>
+
+{<pseudopfx>}, PSEUDO_PREFIX, Prefix_<pseudopfx:ident>, 0, <pseudopfx:cpu>, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsPrefix, {}
 
 // 486 extensions.
 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] x86: don't use opcode_length to identify pseudo prefixes
  2021-03-23 16:42         ` [PATCH v2] " Jan Beulich
@ 2021-03-23 18:32           ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-23 18:32 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Tue, Mar 23, 2021 at 9:42 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> This is in preparation of opcode_length going away as a field in the
> templates. Identify pseudo prefixes by a base opcode of zero instead:
> No real prefix has an opcode of zero. This at the same time allows
> dropping a curious special case from i386-gen.
>
> Since most attributes are identical for all pseudo prefixes, take the
> opportunity and also template them.
>
> gas/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (parse_insn): Recognize pseudo prefixes by
>         base_opcode and extension_opcode.
>
> opcodes/
> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * i386-gen.c (process_i386_opcode_modifier): Drop IsPrefix
>         check.
>         * i386-opc.h (Prefix_*): Move #define-s.
>         * i386-opc.tbl: Move pseudo prefix enumerator values to
>         extension opcode field. Introduce pseudopfx template.
>         * i386-tbl.h: Re-generate.
> ---
> v2: Introduce PSEUDO_PREFIX and pseudopfx template.
>

OK.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode
  2021-03-23 16:36     ` Jan Beulich
@ 2021-03-23 18:34       ` H.J. Lu
  2021-03-24  7:27         ` Jan Beulich
  0 siblings, 1 reply; 23+ messages in thread
From: H.J. Lu @ 2021-03-23 18:34 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Tue, Mar 23, 2021 at 9:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.03.2021 19:03, H.J. Lu wrote:
> > On Mon, Mar 22, 2021 at 05:46:14PM +0100, Jan Beulich wrote:
> >> Just like is already done for legacy encoded insns, record the mandatory
> >> prefix information in the respective opcode modifier field. Do this
> >> without changing the source table, but rather by deriving the values from
> >> their existing source representation.
> >>
> >> gas/
> >> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> >>
> >>      * config/tc-i386.c (md_begin): Add assertion.
> >>      (build_vex_prefix): Drop implied prefix calculation.
> >>      (build_evex_prefix): Likewise.
> >>      (optimize_encoding): Adjust opcode checks.
> >>      (load_insn_p): Also check opcodeprefix.
> >>      (match_template): Also check opcodespace.
> >>      (process_suffix): Likewise.
> >>      (process_operands): Likewise.
> >>      (output_insn): Likewise. Also check isprefix when discaring
> >>      standalone LOCK.
> >>      * config/tc-i386-intel.c (i386_intel_operand): Also check
> >>      opcodespace.
> >>
> >> opcodes/
> >> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> >>
> >>      * i386-gen.c (process_i386_opcode_modifier): Return void. New
> >>      parameter "prefix". Drop local variable "regular_encoding".
> >>      Record prefix setting / check for consistency.
> >>      (output_i386_opcode): Parse opcode_length and base_opcode
> >>      earlier. Derive prefix encoding. Drop no longer applicable
> >>      consistency checking. Adjust process_i386_opcode_modifier()
> >>      invocation.
> >>      (process_i386_opcodes): Adjust process_i386_opcode_modifier()
> >>      invocation.
> >>      * i386-tbl.h: Re-generate.
> >
> > OK.  Thanks.
>
> Thanks. Just to confirm - you being okay with the approach here, are
> also okay with the outlined (in a post commit message remark) further
> planned course of action?
>

Sounds good to me.  But I need to see the actual patch for sure.

-- 
H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode
  2021-03-23 18:34       ` H.J. Lu
@ 2021-03-24  7:27         ` Jan Beulich
  2021-03-24 13:43           ` H.J. Lu
  0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2021-03-24  7:27 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 23.03.2021 19:34, H.J. Lu wrote:
> On Tue, Mar 23, 2021 at 9:36 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 22.03.2021 19:03, H.J. Lu wrote:
>>> On Mon, Mar 22, 2021 at 05:46:14PM +0100, Jan Beulich wrote:
>>>> Just like is already done for legacy encoded insns, record the mandatory
>>>> prefix information in the respective opcode modifier field. Do this
>>>> without changing the source table, but rather by deriving the values from
>>>> their existing source representation.
>>>>
>>>> gas/
>>>> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>>>>
>>>>      * config/tc-i386.c (md_begin): Add assertion.
>>>>      (build_vex_prefix): Drop implied prefix calculation.
>>>>      (build_evex_prefix): Likewise.
>>>>      (optimize_encoding): Adjust opcode checks.
>>>>      (load_insn_p): Also check opcodeprefix.
>>>>      (match_template): Also check opcodespace.
>>>>      (process_suffix): Likewise.
>>>>      (process_operands): Likewise.
>>>>      (output_insn): Likewise. Also check isprefix when discaring
>>>>      standalone LOCK.
>>>>      * config/tc-i386-intel.c (i386_intel_operand): Also check
>>>>      opcodespace.
>>>>
>>>> opcodes/
>>>> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
>>>>
>>>>      * i386-gen.c (process_i386_opcode_modifier): Return void. New
>>>>      parameter "prefix". Drop local variable "regular_encoding".
>>>>      Record prefix setting / check for consistency.
>>>>      (output_i386_opcode): Parse opcode_length and base_opcode
>>>>      earlier. Derive prefix encoding. Drop no longer applicable
>>>>      consistency checking. Adjust process_i386_opcode_modifier()
>>>>      invocation.
>>>>      (process_i386_opcodes): Adjust process_i386_opcode_modifier()
>>>>      invocation.
>>>>      * i386-tbl.h: Re-generate.
>>>
>>> OK.  Thanks.
>>
>> Thanks. Just to confirm - you being okay with the approach here, are
>> also okay with the outlined (in a post commit message remark) further
>> planned course of action?
> 
> Sounds good to me.  But I need to see the actual patch for sure.

Well, there are multiple steps. The first one, to extract 0f etc
"prefixes" from the opcodes, is less likely to be controversial.
Reverting the PREFIX_0X<nn> uses on legacy encoded opcodes is
likely to rank in the middle, while moving encoding space
specification to the actual opcodes for VEX/XOP/EVEX templates is
likely the most questionable one, not the least because of the
need to "invent" a representation for XOP (I'm considering to use
8f0[89a] as a prefix, but I can also see alternatives).

Jan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode
  2021-03-24  7:27         ` Jan Beulich
@ 2021-03-24 13:43           ` H.J. Lu
  0 siblings, 0 replies; 23+ messages in thread
From: H.J. Lu @ 2021-03-24 13:43 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Wed, Mar 24, 2021 at 12:27 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 23.03.2021 19:34, H.J. Lu wrote:
> > On Tue, Mar 23, 2021 at 9:36 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 22.03.2021 19:03, H.J. Lu wrote:
> >>> On Mon, Mar 22, 2021 at 05:46:14PM +0100, Jan Beulich wrote:
> >>>> Just like is already done for legacy encoded insns, record the mandatory
> >>>> prefix information in the respective opcode modifier field. Do this
> >>>> without changing the source table, but rather by deriving the values from
> >>>> their existing source representation.
> >>>>
> >>>> gas/
> >>>> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> >>>>
> >>>>      * config/tc-i386.c (md_begin): Add assertion.
> >>>>      (build_vex_prefix): Drop implied prefix calculation.
> >>>>      (build_evex_prefix): Likewise.
> >>>>      (optimize_encoding): Adjust opcode checks.
> >>>>      (load_insn_p): Also check opcodeprefix.
> >>>>      (match_template): Also check opcodespace.
> >>>>      (process_suffix): Likewise.
> >>>>      (process_operands): Likewise.
> >>>>      (output_insn): Likewise. Also check isprefix when discaring
> >>>>      standalone LOCK.
> >>>>      * config/tc-i386-intel.c (i386_intel_operand): Also check
> >>>>      opcodespace.
> >>>>
> >>>> opcodes/
> >>>> 2021-03-XX  Jan Beulich  <jbeulich@suse.com>
> >>>>
> >>>>      * i386-gen.c (process_i386_opcode_modifier): Return void. New
> >>>>      parameter "prefix". Drop local variable "regular_encoding".
> >>>>      Record prefix setting / check for consistency.
> >>>>      (output_i386_opcode): Parse opcode_length and base_opcode
> >>>>      earlier. Derive prefix encoding. Drop no longer applicable
> >>>>      consistency checking. Adjust process_i386_opcode_modifier()
> >>>>      invocation.
> >>>>      (process_i386_opcodes): Adjust process_i386_opcode_modifier()
> >>>>      invocation.
> >>>>      * i386-tbl.h: Re-generate.
> >>>
> >>> OK.  Thanks.
> >>
> >> Thanks. Just to confirm - you being okay with the approach here, are
> >> also okay with the outlined (in a post commit message remark) further
> >> planned course of action?
> >
> > Sounds good to me.  But I need to see the actual patch for sure.
>
> Well, there are multiple steps. The first one, to extract 0f etc
> "prefixes" from the opcodes, is less likely to be controversial.
> Reverting the PREFIX_0X<nn> uses on legacy encoded opcodes is
> likely to rank in the middle, while moving encoding space
> specification to the actual opcodes for VEX/XOP/EVEX templates is
> likely the most questionable one, not the least because of the
> need to "invent" a representation for XOP (I'm considering to use
> 8f0[89a] as a prefix, but I can also see alternatives).
>

We will see how it works out.

-- 
H.J.

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-03-24 13:43 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 16:40 [PATCH 0/8] x86: work towards further opcode table compaction Jan Beulich
2021-03-22 16:42 ` [PATCH 1/8] x86: unbreak certain MPX insn operand forms Jan Beulich
2021-03-22 17:58   ` H.J. Lu
2021-03-22 16:42 ` [PATCH 2/8] x86: don't open-code PREFIX_NONE Jan Beulich
2021-03-22 17:58   ` H.J. Lu
2021-03-22 16:43 ` [PATCH 3/8] x86: don't use opcode_length to identify pseudo prefixes Jan Beulich
2021-03-22 17:55   ` H.J. Lu
2021-03-23  7:36     ` Jan Beulich
2021-03-23 12:05       ` H.J. Lu
2021-03-23 16:42         ` [PATCH v2] " Jan Beulich
2021-03-23 18:32           ` H.J. Lu
2021-03-22 16:44 ` [PATCH 5/8] x86: re-order two fields of struct insn_template Jan Beulich
2021-03-22 18:03   ` H.J. Lu
2021-03-22 16:45 ` [PATCH 6/8] x86: re-number PREFIX_0X<nn> Jan Beulich
2021-03-22 18:03   ` H.J. Lu
2021-03-22 16:46 ` [PATCH 7/8] x86: derive mandatory prefix attribute from base opcode Jan Beulich
2021-03-22 18:03   ` H.J. Lu
2021-03-23 16:36     ` Jan Beulich
2021-03-23 18:34       ` H.J. Lu
2021-03-24  7:27         ` Jan Beulich
2021-03-24 13:43           ` H.J. Lu
     [not found] ` <65b17a6a-cc6a-a706-5e95-a7284c45beb1@suse.com>
2021-03-22 18:02   ` [PATCH 8/8] x86: derive opcode length from opcode value H.J. Lu
     [not found] ` <f116024a-077d-c64e-f37c-1da8299272f0@suse.com>
2021-03-22 18:04   ` [PATCH 4/8] x86: split opcode prefix and opcode space representation H.J. Lu

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