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

[Laurent Vivier <laurent@vivier.eu>]

Le 03/11/2016 à 17:21, Laurent Vivier a écrit :
> 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 <laurent@vivier.eu>
>>>>> ---
>>>>> 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?  

I need to add:

It doesn't fail before. The problem is only with qemu-sh4.

Laurent