Re: [PATCH 2/3] target/loongarch: Fix LLSC for LoongArch32

[Richard Henderson]

On 12/23/24 13:01, Jiaxun Yang wrote:
> 在2024年12月23日十二月 下午3:15，Richard Henderson写道：
> > On 12/22/24 15:40, Jiaxun Yang wrote:
> > > @@ -9,7 +9,7 @@ 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);
> > ...
> > > @@ -28,7 +28,8 @@ static bool gen_sc(DisasContext *ctx, arg_rr_i *a, MemOp mop)
> > >        TCGLabel *l1 = gen_new_label();
> > >        TCGLabel *done = gen_new_label();
> > >    
> > > -    tcg_gen_addi_tl(t0, src1, a->imm);
> > > +    tcg_gen_mov_tl(t0, src1);
> > > +    t0 = make_address_i(ctx, t0, a->imm);
> >
> > The move before make_address_i is not required.
> > See the similar code just above in gen_ll.
>
> I think it’s necessary, I thought the same and end up spending hours to track 
> down the problem.
>
> make_address_i won’t make a new temp_reg if imm is zero.

Correct.

> So when imm is 0 src1 and src2 is the same tcg reg the value will be clobbered 
> by cmpxchg,
> causing a couple of tcg tests to fail.
>
> I think only way to ensure t0 is a new temp reg is to perform a move here.

The correct thing to do is not to re-use the same temporary for both uses.

-    TCGv t0 = tcg_temp_new();
-    tcg_gen_addi_tl(t0, src1, a->imm);
+    TCGv t0 = make_address_i(ctx, src1, a->imm);
     tcg_gen_brcond_tl(TCG_COND_EQ, t0, cpu_lladdr, l1);
     tcg_gen_movi_tl(dest, 0);
     tcg_gen_br(done);

     gen_set_label(l1);
     tcg_gen_mov_tl(val, src2);
     /* generate cmpxchg */
+    t0 = tcg_temp_new();
     tcg_gen_atomic_cmpxchg_tl(t0, cpu_lladdr, cpu_llval,
                               val, ctx->mem_idx, mop);


r~