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

[Jiaxun Yang]

在2024年12月23日十二月 下午11:18，Philippe Mathieu-Daudé写道：
> On 23/12/24 22: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.
>
> Only if va32 = 0, IOW if HW_FLAGS_VA32 is not set. Per is_va32,
> VA32 == !LA64 or VA32L[1-3]
>
> Indeed make_address_x() returns the same TCGv if !VA32 and imm=0.

You are right, thanks for the insight!

>
>> 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.
>
> Missing constraint or mis-designed make_address_x()?
> To KISS I'd use a temp in make_address_x() regardless of addend=0.

Makes sense, will incorporate in next version.

Thanks
>
>> 
>> I think only way to ensure t0 is a new temp reg is to perform a move here.
>> 
>> Thanks
>>>
>>>
>>> r~
>>

-- 
- Jiaxun