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

[Richard Henderson]

On 08/25/2015 06:12 AM, Chen Gang wrote:
> ----------------------------------------
> > 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.

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


r~