Re: [Qemu-devel] [PATCH v5 13/35] target/arm: Implement SVE gather loads

[Peter Maydell]

On 21 June 2018 at 02:53, Richard Henderson
<address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/helper-sve.h    |  67 ++++++++++++++++++++++++
>  target/arm/sve_helper.c    |  77 +++++++++++++++++++++++++++
>  target/arm/translate-sve.c | 104 +++++++++++++++++++++++++++++++++++++
>  target/arm/sve.decode      |  53 +++++++++++++++++++
>  4 files changed, 301 insertions(+)

> +static bool trans_LD1_zprz(DisasContext *s, arg_LD1_zprz *a, uint32_t insn)
> +{
> +    gen_helper_gvec_mem_scatter *fn = NULL;
> +
> +    if (a->esz < a->msz
> +        || (a->msz == 0 && a->scale)

Doesn't this check duplicate work you're already doing in the decode
entries in the .decode file? If we're going to throw out msz==0 scale==1
here we don't need to go to the effort to split this into 4 patterns
so we can force the scale bit to 0 in the msz=00 line:

> +LD1_zprz        1100010 00 .0 ..... 0.. ... ..... ..... \
> +                @rprr_g_load_xs_u esz=3 msz=0 scale=0
> +LD1_zprz        1100010 01 .. ..... 0.. ... ..... ..... \
> +                @rprr_g_load_xs_u_sc esz=3 msz=1
> +LD1_zprz        1100010 10 .. ..... 0.. ... ..... ..... \
> +                @rprr_g_load_xs_u_sc esz=3 msz=2
> +LD1_zprz        1100010 11 .. ..... 0.. ... ..... ..... \
> +                @rprr_g_load_xs_u_sc esz=3 msz=3

and could just have
LD1_zprz     1100010 msz:2 .. ..... 0.. ... ..... ..... \
                @rprr_g_load_xs_u_sc esz=3 scale=0

couldn't we?

(I don't really care which way round we do it.)

> +        || (a->esz == a->msz && !a->u)) {
> +        return false;
> +    }
> +    if (!sve_access_check(s)) {
> +        return true;
> +    }
> +
> +    /* TODO: handle LDFF1.  */

Is LDFF1 support going to appear later in this patchset?

For the moment should we UNDEF it?

> +    switch (a->esz) {
> +    case MO_32:
> +        fn = gather_load_fn32[a->xs][a->u][a->msz];
> +        break;
> +    case MO_64:
> +        fn = gather_load_fn64[a->xs][a->u][a->msz];
> +        break;
> +    }
> +    assert(fn != NULL);
> +
> +    do_mem_zpz(s, a->rd, a->pg, a->rm, a->scale * a->msz,
> +               cpu_reg_sp(s, a->rn), fn);
> +    return true;
> +}

thanks
-- PMM