[PATCH v11 05/26] target/loongarch: Add fixed point shift instruction translation

Song Gao posted 26 patches 1 year, 2 months ago
Maintainers: Song Gao <gaosong@loongson.cn>, Laurent Vivier <laurent@vivier.eu>
There is a newer version of this series
[PATCH v11 05/26] target/loongarch: Add fixed point shift instruction translation
Posted by Song Gao 1 year, 2 months ago
This includes:
- SLL.W, SRL.W, SRA.W, ROTR.W
- SLLI.W, SRLI.W, SRAI.W, ROTRI.W
- SLL.D, SRL.D, SRA.D, ROTR.D
- SLLI.D, SRLI.D, SRAI.D, ROTRI.D

Signed-off-by: Song Gao <gaosong@loongson.cn>
Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/loongarch/insn_trans/trans_shift.c.inc | 128 ++++++++++++++++++++++++++
 target/loongarch/insns.decode                 |  22 +++++
 target/loongarch/translate.c                  |   1 +
 3 files changed, 151 insertions(+)
 create mode 100644 target/loongarch/insn_trans/trans_shift.c.inc

diff --git a/target/loongarch/insn_trans/trans_shift.c.inc b/target/loongarch/insn_trans/trans_shift.c.inc
new file mode 100644
index 0000000..ea2a612
--- /dev/null
+++ b/target/loongarch/insn_trans/trans_shift.c.inc
@@ -0,0 +1,128 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2021 Loongson Technology Corporation Limited
+ */
+
+static bool gen_shift(DisasContext *ctx, arg_rr_i *a,
+                      void(*func)(TCGv, TCGv, TCGv))
+{
+    TCGv dest = gpr_dst(ctx, a->rd, EXT_SIGN);
+    TCGv src1 = gpr_src(ctx, a->rj, EXT_ZERO);
+    TCGv src2 = tcg_constant_tl(a->imm);
+
+    func(dest, src1, src2);
+    gen_set_gpr(a->rd, dest, EXT_SIGN);
+
+    return true;
+}
+
+static bool gen_shift_i(DisasContext *ctx, arg_rr_i *a,
+                        void(*func)(TCGv, TCGv, target_long))
+{
+    TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
+    TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
+
+    func(dest, src1, a->imm);
+
+    return true;
+}
+
+static void gen_sll_w(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, src2, 0x1f);
+    tcg_gen_shl_tl(dest, src1, t0);
+    tcg_temp_free(t0);
+}
+
+static void gen_srl_w(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, src2, 0x1f);
+    tcg_gen_shr_tl(dest, src1, t0);
+    tcg_temp_free(t0);
+}
+
+static void gen_sra_w(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, src2, 0x1f);
+    tcg_gen_sar_tl(dest, src1, t0);
+    tcg_temp_free(t0);
+}
+
+static void gen_sll_d(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, src2, 0x3f);
+    tcg_gen_shl_tl(dest, src1, t0);
+    tcg_temp_free(t0);
+}
+
+static void gen_srl_d(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, src2, 0x3f);
+    tcg_gen_shr_tl(dest, src1, t0);
+    tcg_temp_free(t0);
+}
+
+static void gen_sra_d(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, src2, 0x3f);
+    tcg_gen_sar_tl(dest, src1, t0);
+    tcg_temp_free(t0);
+}
+
+static void gen_rotr_w(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv_i32 t1 = tcg_temp_new_i32();
+    TCGv_i32 t2 = tcg_temp_new_i32();
+    TCGv t0 = tcg_temp_new();
+
+    tcg_gen_andi_tl(t0, src2, 0x1f);
+
+    tcg_gen_trunc_tl_i32(t1, src1);
+    tcg_gen_trunc_tl_i32(t2, t0);
+
+    tcg_gen_rotr_i32(t1, t1, t2);
+    tcg_gen_ext_i32_tl(dest, t1);
+
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t2);
+    tcg_temp_free(t0);
+}
+
+static void gen_rotr_d(TCGv dest, TCGv src1, TCGv src2)
+{
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_andi_tl(t0, src2, 0x3f);
+    tcg_gen_rotr_tl(dest, src1, t0);
+    tcg_temp_free(t0);
+}
+
+static bool trans_srai_w(DisasContext *ctx, arg_srai_w *a)
+{
+    TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
+    TCGv src1 = gpr_src(ctx, a->rj, EXT_ZERO);
+
+    tcg_gen_sextract_tl(dest, src1, a->imm, 32 - a->imm);
+    return true;
+}
+
+TRANS(sll_w, gen_rrr, EXT_ZERO, EXT_NONE, EXT_SIGN, gen_sll_w)
+TRANS(srl_w, gen_rrr, EXT_ZERO, EXT_NONE, EXT_SIGN, gen_srl_w)
+TRANS(sra_w, gen_rrr, EXT_SIGN, EXT_NONE, EXT_SIGN, gen_sra_w)
+TRANS(sll_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_sll_d)
+TRANS(srl_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_srl_d)
+TRANS(sra_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_sra_d)
+TRANS(rotr_w, gen_rrr, EXT_ZERO, EXT_NONE, EXT_SIGN, gen_rotr_w)
+TRANS(rotr_d, gen_rrr, EXT_NONE, EXT_NONE, EXT_NONE, gen_rotr_d)
+TRANS(slli_w, gen_shift, tcg_gen_shl_tl)
+TRANS(slli_d, gen_shift_i, tcg_gen_shli_tl)
+TRANS(srli_w, gen_shift, tcg_gen_shr_tl)
+TRANS(srli_d, gen_shift_i, tcg_gen_shri_tl)
+TRANS(srai_d, gen_shift_i, tcg_gen_sari_tl)
+TRANS(rotri_w, gen_shift, gen_rotr_w)
+TRANS(rotri_d, gen_shift_i, tcg_gen_rotri_tl)
diff --git a/target/loongarch/insns.decode b/target/loongarch/insns.decode
index 8579c11..673aee4 100644
--- a/target/loongarch/insns.decode
+++ b/target/loongarch/insns.decode
@@ -23,6 +23,8 @@
 #
 @rrr               .... ........ ..... rk:5 rj:5 rd:5    &rrr
 @r_i20                          .... ... imm:s20 rd:5    &r_i
+@rr_ui5           .... ........ ..... imm:5 rj:5 rd:5    &rr_i
+@rr_ui6            .... ........ .... imm:6 rj:5 rd:5    &rr_i
 @rr_i12                 .... ...... imm:s12 rj:5 rd:5    &rr_i
 @rr_ui12                 .... ...... imm:12 rj:5 rd:5    &rr_i
 @rr_i16                     .... .. imm:s16 rj:5 rd:5    &rr_i
@@ -77,3 +79,23 @@ addu16i_d       0001 00 ................ ..... .....     @rr_i16
 andi            0000 001101 ............ ..... .....     @rr_ui12
 ori             0000 001110 ............ ..... .....     @rr_ui12
 xori            0000 001111 ............ ..... .....     @rr_ui12
+
+#
+# Fixed point shift operation instruction
+#
+sll_w           0000 00000001 01110 ..... ..... .....    @rrr
+srl_w           0000 00000001 01111 ..... ..... .....    @rrr
+sra_w           0000 00000001 10000 ..... ..... .....    @rrr
+sll_d           0000 00000001 10001 ..... ..... .....    @rrr
+srl_d           0000 00000001 10010 ..... ..... .....    @rrr
+sra_d           0000 00000001 10011 ..... ..... .....    @rrr
+rotr_w          0000 00000001 10110 ..... ..... .....    @rrr
+rotr_d          0000 00000001 10111 ..... ..... .....    @rrr
+slli_w          0000 00000100 00001 ..... ..... .....    @rr_ui5
+slli_d          0000 00000100 0001 ...... ..... .....    @rr_ui6
+srli_w          0000 00000100 01001 ..... ..... .....    @rr_ui5
+srli_d          0000 00000100 0101 ...... ..... .....    @rr_ui6
+srai_w          0000 00000100 10001 ..... ..... .....    @rr_ui5
+srai_d          0000 00000100 1001 ...... ..... .....    @rr_ui6
+rotri_w         0000 00000100 11001 ..... ..... .....    @rr_ui5
+rotri_d         0000 00000100 1101 ...... ..... .....    @rr_ui6
diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
index 3de18ef..f90b63a 100644
--- a/target/loongarch/translate.c
+++ b/target/loongarch/translate.c
@@ -146,6 +146,7 @@ static void gen_set_gpr(int reg_num, TCGv t, DisasExtend dst_ext)
 
 #include "decode-insns.c.inc"
 #include "insn_trans/trans_arith.c.inc"
+#include "insn_trans/trans_shift.c.inc"
 
 static void loongarch_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 {
-- 
1.8.3.1


Re: [PATCH v11 05/26] target/loongarch: Add fixed point shift instruction translation
Posted by Richard Henderson 1 year, 2 months ago
On 11/19/21 7:13 AM, Song Gao wrote:
> +static bool gen_shift(DisasContext *ctx, arg_rr_i *a,
> +                      void(*func)(TCGv, TCGv, TCGv))
> +{
> +    TCGv dest = gpr_dst(ctx, a->rd, EXT_SIGN);
> +    TCGv src1 = gpr_src(ctx, a->rj, EXT_ZERO);
> +    TCGv src2 = tcg_constant_tl(a->imm);
> +
> +    func(dest, src1, src2);
> +    gen_set_gpr(a->rd, dest, EXT_SIGN);
> +
> +    return true;
> +}

This is no longer generic; it's specific to word operations.  But there's nothing in here 
that can't be done with gen_rr_i, so I think you should remove it.

> +
> +static bool gen_shift_i(DisasContext *ctx, arg_rr_i *a,
> +                        void(*func)(TCGv, TCGv, target_long))
> +{
> +    TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
> +    TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
> +
> +    func(dest, src1, a->imm);
> +
> +    return true;
> +}

This one has dropped gen_set_gpr.

I think that your current gen_rr_i should be named gen_rri_v (variable) and this one 
should regain the DisasExtend and be named gen_rri_c (constant).

Then, in the previous,

TRANS(addi_w, gen_rri_c, EXT_NONE, EXT_SIGN, tcg_gen_addi_tl)
TRANS(addi_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_addi_tl)
TRANS(andi, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_andi_tl)
TRANS(ori, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_ori_tl)
TRANS(xori, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_xori_tl)

There are a few identity tests within these tcg_gen_opi_tl functions which would be nice 
to apply.  Particularly because the canonical "nop" instruction for loongarch is "andi 
r0,r0,0".

> +TRANS(slli_w, gen_shift, tcg_gen_shl_tl)
> +TRANS(slli_d, gen_shift_i, tcg_gen_shli_tl)
> +TRANS(srli_w, gen_shift, tcg_gen_shr_tl)
> +TRANS(srli_d, gen_shift_i, tcg_gen_shri_tl)
> +TRANS(srai_d, gen_shift_i, tcg_gen_sari_tl)
> +TRANS(rotri_w, gen_shift, gen_rotr_w)
> +TRANS(rotri_d, gen_shift_i, tcg_gen_rotri_tl)

Then these become

TRANS(slli_w, gen_rri_c, EXT_NONE, EXT_SIGN, tcg_gen_shli_tl)
TRANS(slli_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_shli_tl)
TRANS(srli_w, gen_rri_c, EXT_SIGN, EXT_SIGN, tcg_gen_shri_tl)
TRANS(srli_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_shri_tl)
TRANS(srai_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_sari_tl)
TRANS(rotri_w, gen_rri_v, EXT_NONE, EXT_NONE, gen_rotr_w)
TRANS(rotri_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_rotri_tl)


r~

Re: [PATCH v11 05/26] target/loongarch: Add fixed point shift instruction translation
Posted by gaosong 1 year, 2 months ago
Hi Richard,

On 2021/11/20 下午3:42, Richard Henderson wrote:
> On 11/19/21 7:13 AM, Song Gao wrote:
>> +static bool gen_shift(DisasContext *ctx, arg_rr_i *a,
>> +                      void(*func)(TCGv, TCGv, TCGv))
>> +{
>> +    TCGv dest = gpr_dst(ctx, a->rd, EXT_SIGN);
>> +    TCGv src1 = gpr_src(ctx, a->rj, EXT_ZERO);
>> +    TCGv src2 = tcg_constant_tl(a->imm);
>> +
>> +    func(dest, src1, src2);
>> +    gen_set_gpr(a->rd, dest, EXT_SIGN);
>> +
>> +    return true;
>> +}
>
> This is no longer generic; it's specific to word operations.  But 
> there's nothing in here that can't be done with gen_rr_i, so I think 
> you should remove it.
>
OK.
>> +
>> +static bool gen_shift_i(DisasContext *ctx, arg_rr_i *a,
>> +                        void(*func)(TCGv, TCGv, target_long))
>> +{
>> +    TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
>> +    TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
>> +
>> +    func(dest, src1, a->imm);
>> +
>> +    return true;
>> +}
>
> This one has dropped gen_set_gpr.
>
We need't gen_set_gpr, the dst_ext is EXT_NONE.

> I think that your current gen_rr_i should be named gen_rri_v 
> (variable) and this one should regain the DisasExtend and be named 
> gen_rri_c (constant).
>
> Then, in the previous,
>
> TRANS(addi_w, gen_rri_c, EXT_NONE, EXT_SIGN, tcg_gen_addi_tl)
> TRANS(addi_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_addi_tl)
> TRANS(andi, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_andi_tl)
> TRANS(ori, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_ori_tl)
> TRANS(xori, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_xori_tl)
>
> There are a few identity tests within these tcg_gen_opi_tl functions 
> which would be nice to apply.  Particularly because the canonical 
> "nop" instruction for loongarch is "andi r0,r0,0".
>
>> +TRANS(slli_w, gen_shift, tcg_gen_shl_tl)
>> +TRANS(slli_d, gen_shift_i, tcg_gen_shli_tl)
>> +TRANS(srli_w, gen_shift, tcg_gen_shr_tl)
>> +TRANS(srli_d, gen_shift_i, tcg_gen_shri_tl)
>> +TRANS(srai_d, gen_shift_i, tcg_gen_sari_tl)
>> +TRANS(rotri_w, gen_shift, gen_rotr_w)
>> +TRANS(rotri_d, gen_shift_i, tcg_gen_rotri_tl)
>
> Then these become
>
> TRANS(slli_w, gen_rri_c, EXT_NONE, EXT_SIGN, tcg_gen_shli_tl)
> TRANS(slli_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_shli_tl)
> TRANS(srli_w, gen_rri_c, EXT_SIGN, EXT_SIGN, tcg_gen_shri_tl)
> TRANS(srli_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_shri_tl)
> TRANS(srai_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_sari_tl)
> TRANS(rotri_w, gen_rri_v, EXT_NONE, EXT_NONE, gen_rotr_w)
> TRANS(rotri_d, gen_rri_c, EXT_NONE, EXT_NONE, tcg_gen_rotri_tl)
>
Very nice, very clean.

Thanks
Song Gao

> r~