Re: [PATCH v2 03/22] tcg/i386: Split out target constraints to tcg-target-con-str.h

[Peter Maydell]

On Fri, 15 Jan 2021 at 21:08, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This eliminates the target-specific function target_parse_constraint
> and folds it into the single caller, process_op_defs.  Since this is
> done directly into the switch statement, duplicates are compilation
> errors rather than silently ignored at runtime.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/i386/tcg-target-con-str.h |  33 +++++++++++
>  tcg/i386/tcg-target.h         |   1 +
>  tcg/tcg.c                     |  33 +++++++++--
>  tcg/i386/tcg-target.c.inc     | 101 ++++++----------------------------
>  4 files changed, 78 insertions(+), 90 deletions(-)
>  create mode 100644 tcg/i386/tcg-target-con-str.h

> +REGS('r', ALL_GENERAL_REGS)
> +REGS('x', ALL_VECTOR_REGS)
> +REGS('q', ALL_BYTEL_REGS)     /* regs that can be used as a byte operand */
> +REGS('Q', ALL_BYTEH_REGS)     /* regs with a second byte (e.g. %ah) */
> +REGS('L', ALL_GENERAL_REGS & ~SOFTMMU_RESERVE_REGS)  /* qemu_ld/st */
> +REGS('s', ALL_BYTEL_REGS & ~SOFTMMU_RESERVE_REGS)    /* qemu_st8_i32 data */

In the new setup we define 's' as the BYTEL regs minus the
softmmu reserved ones...

> diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
> index 74637f654a..c4b0b6bfca 100644
> --- a/tcg/i386/tcg-target.c.inc
> +++ b/tcg/i386/tcg-target.c.inc
> @@ -132,6 +132,22 @@ static const int tcg_target_call_oarg_regs[] = {
>  # define TCG_REG_L1 TCG_REG_EDX
>  #endif
>
> +#define ALL_BYTEH_REGS         0x0000000fu
> +#if TCG_TARGET_REG_BITS == 64
> +# define ALL_GENERAL_REGS      0x0000ffffu
> +# define ALL_VECTOR_REGS       0xffff0000u
> +# define ALL_BYTEL_REGS        ALL_GENERAL_REGS
> +#else
> +# define ALL_GENERAL_REGS      0x000000ffu
> +# define ALL_VECTOR_REGS       0x00ff0000u
> +# define ALL_BYTEL_REGS        ALL_BYTEH_REGS
> +#endif
> +#ifdef CONFIG_SOFTMMU
> +# define SOFTMMU_RESERVE_REGS  ((1 << TCG_REG_L0) | (1 << TCG_REG_L1))
> +#else
> +# define SOFTMMU_RESERVE_REGS  0
> +#endif

...which (ignoring L0/L1) is going to be 0xffff on x86-64, and 0xf on i386.

> -    case 's':
> -        /* qemu_st8_i32 data constraint */
> -        ct->regs = 0xf;
> -#ifdef CONFIG_SOFTMMU
> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L0);
> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L1);
> -#endif
> -        break;

But in the old code the 's' constraint is 0xf for both
x86-64 and i386. To match that I think that the new constraint
should use BYTEH, not BYTEL:
REGS('s', ALL_BYTEH_REGS & ~SOFTMMU_RESERVE_REGS)

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM