Re: [PATCH v2 12/57] target/arm: Implement widening/narrowing MVE VLDR/VSTR insns

[Richard Henderson]

On 6/14/21 8:09 AM, Peter Maydell wrote:
> diff --git a/target/arm/mve_helper.c b/target/arm/mve_helper.c
> index c07110a145b..7282f56b18d 100644
> --- a/target/arm/mve_helper.c
> +++ b/target/arm/mve_helper.c
> @@ -168,5 +168,16 @@ DO_VSTR(vstrb, stb, 1, uint8_t)
>   DO_VSTR(vstrh, stw, 2, uint16_t)
>   DO_VSTR(vstrw, stl, 4, uint32_t)
>   
> +DO_VLDR(vldrb_sh, ldsb, 2, int16_t)
> +DO_VLDR(vldrb_sw, ldsb, 4, int32_t)
> +DO_VLDR(vldrb_uh, ldub, 2, uint16_t)
> +DO_VLDR(vldrb_uw, ldub, 4, uint32_t)
> +DO_VLDR(vldrh_sw, ldsw, 4, int32_t)
> +DO_VLDR(vldrh_uw, lduw, 4, uint32_t)
> +
> +DO_VSTR(vstrb_h, stb, 2, int16_t)
> +DO_VSTR(vstrb_w, stb, 4, int32_t)
> +DO_VSTR(vstrh_w, stw, 4, int32_t)

Hmm, from the previous patch,

> +#define DO_VLDR(OP, LDTYPE, ESIZE, TYPE)                                \
> +    void HELPER(mve_##OP)(CPUARMState *env, void *vd, uint32_t addr)    \
> +    {                                                                   \
> +        TYPE *d = vd;                                                   \
> +        uint16_t mask = mve_element_mask(env);                          \
> +        unsigned b, e;                                                  \
> +        /*                                                              \
> +         * R_SXTM allows the dest reg to become UNKNOWN for abandoned   \
> +         * beats so we don't care if we update part of the dest and     \
> +         * then take an exception.                                      \
> +         */                                                             \
> +        for (b = 0, e = 0; b < 16; b += ESIZE, e++) {                   \
> +            if (mask & (1 << b)) {                                      \
> +                d[H##ESIZE(e)] = cpu_##LDTYPE##_data_ra(env, addr, GETPC()); \
> +                addr += ESIZE;                                          \
> +            }                                                           \
> +        }                                                               \
> +        mve_advance_vpt(env);                                           \
> +    }

we are advancing ADDR by ESIZE, and not but the MSIZE of the memory access.
Further, we're advancing ADDR under predication, whereas the increment should be 
unconditional (e * mbytes).


r~