Re: [PATCH v4 10/48] target/loongarch: rename lsx_helper.c to vec_helper.c

[Richard Henderson]

On 8/30/23 01:48, Song Gao wrote:
> Use gen_helper_gvec_* series function.
> and rename lsx_helper.c to vec_helper.c.
>
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   target/loongarch/helper.h                     |  642 ++++----
>   .../loongarch/{lsx_helper.c => vec_helper.c}  | 1297 ++++++++---------

These changes are fine, but should be split.

The helper changes can be done with only minimal changes

>   target/loongarch/insn_trans/trans_lsx.c.inc   |  731 +++++-----

here, rather than to 700+ lines at once.

> -static bool gen_vvvv(DisasContext *ctx, arg_vvvv *a,
> -                     void (*func)(TCGv_ptr, TCGv_i32, TCGv_i32,
> -                                  TCGv_i32, TCGv_i32))
> +static bool gen_vvvv(DisasContext *ctx, arg_vvvv *a, int oprsz,
> +                     gen_helper_gvec_4 *fn)

If you omit the oprsz argument within this patch,

> +    tcg_gen_gvec_4_ool(vec_full_offset(a->vd),
> +                       vec_full_offset(a->vj),
> +                       vec_full_offset(a->vk),
> +                       vec_full_offset(a->va),
> +                       oprsz, ctx->vl / 8, oprsz, fn);

hard-coding 16 here instead,

> -TRANS(vhaddw_h_b, LSX, gen_vvv, gen_helper_vhaddw_h_b)
> +TRANS(vhaddw_h_b, LSX, gen_vvv, 16, gen_helper_vhaddw_h_b)

then you do not need all of these changes.

At which point I'll refer you back to my comments vs patches 5 and 6, wherein separate 
gen_vvv and gen_xxx helpers would avoid the need to replicate 16 across all of these lines.


r~