On Mon, Aug 17, 2020 at 08:52:16AM -0700, Richard Henderson wrote:
> On 8/17/20 7:01 AM, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Use atomic_cmpxchg to implement the atomic cmpxchg sequence.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> > ---
> > target/microblaze/translate.c | 21 +++++++++++++--------
> > 1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> > index c58f334a0f..530c15e5ad 100644
> > --- a/target/microblaze/translate.c
> > +++ b/target/microblaze/translate.c
> > @@ -1075,14 +1075,16 @@ static void dec_store(DisasContext *dc)
> > swx_skip = gen_new_label();
> > tcg_gen_brcond_tl(TCG_COND_NE, env_res_addr, addr, swx_skip);
> >
> > - /* Compare the value loaded at lwx with current contents of
> > - the reserved location.
> > - FIXME: This only works for system emulation where we can expect
> > - this compare and the following write to be atomic. For user
> > - emulation we need to add atomicity between threads. */
> > + /*
> > + * Compare the value loaded at lwx with current contents of
> > + * the reserved location.
> > + */
> > tval = tcg_temp_new_i32();
> > - tcg_gen_qemu_ld_i32(tval, addr, cpu_mmu_index(&dc->cpu->env,
> > false),
> > - MO_TEUL);
> > +
> > + tcg_gen_atomic_cmpxchg_i32(tval, addr, env_res_val,
> > + cpu_R[dc->rd], mem_index,
> > + mop);
> > +
> > tcg_gen_brcond_i32(TCG_COND_NE, env_res_val, tval, swx_skip);
> > write_carryi(dc, 0);
> > tcg_temp_free_i32(tval);
> > @@ -1108,7 +1110,10 @@ static void dec_store(DisasContext *dc)
> > break;
> > }
> > }
> > - tcg_gen_qemu_st_i32(cpu_R[dc->rd], addr, mem_index, mop);
> > +
> > + if (!ex) {
> > + tcg_gen_qemu_st_i32(cpu_R[dc->rd], addr, mem_index, mop);
> > + }
> >
> > /* Verify alignment if needed. */
> > if (dc->cpu->cfg.unaligned_exceptions && size > 1) {
> >
>
> This is fine as far as the actual swx instruction goes, so
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>
> However, some notes for dec_store,
>
> There seems to be a large under-decode here. E.g. rev should never be set for
> swx. But rev is accepted and partially implemented. E.g. there is no sbx
> instruction, but the ex bit is accepted for any store instruction.
>
> Microblaze has a relatively small instruction set. Would you be open to a
> conversion to decodetree? It should be fairly easy.
>
Thanks Richard,
Yes, I've got a conversion to decodetree on my TODO list (before adding 64bit
support).
I'll probably bug you when I get to it!
Best regards,
Edgar
