Re: [PATCH 13/84] tcg: Remove TCGv from tcg_gen_atomic_*

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Expand from TCGv to TCGTemp inline in the translators,
> and validate that the size matches tcg_ctx->addr_type.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/tcg/tcg-op.h | 184 ++++++++++++++++++++++++++++++----------
>  tcg/tcg-op-ldst.c    | 198 ++++++++++++++++++++++++++++---------------
>  2 files changed, 267 insertions(+), 115 deletions(-)
>
<snip>
> diff --git a/tcg/tcg-op-ldst.c b/tcg/tcg-op-ldst.c
> index a94a70e8c4..4624b0a25b 100644
> --- a/tcg/tcg-op-ldst.c
> +++ b/tcg/tcg-op-ldst.c
<snip>
>  
> -static void do_atomic_op_i64(TCGv_i64 ret, TCGv addr, TCGv_i64 val,
> +static void do_atomic_op_i64(TCGv_i64 ret, TCGTemp *addr, TCGv_i64 val,
>                               TCGArg idx, MemOp memop, void * const table[])
>  {
>      memop = tcg_canonicalize_memop(memop, 1, 0);
>  
>      if ((memop & MO_SIZE) == MO_64) {
> -#ifdef CONFIG_ATOMIC64

The commit message could briefly mention the table expansion is
controlled by CONFIG_ATOMIC64 so why we don't check it here.

> -        gen_atomic_op_i64 gen;
> -        TCGv_i64 a64;
> -        MemOpIdx oi;
> +        gen_atomic_op_i64 gen = table[memop & (MO_SIZE | MO_BSWAP)];
>  
> -        gen = table[memop & (MO_SIZE | MO_BSWAP)];
> -        tcg_debug_assert(gen != NULL);
> +        if (gen) {
> +            MemOpIdx oi = make_memop_idx(memop & ~MO_SIGN, idx);
> +            TCGv_i64 a64 = maybe_extend_addr64(addr);
> +            gen(ret, cpu_env, a64, val, tcg_constant_i32(oi));
> +            maybe_free_addr64(a64);
> +            return;
> +        }

personal preference nit, we don't need an early return, you could just
hoist into the else leg.


Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro