| Message ID | 20180427002651.28356-6-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement v8.1-Atomics | expand |
On Fri, Apr 27, 2018 at 12:26 PM, Richard Henderson < richard.henderson@linaro.org> wrote: > Cc: Michael Clark <mjc@sifive.com> > Cc: Palmer Dabbelt <palmer@sifive.com> > Cc: Sagar Karandikar <sagark@eecs.berkeley.edu> > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > Reviewed-by: Michael Clark <mjc@sifive.com> If you give me a remote I can pull into my local workspace and test. The change looks pretty straight-forward. There are tests for amos in riscv-tests but no parallel tests (i.e. contended case). Regarding this target/riscv/translate.c. I'd like to make a patch to remove CPURISCVState *env arguments from several gen functions. --- > target/riscv/translate.c | 72 ++++++++++++++---------------- > ------------------ > 1 file changed, 20 insertions(+), 52 deletions(-) > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index 808eab7f50..9cab717088 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -725,7 +725,6 @@ static void gen_atomic(DisasContext *ctx, uint32_t opc, > TCGv src1, src2, dat; > TCGLabel *l1, *l2; > TCGMemOp mop; > - TCGCond cond; > bool aq, rl; > > /* Extract the size of the atomic operation. */ > @@ -823,60 +822,29 @@ static void gen_atomic(DisasContext *ctx, uint32_t > opc, > tcg_gen_atomic_fetch_or_tl(src2, src1, src2, ctx->mem_idx, mop); > gen_set_gpr(rd, src2); > break; > - > case OPC_RISC_AMOMIN: > - cond = TCG_COND_LT; > - goto do_minmax; > - case OPC_RISC_AMOMAX: > - cond = TCG_COND_GT; > - goto do_minmax; > - case OPC_RISC_AMOMINU: > - cond = TCG_COND_LTU; > - goto do_minmax; > - case OPC_RISC_AMOMAXU: > - cond = TCG_COND_GTU; > - goto do_minmax; > - do_minmax: > - /* Handle the RL barrier. The AQ barrier is handled along the > - parallel path by the SC atomic cmpxchg. On the serial path, > - of course, barriers do not matter. */ > - if (rl) { > - tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); > - } > - if (tb_cflags(ctx->tb) & CF_PARALLEL) { > - l1 = gen_new_label(); > - gen_set_label(l1); > - } else { > - l1 = NULL; > - } > - > gen_get_gpr(src1, rs1); > gen_get_gpr(src2, rs2); > - if ((mop & MO_SSIZE) == MO_SL) { > - /* Sign-extend the register comparison input. */ > - tcg_gen_ext32s_tl(src2, src2); > - } > - dat = tcg_temp_local_new(); > - tcg_gen_qemu_ld_tl(dat, src1, ctx->mem_idx, mop); > - tcg_gen_movcond_tl(cond, src2, dat, src2, dat, src2); > - > - if (tb_cflags(ctx->tb) & CF_PARALLEL) { > - /* Parallel context. Make this operation atomic by verifying > - that the memory didn't change while we computed the > result. */ > - tcg_gen_atomic_cmpxchg_tl(src2, src1, dat, src2, > ctx->mem_idx, mop); > - > - /* If the cmpxchg failed, retry. */ > - /* ??? There is an assumption here that this will eventually > - succeed, such that we don't live-lock. This is not unlike > - a similar loop that the compiler would generate for e.g. > - __atomic_fetch_and_xor, so don't worry about it. */ > - tcg_gen_brcond_tl(TCG_COND_NE, dat, src2, l1); > - } else { > - /* Serial context. Directly store the result. */ > - tcg_gen_qemu_st_tl(src2, src1, ctx->mem_idx, mop); > - } > - gen_set_gpr(rd, dat); > - tcg_temp_free(dat); > + tcg_gen_atomic_fetch_smin_tl(src2, src1, src2, ctx->mem_idx, > mop); > + gen_set_gpr(rd, src2); > + break; > + case OPC_RISC_AMOMAX: > + gen_get_gpr(src1, rs1); > + gen_get_gpr(src2, rs2); > + tcg_gen_atomic_fetch_smax_tl(src2, src1, src2, ctx->mem_idx, > mop); > + gen_set_gpr(rd, src2); > + break; > + case OPC_RISC_AMOMINU: > + gen_get_gpr(src1, rs1); > + gen_get_gpr(src2, rs2); > + tcg_gen_atomic_fetch_umin_tl(src2, src1, src2, ctx->mem_idx, > mop); > + gen_set_gpr(rd, src2); > + break; > + case OPC_RISC_AMOMAXU: > + gen_get_gpr(src1, rs1); > + gen_get_gpr(src2, rs2); > + tcg_gen_atomic_fetch_umax_tl(src2, src1, src2, ctx->mem_idx, > mop); > + gen_set_gpr(rd, src2); > break; > > default: > -- > 2.14.3 > >
On 04/26/2018 09:24 PM, Michael Clark wrote: > > > On Fri, Apr 27, 2018 at 12:26 PM, Richard Henderson > <richard.henderson@linaro.org <mailto:richard.henderson@linaro.org>> wrote: > > Cc: Michael Clark <mjc@sifive.com <mailto:mjc@sifive.com>> > Cc: Palmer Dabbelt <palmer@sifive.com <mailto:palmer@sifive.com>> > Cc: Sagar Karandikar <sagark@eecs.berkeley.edu > <mailto:sagark@eecs.berkeley.edu>> > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de > <mailto:kbastian@mail.uni-paderborn.de>> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org > <mailto:richard.henderson@linaro.org>> > > > Reviewed-by: Michael Clark <mjc@sifive.com <mailto:mjc@sifive.com>> > > If you give me a remote I can pull into my local workspace and test. The change > looks pretty straight-forward. There are tests for amos in riscv-tests but no > parallel tests (i.e. contended case). git://github.com/rth7680/qemu.git tgt-arm-atomic-2 r~
On Sat, Apr 28, 2018 at 5:53 AM, Richard Henderson < richard.henderson@linaro.org> wrote: > On 04/26/2018 09:24 PM, Michael Clark wrote: > > > > > > On Fri, Apr 27, 2018 at 12:26 PM, Richard Henderson > > <richard.henderson@linaro.org <mailto:richard.henderson@linaro.org>> > wrote: > > > > Cc: Michael Clark <mjc@sifive.com <mailto:mjc@sifive.com>> > > Cc: Palmer Dabbelt <palmer@sifive.com <mailto:palmer@sifive.com>> > > Cc: Sagar Karandikar <sagark@eecs.berkeley.edu > > <mailto:sagark@eecs.berkeley.edu>> > > Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de > > <mailto:kbastian@mail.uni-paderborn.de>> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org > > <mailto:richard.henderson@linaro.org>> > > > > > > Reviewed-by: Michael Clark <mjc@sifive.com <mailto:mjc@sifive.com>> > > > > If you give me a remote I can pull into my local workspace and test. The > change > > looks pretty straight-forward. There are tests for amos in riscv-tests > but no > > parallel tests (i.e. contended case). > > git://github.com/rth7680/qemu.git tgt-arm-atomic-2 I just did a fetch and only saw tgt-arm-atomic. Maybe you haven't pushed tgt-arm-atomic-2. In any case the tgt-arm-atomic branch passes riscv-tests. Michael.
diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 808eab7f50..9cab717088 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -725,7 +725,6 @@ static void gen_atomic(DisasContext *ctx, uint32_t opc, TCGv src1, src2, dat; TCGLabel *l1, *l2; TCGMemOp mop; - TCGCond cond; bool aq, rl; /* Extract the size of the atomic operation. */ @@ -823,60 +822,29 @@ static void gen_atomic(DisasContext *ctx, uint32_t opc, tcg_gen_atomic_fetch_or_tl(src2, src1, src2, ctx->mem_idx, mop); gen_set_gpr(rd, src2); break; - case OPC_RISC_AMOMIN: - cond = TCG_COND_LT; - goto do_minmax; - case OPC_RISC_AMOMAX: - cond = TCG_COND_GT; - goto do_minmax; - case OPC_RISC_AMOMINU: - cond = TCG_COND_LTU; - goto do_minmax; - case OPC_RISC_AMOMAXU: - cond = TCG_COND_GTU; - goto do_minmax; - do_minmax: - /* Handle the RL barrier. The AQ barrier is handled along the - parallel path by the SC atomic cmpxchg. On the serial path, - of course, barriers do not matter. */ - if (rl) { - tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); - } - if (tb_cflags(ctx->tb) & CF_PARALLEL) { - l1 = gen_new_label(); - gen_set_label(l1); - } else { - l1 = NULL; - } - gen_get_gpr(src1, rs1); gen_get_gpr(src2, rs2); - if ((mop & MO_SSIZE) == MO_SL) { - /* Sign-extend the register comparison input. */ - tcg_gen_ext32s_tl(src2, src2); - } - dat = tcg_temp_local_new(); - tcg_gen_qemu_ld_tl(dat, src1, ctx->mem_idx, mop); - tcg_gen_movcond_tl(cond, src2, dat, src2, dat, src2); - - if (tb_cflags(ctx->tb) & CF_PARALLEL) { - /* Parallel context. Make this operation atomic by verifying - that the memory didn't change while we computed the result. */ - tcg_gen_atomic_cmpxchg_tl(src2, src1, dat, src2, ctx->mem_idx, mop); - - /* If the cmpxchg failed, retry. */ - /* ??? There is an assumption here that this will eventually - succeed, such that we don't live-lock. This is not unlike - a similar loop that the compiler would generate for e.g. - __atomic_fetch_and_xor, so don't worry about it. */ - tcg_gen_brcond_tl(TCG_COND_NE, dat, src2, l1); - } else { - /* Serial context. Directly store the result. */ - tcg_gen_qemu_st_tl(src2, src1, ctx->mem_idx, mop); - } - gen_set_gpr(rd, dat); - tcg_temp_free(dat); + tcg_gen_atomic_fetch_smin_tl(src2, src1, src2, ctx->mem_idx, mop); + gen_set_gpr(rd, src2); + break; + case OPC_RISC_AMOMAX: + gen_get_gpr(src1, rs1); + gen_get_gpr(src2, rs2); + tcg_gen_atomic_fetch_smax_tl(src2, src1, src2, ctx->mem_idx, mop); + gen_set_gpr(rd, src2); + break; + case OPC_RISC_AMOMINU: + gen_get_gpr(src1, rs1); + gen_get_gpr(src2, rs2); + tcg_gen_atomic_fetch_umin_tl(src2, src1, src2, ctx->mem_idx, mop); + gen_set_gpr(rd, src2); + break; + case OPC_RISC_AMOMAXU: + gen_get_gpr(src1, rs1); + gen_get_gpr(src2, rs2); + tcg_gen_atomic_fetch_umax_tl(src2, src1, src2, ctx->mem_idx, mop); + gen_set_gpr(rd, src2); break; default:
Cc: Michael Clark <mjc@sifive.com> Cc: Palmer Dabbelt <palmer@sifive.com> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/riscv/translate.c | 72 ++++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 52 deletions(-) -- 2.14.3