Re: [PULL 17/45] target/arm: Vectorize SABA/UABA

[Peter Maydell]

On Thu, 14 May 2020 at 15:22, Peter Maydell <address@hidden> wrote:
>
> From: Richard Henderson <address@hidden>
>
> Include 64-bit element size in preparation for SVE2.

Hi; Coverity points out that after this commit there is
dead code in disas_simd_3same_int():

> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 54b06553a65..991e451644c 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -11197,6 +11197,13 @@ static void disas_simd_3same_int(DisasContext *s, 
> uint32_t insn)
>              gen_gvec_fn3(s, is_q, rd, rn, rm, gen_gvec_sabd, size);
>          }
>          return;
> +    case 0xf: /* SABA, UABA */
> +        if (u) {
> +            gen_gvec_fn3(s, is_q, rd, rn, rm, gen_gvec_uaba, size);
> +        } else {
> +            gen_gvec_fn3(s, is_q, rd, rn, rm, gen_gvec_saba, size);
> +        }
> +        return;

Here case 0xf is handled entirely and we return early...

>      case 0x10: /* ADD, SUB */
>          if (u) {
>              gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_sub, size);
> @@ -11329,16 +11336,6 @@ static void disas_simd_3same_int(DisasContext *s, 
> uint32_t insn)
>                  genenvfn = fns[size][u];
>                  break;
>              }
> -            case 0xf: /* SABA, UABA */
> -            {
> -                static NeonGenTwoOpFn * const fns[3][2] = {
> -                    { gen_helper_neon_abd_s8, gen_helper_neon_abd_u8 },
> -                    { gen_helper_neon_abd_s16, gen_helper_neon_abd_u16 },
> -                    { gen_helper_neon_abd_s32, gen_helper_neon_abd_u32 },
> -                };
> -                genfn = fns[size][u];
> -                break;
> -            }

...and we did delete the "do the operation" code...

>              case 0x16: /* SQDMULH, SQRDMULH */
>              {
>                  static NeonGenTwoOpEnvFn * const fns[2][2] = {

...but we missed the handling of the accumulate part near the
bottom of the loop:

            if (opcode == 0xf) {
                /* SABA, UABA: accumulating ops */
                static NeonGenTwoOpFn * const fns[3] = {
                    gen_helper_neon_add_u8,
                    gen_helper_neon_add_u16,
                    tcg_gen_add_i32,
                };

                read_vec_element_i32(s, tcg_op1, rd, pass, MO_32);
                fns[size](tcg_res, tcg_op1, tcg_res);
            }

That whole if() is now dead and can be deleted. Richard, do
you want to send a patch?

thanks
-- PMM