Re: [PATCH v5 01/54] include/exec/memop: Add MO_ATOM_*

[Peter Maydell]

On Mon, 15 May 2023 at 15:34, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This field may be used to describe the precise atomicity requirements
> of the guest, which may then be used to constrain the methods by which
> it may be emulated by the host.


> @@ -2330,17 +2339,23 @@ static void tcg_dump_ops(TCGContext *s, FILE *f, bool 
> have_prefs)
>              case INDEX_op_qemu_ld_i64:
>              case INDEX_op_qemu_st_i64:
>                  {
> +                    const char *s_al, *s_op, *s_at = NULL;

Do we need this NULL initializer? We seem to immediately assign
to s_at below, same as we do for s_al and s_op.

>                      MemOpIdx oi = op->args[k++];
>                      MemOp op = get_memop(oi);
>                      unsigned ix = get_mmuidx(oi);
>
> -                    if (op & ~(MO_AMASK | MO_BSWAP | MO_SSIZE)) {
> -                        col += ne_fprintf(f, ",$0x%x,%u", op, ix);
> +                    s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT];
> +                    s_op = ldst_name[op & (MO_BSWAP | MO_SSIZE)];
> +                    s_at = atom_name[(op & MO_ATOM_MASK) >> MO_ATOM_SHIFT];
> +                    op &= ~(MO_AMASK | MO_BSWAP | MO_SSIZE | MO_ATOM_MASK);
> +
> +                    /* If all fields are accounted for, print symbolically. 
> */
> +                    if (!op && s_al && s_op && s_at) {
> +                        col += ne_fprintf(f, ",%s%s%s,%u",
> +                                          s_at, s_al, s_op, ix);
>                      } else {
> -                        const char *s_al, *s_op;
> -                        s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT];
> -                        s_op = ldst_name[op & (MO_BSWAP | MO_SSIZE)];
> -                        col += ne_fprintf(f, ",%s%s,%u", s_al, s_op, ix);
> +                        op = get_memop(oi);
> +                        col += ne_fprintf(f, ",$0x%x,%u", op, ix);
>                      }
>                      i = 1;
>                  }

Otherwise

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM