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

[Richard Henderson]

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~