Re: [PATCH v2 10/22] target/loongarch: Add fixed point load/store instruction translation

[Song Gao]

Hi, Richard.

On 07/23/2021 09:45 AM, Richard Henderson wrote:
>> +/* Fixed point load/store instruction translation */
>> +static bool trans_ld_b(DisasContext *ctx, arg_ld_b *a)
>> +{
>> +    TCGv t0;
>> +    TCGv Rd = cpu_gpr[a->rd];
>> +    int mem_idx = ctx->mem_idx;
>> +
>> +    if (a->rd == 0) {
>> +        /* Nop */
>> +        return true;
>> +    }
> 
> A load into the zero register is not a nop.  It is a load with the result 
> discarded.  One should still fault if the load is to an invalid address.
> 
> You should be using a common routine, passing in the MO_* operand.
> 
OK.

>> +#define ASRTGT                                \
>> +    do {                                      \
>> +        TCGv t1 = get_gpr(a->rj);             \
>> +        TCGv t2 = get_gpr(a->rk);             \
>> +        gen_helper_asrtgt_d(cpu_env, t1, t2); \
>> +    } while (0)
>> +
>> +#define ASRTLE                                \
>> +    do {                                      \
>> +        TCGv t1 = get_gpr(a->rj);             \
>> +        TCGv t2 = get_gpr(a->rk);             \
>> +        gen_helper_asrtle_d(cpu_env, t1, t2); \
>> +    } while (0)
>> +
>> +#define DECL_ARG(name)   \
>> +    arg_ ## name arg = { \
>> +        .rd = a->rd,     \
>> +        .rj = a->rj,     \
>> +        .rk = a->rk,     \
>> +    };
>> +
>> +static bool trans_ldgt_b(DisasContext *ctx, arg_ldgt_b *a)
>> +{
>> +    ASRTGT;
>> +    DECL_ARG(ldx_b)
>> +    trans_ldx_b(ctx, &arg);
>> +    return true;
>> +}
> 
> Use of a common routine would avoid the macro ugliness.
OK.

Thanks you kindly help.

Thanks
Song Gao.