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

[Chen Gang]

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.

- 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