Re: [PATCH v2 6/8] target/loongarch: Add avail_LAM to check atomic instructions

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

On 8/11/23 03:02, Song Gao wrote:
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   target/loongarch/insn_trans/trans_atomic.c.inc | 12 ++++++++++++
>   target/loongarch/translate.h                   |  1 +
>   2 files changed, 13 insertions(+)
> 
> diff --git a/target/loongarch/insn_trans/trans_atomic.c.inc b/target/loongarch/insn_trans/trans_atomic.c.inc
> index 194818d74d..867d09375a 100644
> --- a/target/loongarch/insn_trans/trans_atomic.c.inc
> +++ b/target/loongarch/insn_trans/trans_atomic.c.inc
> @@ -9,6 +9,10 @@ static bool gen_ll(DisasContext *ctx, arg_rr_i *a, MemOp mop)
>       TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
>       TCGv t0 = make_address_i(ctx, src1, a->imm);
>   
> +    if (!avail_LAM(ctx)) {
> +        return true;
> +    }
> +
>       tcg_gen_qemu_ld_i64(dest, t0, ctx->mem_idx, mop);
>       tcg_gen_st_tl(t0, cpu_env, offsetof(CPULoongArchState, lladdr));
>       tcg_gen_st_tl(dest, cpu_env, offsetof(CPULoongArchState, llval));
> @@ -25,6 +29,10 @@ static bool gen_sc(DisasContext *ctx, arg_rr_i *a, MemOp mop)
>       TCGv t0 = tcg_temp_new();
>       TCGv val = tcg_temp_new();
>   
> +    if (!avail_LAM(ctx)) {
> +        return true;
> +    }
> +
>       TCGLabel *l1 = gen_new_label();
>       TCGLabel *done = gen_new_label();

I think these two are wrong.  LAM says "AM* atomic memory instructions".
I think LL/SC are always available.

>   
> @@ -53,6 +61,10 @@ static bool gen_am(DisasContext *ctx, arg_rrr *a,
>       TCGv addr = gpr_src(ctx, a->rj, EXT_NONE);
>       TCGv val = gpr_src(ctx, a->rk, EXT_NONE);
>   
> +    if (!avail_LAM(ctx)) {
> +        return true;
> +    }

While correct, I think it would be better style to use LAM instead of ALL in the 
TRANS(am*) instructions.


r~