Re: [PATCH v2] x86: re-order insn template fields

["H.J. Lu" <hjl.tools@gmail.com>]

On Fri, Jul 15, 2022 at 5:57 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> This saves quite a number of shift instructions: The "operands" field
> can now be retrieved by just masking (no shift), and extracting the
> "extension_opcode" field now only requires a (signed) right shift, with
> no prereq left one. (Of course there may be architectures where, in a
> cross build, there might be no difference at all, e.g. when there are
> suitable bitfield extraction insns.)
> ---
> v2: Add comment to struct insn_template.
>
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -1444,8 +1444,8 @@ output_i386_opcode (FILE *table, const c
>      fail (_("%s:%d: %s: residual opcode (0x%0*llx) too large\n"),
>           filename, lineno, name, 2 * length, opcode);
>
> -  fprintf (table, "  { \"%s\", 0x%0*llx%s, %s, %lu,\n",
> -          name, 2 * (int)length, opcode, end, extension_opcode, i);
> +  fprintf (table, "  { \"%s\", 0x%0*llx%s, %lu, %s,\n",
> +          name, 2 * (int)length, opcode, end, i, extension_opcode);
>
>    process_i386_opcode_modifier (table, opcode_modifier, space, prefix,
>                                 operand_types, lineno);
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -915,6 +915,11 @@ typedef struct insn_template
>    /* instruction name sans width suffix ("mov" for movl insns) */
>    char *name;
>
> +  /* Bitfield arrangement is such that individual fields can be easily
> +     extracted (in native builds at least) - either by at most a masking
> +     operation (base_opcode, operands), or by just a (signed) right shift
> +     (extension_opcode).  Please try to maintain this property.  */
> +
>    /* base_opcode is the fundamental opcode byte without optional
>       prefix(es).  */
>    unsigned int base_opcode:16;
> @@ -929,6 +934,12 @@ typedef struct insn_template
>     from all other values above.  */
>  #define Opcode_VexW    0xf /* Operand order controlled by VEX.W. */
>
> +  /* how many operands */
> +  unsigned int operands:3;
> +
> +  /* spare bits */
> +  unsigned int :4;
> +
>  /* (Fake) base opcode value for pseudo prefixes.  */
>  #define PSEUDO_PREFIX 0
>
> @@ -952,9 +963,6 @@ typedef struct insn_template
>  #define Prefix_REX             8       /* {rex} */
>  #define Prefix_NoOptimize      9       /* {nooptimize} */
>
> -  /* how many 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
>       the same instruction */

OK.

Thanks.

-- 
H.J.