Re: [Qemu-devel] [PATCH 10/21] target-arm: A64: Implement remaining non-pairwise int SIMD 3-reg-same insns

[Richard Henderson]

On 01/26/2014 11:25 AM, Peter Maydell wrote:
> @@ -6773,9 +6801,9 @@ static void disas_simd_3same_int(DisasContext *s, 
> uint32_t insn)
>              case 0x8: /* SSHL, USHL */
>              {
>                  static NeonGenTwoOpFn * const fns[3][2] = {
> -                    { gen_helper_neon_shl_u8, gen_helper_neon_shl_s8 },
> -                    { gen_helper_neon_shl_u16, gen_helper_neon_shl_s16 },
> -                    { gen_helper_neon_shl_u32, gen_helper_neon_shl_s32 },
> +                    { gen_helper_neon_shl_s8, gen_helper_neon_shl_u8 },
> +                    { gen_helper_neon_shl_s16, gen_helper_neon_shl_u16 },
> +                    { gen_helper_neon_shl_s32, gen_helper_neon_shl_u32 },
>                  };
>                  genfn = fns[size][u];
>                  break;

Oops, I clearly missed this in the review of the previous patch.
But these bug fixes ought to be folded into the previous.

Otherwise,

Reviewed-by: Richard Henderson <address@hidden>

> +            case 0xd: /* SMIN, UMIN */
> +            {
> +                static NeonGenTwoOpFn * const fns[3][2] = {
> +                    { gen_helper_neon_min_s8, gen_helper_neon_min_u8 },
> +                    { gen_helper_neon_min_s16, gen_helper_neon_min_u16 },
> +                    { gen_helper_neon_min_s32, gen_helper_neon_min_u32 },
> +                };
> +                genfn = fns[size][u];
> +                break;

Out of curiosity, what logic are you applying to using the neon helper
functions and implementing stuff inline?

It appears that you're implementing all of the 64-bit stuff inline, but using
the existing 32-bit helpers, even if the helpers are remarkably small, like
some of these.


r~