Re: [PATCH for-6.1] tcg/i386: Split P_VEXW from P_REXW

[Peter Maydell]

On Wed, 11 Aug 2021 at 00:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We need to be able to represent VEX.W on a 32-bit host, where REX.W
> will always be zero.  Fixes the encoding for VPSLLVQ and VPSRLVQ.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/385
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/i386/tcg-target.c.inc | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
> index 98d924b91a..997510109d 100644
> --- a/tcg/i386/tcg-target.c.inc
> +++ b/tcg/i386/tcg-target.c.inc
> @@ -241,8 +241,9 @@ static bool tcg_target_const_match(int64_t val, TCGType 
> type, int ct)
>  #define P_EXT          0x100           /* 0x0f opcode prefix */
>  #define P_EXT38         0x200           /* 0x0f 0x38 opcode prefix */
>  #define P_DATA16        0x400           /* 0x66 opcode prefix */
> +#define P_VEXW          0x1000          /* Set VEX.W = 1 */
>  #if TCG_TARGET_REG_BITS == 64
> -# define P_REXW         0x1000          /* Set REX.W = 1 */
> +# define P_REXW         P_VEXW          /* Set REX.W = 1; match VEXW */
>  # define P_REXB_R       0x2000          /* REG field as byte register */
>  # define P_REXB_RM      0x4000          /* R/M field as byte register */
>  # define P_GS           0x8000          /* gs segment override */
> @@ -410,13 +411,13 @@ static bool tcg_target_const_match(int64_t val, TCGType 
> type, int ct)
>  #define OPC_VPBROADCASTW (0x79 | P_EXT38 | P_DATA16)
>  #define OPC_VPBROADCASTD (0x58 | P_EXT38 | P_DATA16)
>  #define OPC_VPBROADCASTQ (0x59 | P_EXT38 | P_DATA16)
> -#define OPC_VPERMQ      (0x00 | P_EXT3A | P_DATA16 | P_REXW)
> +#define OPC_VPERMQ      (0x00 | P_EXT3A | P_DATA16 | P_VEXW)
>  #define OPC_VPERM2I128  (0x46 | P_EXT3A | P_DATA16 | P_VEXL)
>  #define OPC_VPSLLVD     (0x47 | P_EXT38 | P_DATA16)
> -#define OPC_VPSLLVQ     (0x47 | P_EXT38 | P_DATA16 | P_REXW)
> +#define OPC_VPSLLVQ     (0x47 | P_EXT38 | P_DATA16 | P_VEXW)
>  #define OPC_VPSRAVD     (0x46 | P_EXT38 | P_DATA16)
>  #define OPC_VPSRLVD     (0x45 | P_EXT38 | P_DATA16)
> -#define OPC_VPSRLVQ     (0x45 | P_EXT38 | P_DATA16 | P_REXW)
> +#define OPC_VPSRLVQ     (0x45 | P_EXT38 | P_DATA16 | P_VEXW)
>  #define OPC_VZEROUPPER  (0x77 | P_EXT)
>  #define OPC_XCHG_ax_r32        (0x90)
>
> @@ -576,7 +577,7 @@ static void tcg_out_vex_opc(TCGContext *s, int opc, int 
> r, int v,
>
>      /* Use the two byte form if possible, which cannot encode
>         VEX.W, VEX.B, VEX.X, or an m-mmmm field other than P_EXT.  */
> -    if ((opc & (P_EXT | P_EXT38 | P_EXT3A | P_REXW)) == P_EXT
> +    if ((opc & (P_EXT | P_EXT38 | P_EXT3A | P_VEXW)) == P_EXT
>          && ((rm | index) & 8) == 0) {
>          /* Two byte VEX prefix.  */
>          tcg_out8(s, 0xc5);
> @@ -601,7 +602,7 @@ static void tcg_out_vex_opc(TCGContext *s, int opc, int 
> r, int v,
>          tmp |= (rm & 8 ? 0 : 0x20);            /* VEX.B */
>          tcg_out8(s, tmp);
>
> -        tmp = (opc & P_REXW ? 0x80 : 0);       /* VEX.W */
> +        tmp = (opc & P_VEXW ? 0x80 : 0);       /* VEX.W */
>      }
>
>      tmp |= (opc & P_VEXL ? 0x04 : 0);      /* VEX.L */

These changes look OK as far as they go, but it's not clear to
me why the other places that set P_REXW are all OK to use P_REXW
and not P_VEXW. For instance tcg_out_mov() sets rexw = P_REXW
and some of the codepaths there will then pass that into
tcg_out_vex_modrm() which ends up in tcg_out_vex_opc().

More generally, is there somewhere we can assert that we
didn't try to use a REXW prefix for i386 codegen rather
than just silently ignoring it ?

thanks
-- PMM