Re: [Qemu-devel] [PATCH v14 30/33] target-tilegx: Handle atomic instructions

[Chen Gang]

----------------------------------------
> From: address@hidden
> To: address@hidden; address@hidden
> CC: address@hidden; address@hidden; address@hidden
> Subject: Re: [Qemu-devel] [PATCH v14 30/33] target-tilegx: Handle atomic 
> instructions
> Date: Tue, 25 Aug 2015 21:11:11 +0800
>
> On 8/25/15 12:15, Richard Henderson wrote:
>> On 08/24/2015 09:17 AM, Richard Henderson wrote:
>>> Signed-off-by: Richard Henderson <address@hidden>
>>> ---
>>> target-tilegx/translate.c | 50 
>>> ++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 49 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
>>> index 210e912..2a0798a 100644
>>> --- a/target-tilegx/translate.c
>>> +++ b/target-tilegx/translate.c
>>> @@ -180,6 +180,19 @@ static void gen_saturate_op(TCGv tdest, TCGv tsrca, 
>>> TCGv tsrcb,
>>> tcg_temp_free(t0);
>>> }
>>>
>>> +static void gen_atomic_excp(DisasContext *dc, unsigned dest, unsigned srca,
>>> + unsigned srcb, TileExcp excp)
>>> +{
>>> +#ifdef CONFIG_USER_ONLY
>>> + TCGv_i32 t = tcg_const_i32((dest << 16) | (srca << 8) | srcb);
>>> + tcg_gen_st_i32(t, cpu_env, offsetof(CPUTLGState, excparam));
>>> + tcg_temp_free_i32(t);
>>> + gen_exception(dc, excp);
>>> +#else
>>> + gen_exception(dc, TILEGX_EXCP_OPCODE_UNIMPLEMENTED);
>>> +#endif
>>> +}
>
> Originally, I used set_exception(), not gen_exception().
>
>>
>>
>> This is broken. While it does work well enough for Hello World, implementing 
>> a non-trap instruction with an exception is extremely dicey for TileGX. The 
>> issue is that TileGX bundles operate atomically, with no RAW issues between 
>> the instructions of the bundle.
>>
>> Consider a bundle like
>>
>> { add r0, r0, r1 ; exch r2, r0, r3 }
>>
>> In Chen's implementation, the writeback to r0 would occur before the 
>> exception, and so the exch would happen to the wrong address. In my 
>> implementation here, the exception would occur before the writeback, and so 
>> the result of the add would be discarded.
>
> We use tmp regs for buffering the r0.
>
> - calculate x1 pipe, and save result to r0 tmp reg.
>

Oh, typo, calculate x0 pipe, and save result to r0 tmp reg.

> - exch the original r0 and r3 to r2 tmp reg.
>
> - set exception flag (which will cause exception, later).
>
> - save the result tmp regs to r0 or r2.
>
> - gen exception.
>
>>
>> While retaining the current start_exclusive scheme, it would appear that we 
>> need to store more data here -- to wit, the contents of the registers not 
>> their numbers -- and delay the exception until after writeback.
>>
>> Even better, of course, would be to not exit the TB at all, and use host 
>> atomic instructions... I suppose that multi-threading patch set is still in 
>> limbo?
>>
>
> I guess, we need not.
>
>
> Thanks.
> --
> Chen Gang
>
> Open, share, and attitude like air, water, and life which God blessed
>