1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 0/7] riscv decodetree followup
  4. View patch

[Qemu-devel] [PATCH 5/7] target/riscv: Convert @cs_2 insns to share translation functions

Richard Henderson posted 7 patches 7 years ago
[Qemu-devel] [PATCH 5/7] target/riscv: Convert @cs_2 insns to share translation functions
Posted by Richard Henderson 7 years ago 
These all expand simply to R format instructions.
---
 target/riscv/insn_trans/trans_rvc.inc.c | 36 -------------------------
 target/riscv/translate.c                | 20 +++++++++++---
 target/riscv/insn16.decode              | 17 +++++++-----
 3 files changed, 26 insertions(+), 47 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvc.inc.c b/target/riscv/insn_trans/trans_rvc.inc.c
index 7e2668c03d..152c1c9bca 100644
--- a/target/riscv/insn_trans/trans_rvc.inc.c
+++ b/target/riscv/insn_trans/trans_rvc.inc.c
@@ -176,42 +176,6 @@ static bool trans_c_andi(DisasContext *ctx, arg_c_andi *a)
     return trans_andi(ctx, &arg);
 }
 
-static bool trans_c_sub(DisasContext *ctx, arg_c_sub *a)
-{
-    arg_sub arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
-    return trans_sub(ctx, &arg);
-}
-
-static bool trans_c_xor(DisasContext *ctx, arg_c_xor *a)
-{
-    arg_xor arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
-    return trans_xor(ctx, &arg);
-}
-
-static bool trans_c_or(DisasContext *ctx, arg_c_or *a)
-{
-    arg_or arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
-    return trans_or(ctx, &arg);
-}
-
-static bool trans_c_and(DisasContext *ctx, arg_c_and *a)
-{
-    arg_and arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
-    return trans_and(ctx, &arg);
-}
-
-static bool trans_c_subw(DisasContext *ctx, arg_c_subw *a)
-{
-    arg_subw arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
-    return trans_subw(ctx, &arg);
-}
-
-static bool trans_c_addw(DisasContext *ctx, arg_c_addw *a)
-{
-    arg_addw arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
-    return trans_addw(ctx, &arg);
-}
-
 static bool trans_c_j(DisasContext *ctx, arg_c_j *a)
 {
     arg_jal arg = { .rd = 0, .imm = a->imm };
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index e7fe8720ac..f4d2a56f9a 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -322,10 +322,25 @@ static int ex_rvc_register(int reg)
     return 8 + reg;
 }
 
+/*
+ * Include the auto-generated decoders.
+ * Note that the 16-bit decoder reuses some of the trans_* functions
+ * from the 32-bit decoder, which results in duplicate declarations
+ * of the relevant helpers.  Suppress the warning.
+ */
 bool decode_insn32(DisasContext *ctx, uint32_t insn);
-/* Include the auto-generated decoder for 32 bit insn */
+bool decode_insn16(DisasContext *ctx, uint16_t insn);
+
 #include "decode_insn32.inc.c"
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wredundant-decls"
+
+#include "decode_insn16.inc.c"
+
+#pragma GCC diagnostic pop
+
+
 static bool gen_arith_imm(DisasContext *ctx, arg_i *a,
                           void(*func)(TCGv, TCGv, TCGv))
 {
@@ -410,9 +425,6 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
 #include "insn_trans/trans_rvd.inc.c"
 #include "insn_trans/trans_privileged.inc.c"
 
-bool decode_insn16(DisasContext *ctx, uint16_t insn);
-/* auto-generated decoder*/
-#include "decode_insn16.inc.c"
 #include "insn_trans/trans_rvc.inc.c"
 
 static void decode_opc(DisasContext *ctx)
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 138290c450..16525486ae 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -39,6 +39,9 @@
 %imm_addi16sp  12:s1 3:2 5:1 2:1 6:1 !function=ex_shift_4
 %imm_lui       12:s1 2:5             !function=ex_shift_12
 
+# Argument sets imported from insn32.decode:
+&r         rd rs1 rs2   !extern
+
 # Argument sets:
 &cl               rs1 rd
 &cl_dw     uimm   rs1 rd
@@ -65,7 +68,7 @@
 @cl_w      ... ... ... .. ... .. &cl_dw  uimm=%uimm_cl_w  rs1=%rs1_3  rd=%rs2_3
 @cl        ... ... ... .. ... .. &cl                      rs1=%rs1_3  rd=%rs2_3
 @cs        ... ... ... .. ... .. &cs                      rs1=%rs1_3  rs2=%rs2_3
-@cs_2      ... ... ... .. ... .. &cr                      rd=%rs1_3   rs2=%rs2_3
+@cs_2      ... ... ... .. ... .. &r      rd=%rs1_3 rs1=%rs1_3 rs2=%rs2_3
 @cs_d      ... ... ... .. ... .. &cs_dw  uimm=%uimm_cl_d  rs1=%rs1_3  rs2=%rs2_3
 @cs_w      ... ... ... .. ... .. &cs_dw  uimm=%uimm_cl_w  rs1=%rs1_3  rs2=%rs2_3
 @cb        ... ... ... .. ... .. &cb     imm=%imm_cb      rs1=%rs1_3
@@ -104,12 +107,12 @@ c_addi16sp_lui    011 .  .....  ..... 01 @c_addi16sp_lui # shares opc with C.LUI
 c_srli            100 . 00 ...  ..... 01 @c_shift
 c_srai            100 . 01 ...  ..... 01 @c_shift
 c_andi            100 . 10 ...  ..... 01 @c_andi
-c_sub             100 0 11 ... 00 ... 01 @cs_2
-c_xor             100 0 11 ... 01 ... 01 @cs_2
-c_or              100 0 11 ... 10 ... 01 @cs_2
-c_and             100 0 11 ... 11 ... 01 @cs_2
-c_subw            100 1 11 ... 00 ... 01 @cs_2
-c_addw            100 1 11 ... 01 ... 01 @cs_2
+sub               100 0 11 ... 00 ... 01 @cs_2
+xor               100 0 11 ... 01 ... 01 @cs_2
+or                100 0 11 ... 10 ... 01 @cs_2
+and               100 0 11 ... 11 ... 01 @cs_2
+subw              100 1 11 ... 00 ... 01 @cs_2
+addw              100 1 11 ... 01 ... 01 @cs_2
 c_j               101     ........... 01 @cj
 c_beqz            110  ... ...  ..... 01 @cb
 c_bnez            111  ... ...  ..... 01 @cb
-- 
2.17.2
Re: [Qemu-devel] [PATCH 5/7] target/riscv: Convert @cs_2 insns to share translation functions
Posted by Philippe Mathieu-Daudé 7 years ago 
On 23/10/18 14:04, Richard Henderson wrote:
> These all expand simply to R format instructions.
> ---
>   target/riscv/insn_trans/trans_rvc.inc.c | 36 -------------------------
>   target/riscv/translate.c                | 20 +++++++++++---
>   target/riscv/insn16.decode              | 17 +++++++-----
>   3 files changed, 26 insertions(+), 47 deletions(-)
> 
> diff --git a/target/riscv/insn_trans/trans_rvc.inc.c b/target/riscv/insn_trans/trans_rvc.inc.c
> index 7e2668c03d..152c1c9bca 100644
> --- a/target/riscv/insn_trans/trans_rvc.inc.c
> +++ b/target/riscv/insn_trans/trans_rvc.inc.c
> @@ -176,42 +176,6 @@ static bool trans_c_andi(DisasContext *ctx, arg_c_andi *a)
>       return trans_andi(ctx, &arg);
>   }
>   
> -static bool trans_c_sub(DisasContext *ctx, arg_c_sub *a)
> -{
> -    arg_sub arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
> -    return trans_sub(ctx, &arg);
> -}
> -
> -static bool trans_c_xor(DisasContext *ctx, arg_c_xor *a)
> -{
> -    arg_xor arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
> -    return trans_xor(ctx, &arg);
> -}
> -
> -static bool trans_c_or(DisasContext *ctx, arg_c_or *a)
> -{
> -    arg_or arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
> -    return trans_or(ctx, &arg);
> -}
> -
> -static bool trans_c_and(DisasContext *ctx, arg_c_and *a)
> -{
> -    arg_and arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
> -    return trans_and(ctx, &arg);
> -}
> -
> -static bool trans_c_subw(DisasContext *ctx, arg_c_subw *a)
> -{
> -    arg_subw arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
> -    return trans_subw(ctx, &arg);
> -}
> -
> -static bool trans_c_addw(DisasContext *ctx, arg_c_addw *a)
> -{
> -    arg_addw arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
> -    return trans_addw(ctx, &arg);
> -}
> -
>   static bool trans_c_j(DisasContext *ctx, arg_c_j *a)
>   {
>       arg_jal arg = { .rd = 0, .imm = a->imm };
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index e7fe8720ac..f4d2a56f9a 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -322,10 +322,25 @@ static int ex_rvc_register(int reg)
>       return 8 + reg;
>   }
>   
> +/*
> + * Include the auto-generated decoders.
> + * Note that the 16-bit decoder reuses some of the trans_* functions
> + * from the 32-bit decoder, which results in duplicate declarations
> + * of the relevant helpers.  Suppress the warning.
> + */
>   bool decode_insn32(DisasContext *ctx, uint32_t insn);
> -/* Include the auto-generated decoder for 32 bit insn */
> +bool decode_insn16(DisasContext *ctx, uint16_t insn);
> +
>   #include "decode_insn32.inc.c"
>   
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wredundant-decls"

We might move that to a generic Makefile rule, this will be useful for 
other archs.

Something like:

translate*.o: CPPFLAGS += -Wno-redundant-decls

> +
> +#include "decode_insn16.inc.c"
> +
> +#pragma GCC diagnostic pop
> +
> +
>   static bool gen_arith_imm(DisasContext *ctx, arg_i *a,
>                             void(*func)(TCGv, TCGv, TCGv))
>   {
> @@ -410,9 +425,6 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
>   #include "insn_trans/trans_rvd.inc.c"
>   #include "insn_trans/trans_privileged.inc.c"
>   
> -bool decode_insn16(DisasContext *ctx, uint16_t insn);
> -/* auto-generated decoder*/
> -#include "decode_insn16.inc.c"
>   #include "insn_trans/trans_rvc.inc.c"
>   
>   static void decode_opc(DisasContext *ctx)
> diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
> index 138290c450..16525486ae 100644
> --- a/target/riscv/insn16.decode
> +++ b/target/riscv/insn16.decode
> @@ -39,6 +39,9 @@
>   %imm_addi16sp  12:s1 3:2 5:1 2:1 6:1 !function=ex_shift_4
>   %imm_lui       12:s1 2:5             !function=ex_shift_12
>   
> +# Argument sets imported from insn32.decode:
> +&r         rd rs1 rs2   !extern
> +
>   # Argument sets:
>   &cl               rs1 rd
>   &cl_dw     uimm   rs1 rd
> @@ -65,7 +68,7 @@
>   @cl_w      ... ... ... .. ... .. &cl_dw  uimm=%uimm_cl_w  rs1=%rs1_3  rd=%rs2_3
>   @cl        ... ... ... .. ... .. &cl                      rs1=%rs1_3  rd=%rs2_3
>   @cs        ... ... ... .. ... .. &cs                      rs1=%rs1_3  rs2=%rs2_3
> -@cs_2      ... ... ... .. ... .. &cr                      rd=%rs1_3   rs2=%rs2_3
> +@cs_2      ... ... ... .. ... .. &r      rd=%rs1_3 rs1=%rs1_3 rs2=%rs2_3
>   @cs_d      ... ... ... .. ... .. &cs_dw  uimm=%uimm_cl_d  rs1=%rs1_3  rs2=%rs2_3
>   @cs_w      ... ... ... .. ... .. &cs_dw  uimm=%uimm_cl_w  rs1=%rs1_3  rs2=%rs2_3
>   @cb        ... ... ... .. ... .. &cb     imm=%imm_cb      rs1=%rs1_3
> @@ -104,12 +107,12 @@ c_addi16sp_lui    011 .  .....  ..... 01 @c_addi16sp_lui # shares opc with C.LUI
>   c_srli            100 . 00 ...  ..... 01 @c_shift
>   c_srai            100 . 01 ...  ..... 01 @c_shift
>   c_andi            100 . 10 ...  ..... 01 @c_andi
> -c_sub             100 0 11 ... 00 ... 01 @cs_2
> -c_xor             100 0 11 ... 01 ... 01 @cs_2
> -c_or              100 0 11 ... 10 ... 01 @cs_2
> -c_and             100 0 11 ... 11 ... 01 @cs_2
> -c_subw            100 1 11 ... 00 ... 01 @cs_2
> -c_addw            100 1 11 ... 01 ... 01 @cs_2
> +sub               100 0 11 ... 00 ... 01 @cs_2
> +xor               100 0 11 ... 01 ... 01 @cs_2
> +or                100 0 11 ... 10 ... 01 @cs_2
> +and               100 0 11 ... 11 ... 01 @cs_2
> +subw              100 1 11 ... 00 ... 01 @cs_2
> +addw              100 1 11 ... 01 ... 01 @cs_2
>   c_j               101     ........... 01 @cj
>   c_beqz            110  ... ...  ..... 01 @cb
>   c_bnez            111  ... ...  ..... 01 @cb
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Re: [Qemu-devel] [PATCH 5/7] target/riscv: Convert @cs_2 insns to share translation functions
Posted by Richard Henderson 7 years ago 
On 10/23/18 1:26 PM, Philippe Mathieu-Daudé wrote:
>>   +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wredundant-decls"
> 
> We might move that to a generic Makefile rule, this will be useful for other
> archs.
> 
> Something like:
> 
> translate*.o: CPPFLAGS += -Wno-redundant-decls

I did want to restrict that to the one region though.


r~
Re: [Qemu-devel] [PATCH 5/7] target/riscv: Convert @cs_2 insns to share translation functions
Posted by Bastian Koppelmann 7 years ago 
On 10/23/18 2:04 PM, Richard Henderson wrote:
> These all expand simply to R format instructions.
> ---
>   target/riscv/insn_trans/trans_rvc.inc.c | 36 -------------------------
>   target/riscv/translate.c                | 20 +++++++++++---
>   target/riscv/insn16.decode              | 17 +++++++-----
>   3 files changed, 26 insertions(+), 47 deletions(-)
>

Pretty cool :)

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.