Re: [PATCH v4 05/16] cputlb: Split out load/store_memop

[David Hildenbrand <david@redhat.com>]

On 24.09.19 00:59, Richard Henderson wrote:
> We will shortly be using these more than once.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/cputlb.c | 110 +++++++++++++++++++++++----------------------
>  1 file changed, 57 insertions(+), 53 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e529af6d09..430ba4a69d 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1281,6 +1281,29 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
>  typedef uint64_t FullLoadHelper(CPUArchState *env, target_ulong addr,
>                                  TCGMemOpIdx oi, uintptr_t retaddr);
>  
> +static inline uint64_t QEMU_ALWAYS_INLINE
> +load_memop(const void *haddr, MemOp op)
> +{
> +    switch (op) {
> +    case MO_UB:
> +        return ldub_p(haddr);
> +    case MO_BEUW:
> +        return lduw_be_p(haddr);
> +    case MO_LEUW:
> +        return lduw_le_p(haddr);
> +    case MO_BEUL:
> +        return (uint32_t)ldl_be_p(haddr);
> +    case MO_LEUL:
> +        return (uint32_t)ldl_le_p(haddr);
> +    case MO_BEQ:
> +        return ldq_be_p(haddr);
> +    case MO_LEQ:
> +        return ldq_le_p(haddr);
> +    default:
> +        optimize_away();
> +    }
> +}
> +
>  static inline uint64_t QEMU_ALWAYS_INLINE
>  load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
>              uintptr_t retaddr, MemOp op, bool code_read,
> @@ -1373,33 +1396,7 @@ load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
>  
>   do_aligned_access:
>      haddr = (void *)((uintptr_t)addr + entry->addend);
> -    switch (op) {
> -    case MO_UB:
> -        res = ldub_p(haddr);
> -        break;
> -    case MO_BEUW:
> -        res = lduw_be_p(haddr);
> -        break;
> -    case MO_LEUW:
> -        res = lduw_le_p(haddr);
> -        break;
> -    case MO_BEUL:
> -        res = (uint32_t)ldl_be_p(haddr);
> -        break;
> -    case MO_LEUL:
> -        res = (uint32_t)ldl_le_p(haddr);
> -        break;
> -    case MO_BEQ:
> -        res = ldq_be_p(haddr);
> -        break;
> -    case MO_LEQ:
> -        res = ldq_le_p(haddr);
> -        break;
> -    default:
> -        optimize_away();
> -    }
> -
> -    return res;
> +    return load_memop(haddr, op);
>  }
>  
>  /*
> @@ -1415,7 +1412,8 @@ load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
>  static uint64_t full_ldub_mmu(CPUArchState *env, target_ulong addr,
>                                TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> -    return load_helper(env, addr, oi, retaddr, MO_UB, false, full_ldub_mmu);
> +    return load_helper(env, addr, oi, retaddr, MO_UB, false,
> +                       full_ldub_mmu);

Unnecessary change.


Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb