Re: [Qemu-devel] [PATCH v2] target-sh4: add atomic tas

[Laurent Vivier]

Le 03/11/2016 à 17:18, Paolo Bonzini a écrit :
> 
> 
> On 03/11/2016 16:35, Laurent Vivier wrote:
>> Le 03/11/2016 à 16:32, Paolo Bonzini a écrit :
>>>
>>>
>>> On 03/11/2016 15:07, Laurent Vivier wrote:
>>>> Implement real atomic tas:
>>>>
>>>>     When (Rn) = 0, 1 -> T
>>>>     Otherwise, 0 -> T
>>>>     In both cases, 1 -> MSB of (Rn)
>>>>
>>>> using atomic_fetch_or_i32() and setcondi_i32().
>>>>
>>>> Tested with image from:
>>>> http://wiki.qemu.org/download/sh-test-0.2.tar.bz2
>>>>
>>>> This image contains a "tas_test" that runs without
>>>> error with this change.
>>>>
>>>> Signed-off-by: Laurent Vivier <address@hidden>
>>>> ---
>>>> v2:
>>>> - don't use helper but atomic_fetch_or_i32
>>>>   Thank you Paolo!
>>>>
>>>>  target-sh4/translate.c | 19 ++++++++-----------
>>>>  1 file changed, 8 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/target-sh4/translate.c b/target-sh4/translate.c
>>>> index c89a147..1b83d59 100644
>>>> --- a/target-sh4/translate.c
>>>> +++ b/target-sh4/translate.c
>>>> @@ -1640,18 +1640,15 @@ static void _decode_opc(DisasContext * ctx)
>>>>    tcg_gen_shri_i32(REG(B11_8), REG(B11_8), 16);
>>>>    return;
>>>>      case 0x401b:          /* tas.b @Rn */
>>>> -  {
>>>> -      TCGv addr, val;
>>>> -      addr = tcg_temp_local_new();
>>>> -      tcg_gen_mov_i32(addr, REG(B11_8));
>>>> -      val = tcg_temp_local_new();
>>>> -            tcg_gen_qemu_ld_i32(val, addr, ctx->memidx, MO_UB);
>>>> +        {
>>>> +            TCGv val = tcg_temp_new();
>>>> +            TCGv msb = tcg_const_i32(0x80);
>>>> +            tcg_gen_atomic_fetch_or_i32(val, REG(B11_8), msb,
>>>> +                                        ctx->memidx, MO_UB);
>>>> +            tcg_temp_free(msb);
>>>>              tcg_gen_setcondi_i32(TCG_COND_EQ, cpu_sr_t, val, 0);
>>>> -      tcg_gen_ori_i32(val, val, 0x80);
>>>> -            tcg_gen_qemu_st_i32(val, addr, ctx->memidx, MO_UB);
>>>> -      tcg_temp_free(val);
>>>> -      tcg_temp_free(addr);
>>>> -  }
>>>> +            tcg_temp_free(val);
>>>> +        }
>>>>    return;
>>>>      case 0xf00d: /* fsts FPUL,FRn - FPSCR: Nothing */
>>>>    CHECK_FPU_ENABLED
>>>>
>>>
>>> For 2.8?
>>
>> Is it possible?
> 
> Well, tas_test "runs without error with this change", I suppose it fails
> before?  In other words, is this patch enough to run multithreaded sh4
> programs with qemu-user?

It should,:the problem was reported by Adrian (cc:) while compiling ghc
in qemu-sh4, but I have just tested the functionality with the softmmu
version, not the atomicity.

Adrian, could you test this patch?

Thanks,
Laurent