Re: [PATCH] tcg/i386: Bound shift count expanding sari_vec

[Richard Henderson]

Ping.

On 3/10/20 10:21 PM, Richard Henderson wrote:
> A given RISU testcase for SVE can produce
> 
> tcg-op-vec.c:511: do_shifti: Assertion `i >= 0 && i < (8 << vece)' failed.
> 
> because expand_vec_sari gave a shift count of 32 to a MO_32
> vector shift.
> 
> In 44f1441dbe1, we changed from direct expansion of vector opcodes
> to re-use of the tcg expanders.  So while the comment correctly notes
> that the hw will handle such a shift count, we now have to take our
> own sanity checks into account.  Which is easy in this particular case.
> 
> Fixes: 44f1441dbe1
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  tcg/i386/tcg-target.inc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
> index cdedcb2b25..223dba9c8c 100644
> --- a/tcg/i386/tcg-target.inc.c
> +++ b/tcg/i386/tcg-target.inc.c
> @@ -3391,12 +3391,15 @@ static void expand_vec_sari(TCGType type, unsigned 
> vece,
>  
>      case MO_64:
>          if (imm <= 32) {
> -            /* We can emulate a small sign extend by performing an arithmetic
> +            /*
> +             * We can emulate a small sign extend by performing an arithmetic
>               * 32-bit shift and overwriting the high half of a 64-bit logical
> -             * shift (note that the ISA says shift of 32 is valid).
> +             * shift.  Note that the ISA says shift of 32 is valid, but TCG
> +             * does not, so we have to bound the smaller shift -- we get the
> +             * same result in the high half either way.
>               */
>              t1 = tcg_temp_new_vec(type);
> -            tcg_gen_sari_vec(MO_32, t1, v1, imm);
> +            tcg_gen_sari_vec(MO_32, t1, v1, MIN(imm, 31));
>              tcg_gen_shri_vec(MO_64, v0, v1, imm);
>              vec_gen_4(INDEX_op_x86_blend_vec, type, MO_32,
>                        tcgv_vec_arg(v0), tcgv_vec_arg(v0),
>