Re: [PATCH v4 08/21] target/loongarch: Add fixed point atomic instruction translation

[Richard Henderson <richard.henderson@linaro.org>]

On 9/2/21 2:40 PM, Song Gao wrote:
> +static bool gen_sc(DisasContext *ctx, arg_fmt_rdrjsi14 *a, MemOp mop)
> +{
> +    TCGv dest = gpr_dst(ctx, a->rd);
> +    TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
> +    TCGv src2 = gpr_src(ctx, a->rd, EXT_NONE);
> +    TCGv t0 = tcg_temp_new();
> +
> +    TCGLabel *l1 = gen_new_label();
> +    TCGLabel *done = gen_new_label();
> +
> +    tcg_gen_addi_tl(t0, src1, a->si14 << 2);
> +    tcg_gen_brcond_tl(TCG_COND_EQ, t0, cpu_lladdr, l1);
> +    tcg_gen_movi_tl(dest, 0);
> +    tcg_gen_br(done);
> +
> +    gen_set_label(l1);
> +    /* generate cmpxchg */
> +    tcg_gen_atomic_cmpxchg_tl(t0, cpu_lladdr, cpu_llval,
> +                              src2, ctx->mem_idx, mop);

This is incorrect when dest and src2 overlap, as you've already clobbered dest.

> +static bool gen_am(DisasContext *ctx, arg_fmt_rdrjrk *a, DisasExtend ext,
> +                   void (*func)(TCGv, TCGv, TCGv, TCGArg, MemOp),
> +                   MemOp mop)
> +{
> +    ctx->dst_ext = ext;
> +    TCGv dest = gpr_dst(ctx, a->rd);
> +    TCGv addr = gpr_src(ctx, a->rj, EXT_NONE);
> +    TCGv val = gpr_src(ctx, a->rk, EXT_NONE);
> +
> +    if ((a->rd != 0) && ((a->rj == a->rd) || (a->rk == a->rd))) {
> +        qemu_log("%s: waring, register equal\n", __func__);
> +        return false;
> +    }
> +
> +    func(dest, addr, val, ctx->mem_idx, mop);
> +
> +    if (ctx->dst_ext) {
> +        gen_set_gpr(ctx, a->rd, dest);
> +    }
> +    return true;
> +}
> +
> +static bool gen_am_db(DisasContext *ctx, arg_fmt_rdrjrk *a, DisasExtend ext,
> +                      void (*func)(TCGv, TCGv, TCGv, TCGArg, MemOp),
> +                      MemOp mop)
> +{
> +    ctx->dst_ext = ext;
> +    TCGv dest = gpr_dst(ctx, a->rd);
> +    TCGv addr = gpr_src(ctx, a->rj, EXT_NONE);
> +    TCGv val = gpr_src(ctx, a->rk, EXT_NONE);
> +
> +    if ((a->rd != 0) && ((a->rj == a->rd) || (a->rk == a->rd))) {
> +        qemu_log("%s: waring, register equal\n", __func__);

qemu_log_mask(LOG_GUEST_ERROR,
               "Warning: source register overlaps destination register"
               "in atomic insn at pc=0x" TARGET_FMT_lx "\n",
               ctx->base.pc_next - 4);

> +    if (ctx->dst_ext) {
> +        gen_set_gpr(ctx, a->rd, dest);
> +    }

Again, extension should be taken care of by mop.


r~