Re: [PATCH 7/8] target/mips: Support TCG_TARGET_SIGNED_ADDR32

[WANG Xuerui <i.qemu@xen0n.name>]

Hi Richard,

On 2021/10/11 01:44, Richard Henderson wrote:
> All 32-bit mips operations sign-extend the output, so we are easily
> able to keep TCG_TYPE_I32 values sign-extended in host registers.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/mips/tcg-target-sa32.h |  8 ++++++++
>  tcg/mips/tcg-target.c.inc  | 13 +++----------
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/tcg/mips/tcg-target-sa32.h b/tcg/mips/tcg-target-sa32.h
> index cb185b1526..51255e7cba 100644
> --- a/tcg/mips/tcg-target-sa32.h
> +++ b/tcg/mips/tcg-target-sa32.h
> @@ -1 +1,9 @@
> +/*
> + * Do not set TCG_TARGET_SIGNED_ADDR32 for mips32;
> + * TCG expects this to only be set for 64-bit hosts.
> + */
> +#ifdef __mips64
> +#define TCG_TARGET_SIGNED_ADDR32 1
> +#else
>  #define TCG_TARGET_SIGNED_ADDR32 0
> +#endif
It looks like we never want to set TCG_TARGET_SIGNED_ADDR32 on 32-bit
hosts; maybe a compile-time assert could be added somewhere for
statically guaranteeing this?
> diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc
> index d8f6914f03..cc3b4d5b90 100644
> --- a/tcg/mips/tcg-target.c.inc
> +++ b/tcg/mips/tcg-target.c.inc
> @@ -1161,20 +1161,13 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl,
>          tcg_out_ld(s, TCG_TYPE_I32, TCG_TMP0, TCG_TMP3, cmp_off + LO_OFF);
>          tcg_out_movi(s, TCG_TYPE_I32, TCG_TMP1, mask);
>      } else {
> -        tcg_out_ldst(s, (TARGET_LONG_BITS == 64 ? OPC_LD
> -                         : TCG_TARGET_REG_BITS == 64 ? OPC_LWU : OPC_LW),
> -                     TCG_TMP0, TCG_TMP3, cmp_off);
> +        tcg_out_ld(s, TCG_TYPE_TL, TCG_TMP0, TCG_TMP3, cmp_off);
>          tcg_out_movi(s, TCG_TYPE_TL, TCG_TMP1, mask);
>          /* No second compare is required here;
>             load the tlb addend for the fast path.  */
>          tcg_out_ld(s, TCG_TYPE_PTR, TCG_TMP2, TCG_TMP3, add_off);
>      }
>  
> -    /* Zero extend a 32-bit guest address for a 64-bit host. */
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> -        tcg_out_ext32u(s, base, addrl);
> -        addrl = base;
> -    }
>      tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl);
>  
>      label_ptr[0] = s->code_ptr;
> @@ -1456,7 +1449,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64)
>                          data_regl, data_regh, addr_regl, addr_regh,
>                          s->code_ptr, label_ptr);
>  #else
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> +    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
Is there precedent of extracting predicates like this into some header
for reuse? However I agree that the current expression conveys enough
meaning without being overly complicated.
>          tcg_out_ext32u(s, base, addr_regl);
>          addr_regl = base;
>      }
> @@ -1559,7 +1552,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64)
>                          s->code_ptr, label_ptr);
>  #else
>      base = TCG_REG_A0;
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> +    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
>          tcg_out_ext32u(s, base, addr_regl);
>          addr_regl = base;
>      }