Re: [PATCH v2 14/24] target/arm: Enforce alignment for VLD1 (all lanes)

[Peter Maydell]

On Tue, 8 Dec 2020 at 18:01, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate-neon.c.inc | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Patch subject says "VLD1", but trans_VLD_all_lanes handles all of
the "VLDn to all lanes" insns.

> diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc
> index 18d9042130..7cb89b18e0 100644
> --- a/target/arm/translate-neon.c.inc
> +++ b/target/arm/translate-neon.c.inc
> @@ -522,6 +522,7 @@ static bool trans_VLD_all_lanes(DisasContext *s, 
> arg_VLD_all_lanes *a)
>      int size = a->size;
>      int nregs = a->n + 1;
>      TCGv_i32 addr, tmp;
> +    MemOp mop;
>
>      if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
>          return false;
> @@ -556,12 +557,12 @@ static bool trans_VLD_all_lanes(DisasContext *s, 
> arg_VLD_all_lanes *a)
>       */
>      stride = a->t ? 2 : 1;
>      vec_size = nregs == 1 ? stride * 8 : 8;
> -
> +    mop = size | (a->a ? MO_ALIGN : 0);
>      tmp = tcg_temp_new_i32();
>      addr = tcg_temp_new_i32();
>      load_reg_var(s, addr, a->rn);
>      for (reg = 0; reg < nregs; reg++) {
> -        gen_aa32_ld_i32(s, tmp, addr, get_mem_index(s), size);
> +        gen_aa32_ld_i32(s, tmp, addr, get_mem_index(s), mop);
>          if ((vd & 1) && vec_size == 16) {
>              /*
>               * We cannot write 16 bytes at once because the

I think this code is correct for VLD1 (A bit means "address must
be aligned to the size of the element") and VLD3 (A bit is always 0),
but not for VLD2 (A bit means "address must be aligned to 2* size
of element") or VLD4 (A bit means "address must be aligned to 4* size",
and there is a special case for size==3 meaning "32 bits at 16-byte
alignment").

thanks
-- PMM