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

From: gaosong <gaosong@loongson.cn>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>,
	qemu-devel@nongnu.org, laurent@vivier.eu
Subject: Re: [PATCH v10 04/26] target/loongarch: Add fixed point arithmetic instruction translation
Date: Wed, 17 Nov 2021 17:29:51 +0800	[thread overview]
Message-ID: <a46e8df8-ccb9-52d0-2e73-33a49e92bfd8@loongson.cn> (raw)
In-Reply-To: <e26b8243-2797-cde9-3833-a7c4cd3b6a65@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 3392 bytes --]

Hi Richard,

On 2021/11/17 下午4:28, Richard Henderson wrote:
> 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 ?

The code is not written completely,  like this:

gen_rr_i12:

@fmt_rr_i12               .... ...... imm:s12 rj:5 rd:5 &rr_i
slti         0000 001000 ............ ..... .....     @fmt_rr_i12
sltui        0000 001001 ............ ..... .....     @fmt_rr_i12
...

gen_rr_ui12:

@fmt_rr_ui12               .... ...... imm:12 rj:5 rd:5 &rr_i
andi         0000 001101 ............ ..... .....     @fmt_rr_ui12
ori          0000 001110 ............ ..... .....     @fmt_rr_ui12
xori         0000 001111 ............ ..... .....     @fmt_rr_ui12
...

@fmt_rr_i12 and @fmt_rr_ui12 are two 'Formats',  but they use the same 'Argument sets'(rr_i).

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

gen_rrr_sa2p1:

@fmt_rrr_sa2p1        .... ........ ... .. rk:5 rj:5 rd:5   &fmt_rr_sa  sa=%sa2p1
lsl_w           0000 00000000 010 .. ..... ..... .....    @fmt_rrr_sa2p1
alsl_wu          0000 00000000 011 .. ..... ..... .....   @fmt_rrr_sa2p1
alsl_d           0000 00000010 110 .. ..... ..... .....   @fmt_rrr_sa2p1
...

gen_rrr_sa2:
@fmt_rrr_sa2        .... ........ ... sa:2 rk:5 rj:5 rd:5   &fmt_rr_sa
bytepick_w       0000 00000000 100 .. ..... ..... .....   @fmt_rrr_sa3
...

gen_rrr_sa3:
@fmt_rrr_sa3         .... ........ .. sa:3 rk:5 rj:5 rd:5   &fmt_rr_sa
bytepick_d       0000 00000000 11 ... ..... ..... .....   @fmt_rrr_sa3
...

>> Richard, is that OK?
>
> Other than those two nits, this looks very clean.  Thanks,
>
OK, I'll correct it on v11.

Thanks.
Song Gao

[-- Attachment #2: Type: text/html, Size: 5096 bytes --]