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

[Richard Henderson <rth@twiddle.net>]

On 08/25/2015 06:12 AM, Chen Gang wrote:
>
>
> ----------------------------------------
>> From: xili_gchen_5257@hotmail.com
>> To: rth@twiddle.net; qemu-devel@nongnu.org
>> CC: walt@tilera.com; cmetcalf@ezchip.com; peter.maydell@linaro.org
>> 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 <rth@twiddle.net>
>>>> ---
>>>> 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.

Exactly.  Now re-read what I wrote and see if you can spot the problem with this.


r~