Re: [Qemu-devel] [PATCH 18/20] target/arm: Rewrite vector gather stores

[Peter Maydell]

On 9 August 2018 at 05:22, Richard Henderson
<address@hidden> wrote:
> This fixes the endianness problem for softmmu, and does
> move the main loop out of a macro and into an inlined function.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/helper-sve.h    |  52 ++++++++++----
>  target/arm/sve_helper.c    | 139 ++++++++++++++++++++++++-------------
>  target/arm/translate-sve.c |  74 +++++++++++++-------
>  3 files changed, 177 insertions(+), 88 deletions(-)
>

> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 76d3f021e4..0a4756bff9 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -5235,61 +5235,100 @@ DO_LDFF1_ZPZ_D(sve_ldffsds_zd, uint64_t, int32_t,  
> cpu_ldl_data_ra)
>
>  /* Stores with a vector index.  */
>
> -#define DO_ST1_ZPZ_S(NAME, TYPEI, FN)                                   \
> -void HELPER(NAME)(CPUARMState *env, void *vd, void *vg, void *vm,       \
> -                  target_ulong base, uint32_t desc)                     \
> -{                                                                       \
> -    intptr_t i, oprsz = simd_oprsz(desc);                               \
> -    unsigned scale = simd_data(desc);                                   \
> -    uintptr_t ra = GETPC();                                             \
> -    for (i = 0; i < oprsz; ) {                                          \
> -        uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));                 \
> -        do {                                                            \
> -            if (likely(pg & 1)) {                                       \
> -                target_ulong off = *(TYPEI *)(vm + H1_4(i));            \
> -                uint32_t d = *(uint32_t *)(vd + H1_4(i));               \
> -                FN(env, base + (off << scale), d, ra);                  \
> -            }                                                           \
> -            i += sizeof(uint32_t), pg >>= sizeof(uint32_t);             \
> -        } while (i & 15);                                               \
> -    }                                                                   \
> +static void sve_st1_zs(CPUARMState *env, void *vd, void *vg, void *vm,
> +                       target_ulong base, uint32_t desc, uintptr_t ra,
> +                       zreg_off_fn *off_fn, sve_ld1_tlb_fn *tlb_fn)
> +{
> +    const int mmu_idx = cpu_mmu_index(env, false);
> +    intptr_t i, oprsz = simd_oprsz(desc);
> +    unsigned scale = simd_data(desc);
> +
> +    set_helper_retaddr(ra);
> +    for (i = 0; i < oprsz; ) {
> +        uint16_t pg = *(uint16_t *)(vg + H1_2(i >> 3));
> +        do {
> +            if (pg & 1) {

Is dropping the "likely()" off this conditional intentional ?


> +                target_ulong off = off_fn(vm, i);
> +                tlb_fn(env, vd, i, base + (off << scale), mmu_idx, ra);
> +            }
> +            i += 4, pg >>= 4;
> +        } while (i & 15);
> +    }
> +    set_helper_retaddr(0);
>  }
>
Either way,
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM