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