Re: [PATCH 16/27] accel/tcg: Unify cpu_{ld,st}*_{be,le}_mmu

[Philippe Mathieu-Daudé]

Hi Richard,

On 20/5/23 18:26, Richard Henderson wrote:
> With the current structure of cputlb.c, there is no difference
> between the little-endian and big-endian entry points, aside
> from the assert.  Unify the pairs of functions.
>
> The only use of the functions with explicit endianness was in
> target/sparc64, and that was only to satisfy the assert.

I'm having hard time to follow all the handling of the various
ASI definitions from target/sparc/asi.h. ...

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/cpu_ldst.h     |  58 ++-----
>   accel/tcg/cputlb.c          | 122 +++-----------
>   accel/tcg/user-exec.c       | 322 ++++++++++--------------------------
>   target/arm/tcg/m_helper.c   |   4 +-
>   target/sparc/ldst_helper.c  |  18 +-
>   accel/tcg/ldst_common.c.inc |  24 +--
>   6 files changed, 137 insertions(+), 411 deletions(-)


> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index 7972d56a72..981a47d8bb 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -1334,25 +1334,13 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
> addr,


Shouldn't we propagate the ASI endianness?

                ...
  +             memop |= (asi & 8) ? MO_LE : MO_BE;
                oi = make_memop_idx(memop, idx);
                switch (size) {
                case 1:

>                   ret = cpu_ldb_mmu(env, addr, oi, GETPC());
>                   break;
>               case 2:
> -                if (asi & 8) {
> -                    ret = cpu_ldw_le_mmu(env, addr, oi, GETPC());
> -                } else {
> -                    ret = cpu_ldw_be_mmu(env, addr, oi, GETPC());
> -                }
> +                ret = cpu_ldw_mmu(env, addr, oi, GETPC());
>                   break;
>               case 4:
> -                if (asi & 8) {
> -                    ret = cpu_ldl_le_mmu(env, addr, oi, GETPC());
> -                } else {
> -                    ret = cpu_ldl_be_mmu(env, addr, oi, GETPC());
> -                }
> +                ret = cpu_ldl_mmu(env, addr, oi, GETPC());
>                   break;
>               case 8:
> -                if (asi & 8) {
> -                    ret = cpu_ldq_le_mmu(env, addr, oi, GETPC());
> -                } else {
> -                    ret = cpu_ldq_be_mmu(env, addr, oi, GETPC());
> -                }
> +                ret = cpu_ldq_mmu(env, addr, oi, GETPC());
>                   break;
>               default:
>                   g_assert_not_reached();
Otherwise great simplification!