[2/6] x86: shrink some struct insn_template fields

Message ID 379a1b1a-f48f-3070-ee76-1b460450d518@suse.com
State New
Headers show
Series
  • x86: further opcode table compaction plus fallout
Related show

Commit Message

H.J. Lu via Binutils March 26, 2021, 10:50 a.m.
Now that all base opcodes are only at most 2 bytes in size, shrink its
template field to just as much. By also shrinking extension_opcode and
operands to just what they really need, we can free up an entire 32-bit
slot (plus 4 left bits past the bitfields themselves).

At present this alters sizeof(struct insn_template) only for 32-bit
builds. In 64-bit builds it instead leaves a padding hole that will
allow to buffer future growth of other fields (opcode_modifier,
cpu_flags, operand_types[]).

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

	* i386-opc.h (struct insn_template): Shrink base_opcode to 16
	bits. Shrink extension_opcode to 9 bits. Make it signed. Change
	value of None. Shrink operands to 3 bits.
---
Code-generation wise it may be better to move the signed
extension_opcode field last within the containing 32-bit slot.

Comments

H.J. Lu via Binutils March 29, 2021, 1 p.m. | #1
On Fri, Mar 26, 2021 at 3:50 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> Now that all base opcodes are only at most 2 bytes in size, shrink its

> template field to just as much. By also shrinking extension_opcode and

> operands to just what they really need, we can free up an entire 32-bit

> slot (plus 4 left bits past the bitfields themselves).

>

> At present this alters sizeof(struct insn_template) only for 32-bit

> builds. In 64-bit builds it instead leaves a padding hole that will

> allow to buffer future growth of other fields (opcode_modifier,

> cpu_flags, operand_types[]).

>

> opcodes/

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

>

>         * i386-opc.h (struct insn_template): Shrink base_opcode to 16

>         bits. Shrink extension_opcode to 9 bits. Make it signed. Change

>         value of None. Shrink operands to 3 bits.

> ---

> Code-generation wise it may be better to move the signed

> extension_opcode field last within the containing 32-bit slot.

>


extension_opcode should be next to opcode in template.

-- 
H.J.
H.J. Lu via Binutils March 29, 2021, 2:03 p.m. | #2
On 29.03.2021 15:00, H.J. Lu wrote:
> On Fri, Mar 26, 2021 at 3:50 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> Now that all base opcodes are only at most 2 bytes in size, shrink its

>> template field to just as much. By also shrinking extension_opcode and

>> operands to just what they really need, we can free up an entire 32-bit

>> slot (plus 4 left bits past the bitfields themselves).

>>

>> At present this alters sizeof(struct insn_template) only for 32-bit

>> builds. In 64-bit builds it instead leaves a padding hole that will

>> allow to buffer future growth of other fields (opcode_modifier,

>> cpu_flags, operand_types[]).

>>

>> opcodes/

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

>>

>>         * i386-opc.h (struct insn_template): Shrink base_opcode to 16

>>         bits. Shrink extension_opcode to 9 bits. Make it signed. Change

>>         value of None. Shrink operands to 3 bits.

>> ---

>> Code-generation wise it may be better to move the signed

>> extension_opcode field last within the containing 32-bit slot.

> 

> extension_opcode should be next to opcode in template.


In the source table, in the binary representation, or both? (I certainly
agree they should be next to each other in the source table.)

Jan
H.J. Lu via Binutils March 29, 2021, 2:41 p.m. | #3
On Mon, Mar 29, 2021 at 7:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 29.03.2021 15:00, H.J. Lu wrote:

> > On Fri, Mar 26, 2021 at 3:50 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> Now that all base opcodes are only at most 2 bytes in size, shrink its

> >> template field to just as much. By also shrinking extension_opcode and

> >> operands to just what they really need, we can free up an entire 32-bit

> >> slot (plus 4 left bits past the bitfields themselves).

> >>

> >> At present this alters sizeof(struct insn_template) only for 32-bit

> >> builds. In 64-bit builds it instead leaves a padding hole that will

> >> allow to buffer future growth of other fields (opcode_modifier,

> >> cpu_flags, operand_types[]).

> >>

> >> opcodes/

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

> >>

> >>         * i386-opc.h (struct insn_template): Shrink base_opcode to 16

> >>         bits. Shrink extension_opcode to 9 bits. Make it signed. Change

> >>         value of None. Shrink operands to 3 bits.

> >> ---

> >> Code-generation wise it may be better to move the signed

> >> extension_opcode field last within the containing 32-bit slot.

> >

> > extension_opcode should be next to opcode in template.

>

> In the source table, in the binary representation, or both? (I certainly

> agree they should be next to each other in the source table.)


Only in the source code, not in the binary representation.

-- 
H.J.
H.J. Lu via Binutils March 29, 2021, 2:49 p.m. | #4
On 29.03.2021 16:41, H.J. Lu wrote:
> On Mon, Mar 29, 2021 at 7:03 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 29.03.2021 15:00, H.J. Lu wrote:

>>> On Fri, Mar 26, 2021 at 3:50 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>

>>>> Now that all base opcodes are only at most 2 bytes in size, shrink its

>>>> template field to just as much. By also shrinking extension_opcode and

>>>> operands to just what they really need, we can free up an entire 32-bit

>>>> slot (plus 4 left bits past the bitfields themselves).

>>>>

>>>> At present this alters sizeof(struct insn_template) only for 32-bit

>>>> builds. In 64-bit builds it instead leaves a padding hole that will

>>>> allow to buffer future growth of other fields (opcode_modifier,

>>>> cpu_flags, operand_types[]).

>>>>

>>>> opcodes/

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

>>>>

>>>>         * i386-opc.h (struct insn_template): Shrink base_opcode to 16

>>>>         bits. Shrink extension_opcode to 9 bits. Make it signed. Change

>>>>         value of None. Shrink operands to 3 bits.

>>>> ---

>>>> Code-generation wise it may be better to move the signed

>>>> extension_opcode field last within the containing 32-bit slot.

>>>

>>> extension_opcode should be next to opcode in template.

>>

>> In the source table, in the binary representation, or both? (I certainly

>> agree they should be next to each other in the source table.)

> 

> Only in the source code, not in the binary representation.


Yet the remark was about the (positive) code gen effects changing the
binary representation was likely to have. I then understand you wouldn't
object to moving this field.

Jan
H.J. Lu via Binutils March 29, 2021, 2:51 p.m. | #5
On Mon, Mar 29, 2021 at 7:49 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 29.03.2021 16:41, H.J. Lu wrote:

> > On Mon, Mar 29, 2021 at 7:03 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> On 29.03.2021 15:00, H.J. Lu wrote:

> >>> On Fri, Mar 26, 2021 at 3:50 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>>

> >>>> Now that all base opcodes are only at most 2 bytes in size, shrink its

> >>>> template field to just as much. By also shrinking extension_opcode and

> >>>> operands to just what they really need, we can free up an entire 32-bit

> >>>> slot (plus 4 left bits past the bitfields themselves).

> >>>>

> >>>> At present this alters sizeof(struct insn_template) only for 32-bit

> >>>> builds. In 64-bit builds it instead leaves a padding hole that will

> >>>> allow to buffer future growth of other fields (opcode_modifier,

> >>>> cpu_flags, operand_types[]).

> >>>>

> >>>> opcodes/

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

> >>>>

> >>>>         * i386-opc.h (struct insn_template): Shrink base_opcode to 16

> >>>>         bits. Shrink extension_opcode to 9 bits. Make it signed. Change

> >>>>         value of None. Shrink operands to 3 bits.

> >>>> ---

> >>>> Code-generation wise it may be better to move the signed

> >>>> extension_opcode field last within the containing 32-bit slot.

> >>>

> >>> extension_opcode should be next to opcode in template.

> >>

> >> In the source table, in the binary representation, or both? (I certainly

> >> agree they should be next to each other in the source table.)

> >

> > Only in the source code, not in the binary representation.

>

> Yet the remark was about the (positive) code gen effects changing the

> binary representation was likely to have. I then understand you wouldn't

> object to moving this field.

>


Correct.

-- 
H.J.

Patch

--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -917,7 +917,7 @@  typedef struct insn_template
 
   /* base_opcode is the fundamental opcode byte without optional
      prefix(es).  */
-  unsigned int base_opcode;
+  unsigned int base_opcode:16;
 #define Opcode_D	0x2 /* Direction bit:
 			       set if Reg --> Regmem;
 			       unset if Regmem --> Reg. */
@@ -934,8 +934,8 @@  typedef struct insn_template
      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.  */
+  signed int extension_opcode:9;
+#define None (-1)		/* If no extension_opcode is possible.  */
 
 /* Pseudo prefixes.  */
 #define Prefix_Disp8		0	/* {disp8} */
@@ -950,7 +950,7 @@  typedef struct insn_template
 #define Prefix_NoOptimize	9	/* {nooptimize} */
 
   /* how many operands */
-  unsigned char operands;
+  unsigned int operands:3;
 
   /* 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