Re: [PATCH v10 04/26] target/loongarch: Add fixed point arithmetic instruction translation

[Richard Henderson]

On 11/17/21 8:57 AM, gaosong wrote:
> I see that  insns.decode format is not very consistent with other architectures, such 
> ARM/RISCV

No.  I don't like how riscv has done it, though they have quite a few split fields, so 
perhaps they thought it looked weird.


> #
> # Argument sets
> #
> &r_i          rd imm
> &rrr          rd rj rk
> &rr_i         rd rj imm
> &rrr_sa     rd rj rk sa
>
> #
> # Formats
> #
> @fmt_rrr             .... ........ ..... rk:5 rj:5 rd:5 &rrr
> @fmt_r_i20                        .... ... imm:s20 rd:5 &r_i
> @fmt_rr_i12               .... ...... imm:s12 rj:5 rd:5 &rr_i
> @fmt_rr_ui12               .... ...... imm:12 rj:5 rd:5 &rr_i
> @fmt_rr_i16                   .... .. imm:s16 rj:5 rd:5 &rr_i
> @fmt_rrr_sa2p1      .... ........ ... .. rk:5 rj:5 rd:5 &rrr_sa  sa=%sa2p1
>
> #
> # Fixed point arithmetic operation instruction
> #
> add_w            0000 00000001 00000 ..... ..... .....    @fmt_rrr
> add_d            0000 00000001 00001 ..... ..... .....    @fmt_rrr
> sub_w            0000 00000001 00010 ..... ..... .....    @fmt_rrr
> sub_d            0000 00000001 00011 ..... ..... .....    @fmt_rrr
> slt              0000 00000001 00100 ..... ..... ..... @fmt_rrr
> sltu             0000 00000001 00101 ..... ..... ..... @fmt_rrr
> slti             0000 001000 ............ ..... .....               @fmt_rr_i12
>
>
> and trans_xxx.c.inc
>
> static bool gen_rrr(DisasContext *ctx, arg_rrr *a, ...) {}
> static bool gen_rr_i12(DisasContext *ctx, arg_rr_i *a, ) {}

gen_rr_i ?

> static bool gen_rrr_sa2p1(DisasContext *ctx, arg_rrr_sa *a, ...) {}

gen_rrr_sa ?

> Richard, is that OK?

Other than those two nits, this looks very clean.  Thanks,


r~