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

[gaosong]

在 2023/8/12 上午12:13, Richard Henderson 写道:
> 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.
Yes, you are right, I will correct it on v3.

> > @@ -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.
Ok.

Thanks.
Song Gao