Re: [Qemu-devel] [PATCH 11/20] target/arm: Clear unused predicate bits for LD1RQ

[Peter Maydell]

On 9 August 2018 at 05:21, Richard Henderson
<address@hidden> wrote:
> The 16-byte load only uses 16 predicate bits.  But while
> reusing the other load infrastructure, we find other bits
> that are set and trigger an assert.  To avoid this and
> retain the assert, zero-extend the predicate that we pass
> to the LD1 helper.
>
> Reported-by: Laurent Desnogues <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate-sve.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
> index d27bc8c946..bef6b8242d 100644
> --- a/target/arm/translate-sve.c
> +++ b/target/arm/translate-sve.c
> @@ -4765,12 +4765,33 @@ static void do_ldrq(DisasContext *s, int zt, int pg, 
> TCGv_i64 addr, int msz)
>      unsigned vsz = vec_full_reg_size(s);
>      TCGv_ptr t_pg;
>      TCGv_i32 desc;
> +    int poff;
>
>      /* Load the first quadword using the normal predicated load helpers.  */
>      desc = tcg_const_i32(simd_desc(16, 16, zt));
> -    t_pg = tcg_temp_new_ptr();
>
> -    tcg_gen_addi_ptr(t_pg, cpu_env, pred_full_reg_offset(s, pg));
> +    poff = pred_full_reg_offset(s, pg);
> +    if (vsz > 16) {
> +        /*
> +         * Zero-extend the first 16 bits of the predicate into a temporary.
> +         * This avoids triggering an assert making sure we don't have bits
> +         * set within a predicate beyond VQ, but we have lowered VQ to 1
> +         * for this load operation.
> +         */
> +        TCGv_i64 tmp = tcg_temp_new_i64();
> +#ifdef HOST_WORDS_BIGENDIAN
> +        poff += 6;
> +#endif
> +        tcg_gen_ld16u_i64(tmp, cpu_env, poff);
> +
> +        poff = offsetof(CPUARMState, vfp.preg_tmp);
> +        tcg_gen_st_i64(tmp, cpu_env, poff);
> +        tcg_temp_free_i64(tmp);
> +    }
> +
> +    t_pg = tcg_temp_new_ptr();
> +    tcg_gen_addi_ptr(t_pg, cpu_env, poff);

Reviewed-by: Peter Maydell <address@hidden>

The bigendian #ifdef in the middle of the code is a little
ugly, though -- I don't suppose it's possible to avoid it
(or abstract it away) somehow?

thanks
-- PMM