Re: [PATCH v2 23/30] tcg/loongarch64: Add softmmu load/store helpers, implement qemu_ld/qemu_st ops

[Richard Henderson <richard.henderson@linaro.org>]

On 9/21/21 1:19 PM, WANG Xuerui wrote:
> +    /* Compare masked address with the TLB entry.  */
> +    label_ptr[0] = s->code_ptr;
> +    tcg_out_opc_bne(s, TCG_REG_TMP0, TCG_REG_TMP1, 0);
> +
> +    /* TLB Hit - translate address using addend.  */
> +    tcg_out_opc_add_d(s, TCG_REG_TMP0, TCG_REG_TMP2, addrl);

You removed a little too much here.  You still need

     if (TARGET_LONG_BITS == 32) {
         tcg_out_ext32u(s, TCG_REG_TMP0, addrl);
         addrl = TCG_REG_TMP0;
     }
     tcg_out_opc_add_d(s, TCG_REG_TMP0, TCG_REG_TMP2, addrl);

> +static void add_qemu_ldst_label(TCGContext *s, int is_ld, TCGMemOpIdx oi,
> +                                TCGReg datalo, TCGReg addrlo,
> +                                void *raddr, tcg_insn_unit **label_ptr)
> +{
> +    TCGLabelQemuLdst *label = new_ldst_label(s);
> +
> +    label->is_ld = is_ld;
> +    label->oi = oi;
> +    label->type = 0;

Type should be set based on "is_64" argument to tcg_out_qemu_ld (or indeed, is_64 could be 
replaced by "type", which would probably make more sense).

This will be used to fix...

> +    if (opc & MO_SIGN) {
> +        /* Sign-extend directly into destination.  */
> +        switch (size) {
> +        case MO_8:
> +            tcg_out_ext8s(s, l->datalo_reg, TCG_REG_A0);
> +            break;
> +        case MO_16:
> +            tcg_out_ext16s(s, l->datalo_reg, TCG_REG_A0);
> +            break;
> +        case MO_32:
> +            tcg_out_ext32s(s, l->datalo_reg, TCG_REG_A0);
> +            break;
> +        default:
> +            g_assert_not_reached();
> +            break;
> +        }
> +    } else {
> +        tcg_out_mov(s, size == MO_64, l->datalo_reg, TCG_REG_A0);
> +    }

... this, where TCG_TYPE_I32 loads should always be sign-extended from 32-bits.  Something 
like

     switch (opc & MO_SSIZE) {
     case MO_SB:
         ext8s;
         break;
     case MO_SH:
         ext16s;
         break;
     case MO_SL:
         ext32s;
         break;
     case MO_UL:
         if (type == TCG_TYPE_I32) {
             ext32s;
             break;
         }
         /* fall through */
     default:
         tcg_out_mov(s, TCG_TYPE_REG, datalo, A0);
         break;
     }

> +    case MO_64:
> +        tcg_out_mov(s, TCG_TYPE_PTR, TCG_REG_A2, l->datalo_reg);

TCG_TYPE_I64, to match MO_64.

> +    if (USE_GUEST_BASE) {
> +        tcg_out_opc_add_d(s, base, TCG_GUEST_BASE_REG, addr_regl);
> +    } else {
> +        tcg_out_opc_add_d(s, base, addr_regl, TCG_REG_ZERO);
> +    }

Still adding zero in tcg_out_qemu_st.


r~