[Qemu-devel] [PATCH 4/7] target/riscv: Rename some argument sets in insn32.decode

Richard Henderson posted 7 patches 7 years ago
[Qemu-devel] [PATCH 4/7] target/riscv: Rename some argument sets in insn32.decode
Posted by Richard Henderson 7 years ago
For format x, use &x for the argument set and @x for the extract.
This is less confusing than e.g. "arith" for format R.
---
 target/riscv/insn_trans/trans_rvi.inc.c |  2 +-
 target/riscv/translate.c                | 10 +++++-----
 target/riscv/insn32.decode              | 12 ++++++------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvi.inc.c b/target/riscv/insn_trans/trans_rvi.inc.c
index 2ef355019d..7e676fe2e4 100644
--- a/target/riscv/insn_trans/trans_rvi.inc.c
+++ b/target/riscv/insn_trans/trans_rvi.inc.c
@@ -72,7 +72,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
     return true;
 }
 
-static bool gen_branch(DisasContext *ctx, arg_branch *a, TCGCond cond)
+static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
 {
     TCGLabel *l = gen_new_label();
     TCGv source1, source2;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index ece163e69f..e7fe8720ac 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -326,7 +326,7 @@ bool decode_insn32(DisasContext *ctx, uint32_t insn);
 /* Include the auto-generated decoder for 32 bit insn */
 #include "decode_insn32.inc.c"
 
-static bool gen_arith_imm(DisasContext *ctx, arg_arith_imm *a,
+static bool gen_arith_imm(DisasContext *ctx, arg_i *a,
                           void(*func)(TCGv, TCGv, TCGv))
 {
     TCGv source1, source2;
@@ -344,7 +344,7 @@ static bool gen_arith_imm(DisasContext *ctx, arg_arith_imm *a,
     return true;
 }
 
-static bool gen_arith(DisasContext *ctx, arg_arith *a,
+static bool gen_arith(DisasContext *ctx, arg_r *a,
                       void(*func)(TCGv, TCGv, TCGv))
 {
     TCGv source1, source2;
@@ -363,7 +363,7 @@ static bool gen_arith(DisasContext *ctx, arg_arith *a,
 }
 
 #ifdef TARGET_RISCV64
-static bool gen_arith_w(DisasContext *ctx, arg_arith *a,
+static bool gen_arith_w(DisasContext *ctx, arg_r *a,
                         void(*func)(TCGv, TCGv, TCGv))
 {
     TCGv source1, source2;
@@ -384,8 +384,8 @@ static bool gen_arith_w(DisasContext *ctx, arg_arith *a,
 }
 #endif
 
-static bool gen_shift(DisasContext *ctx, arg_arith *a,
-                        void(*func)(TCGv, TCGv, TCGv))
+static bool gen_shift(DisasContext *ctx, arg_r *a,
+                      void(*func)(TCGv, TCGv, TCGv))
 {
     TCGv source1 = tcg_temp_new();
     TCGv source2 = tcg_temp_new();
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 1541c254df..77e093a060 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -38,17 +38,17 @@
 %imm_u    12:s20                 !function=ex_shift_12
 
 # Argument sets:
-&branch    imm rs2 rs1
-&arith_imm imm rs1 rd
-&arith     rd rs1 rs2
+&b         imm rs2 rs1
+&i         imm rs1 rd
+&r         rd rs1 rs2
 &shift     shamt rs1 rd
 &atomic    aq rl rs2 rs1 rd
 
 # Formats 32:
 
-@r       .......   ..... ..... ... ..... ....... &arith            %rs2 %rs1 %rd
-@i       ............    ..... ... ..... ....... &arith_imm imm=%imm_i  %rs1 %rd
-@b       .......   ..... ..... ... ..... ....... &branch imm=%imm_b %rs2 %rs1
+@r       .......   ..... ..... ... ..... ....... &r    %rs2 %rs1 %rd
+@i       ............    ..... ... ..... ....... &i    imm=%imm_i %rs1 %rd
+@b       .......   ..... ..... ... ..... ....... &b    imm=%imm_b %rs2 %rs1
 @s       .......   ..... ..... ... ..... .......         imm=%imm_s %rs2 %rs1
 @u       ....................      ..... .......         imm=%imm_u          %rd
 @j       ....................      ..... .......         imm=%imm_j          %rd
-- 
2.17.2


Re: [Qemu-devel] [PATCH 4/7] target/riscv: Rename some argument sets in insn32.decode
Posted by Philippe Mathieu-Daudé 7 years ago
On 23/10/18 14:04, Richard Henderson wrote:
> For format x, use &x for the argument set and @x for the extract.
> This is less confusing than e.g. "arith" for format R.

With your S-o-b:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   target/riscv/insn_trans/trans_rvi.inc.c |  2 +-
>   target/riscv/translate.c                | 10 +++++-----
>   target/riscv/insn32.decode              | 12 ++++++------
>   3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/target/riscv/insn_trans/trans_rvi.inc.c b/target/riscv/insn_trans/trans_rvi.inc.c
> index 2ef355019d..7e676fe2e4 100644
> --- a/target/riscv/insn_trans/trans_rvi.inc.c
> +++ b/target/riscv/insn_trans/trans_rvi.inc.c
> @@ -72,7 +72,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>       return true;
>   }
>   
> -static bool gen_branch(DisasContext *ctx, arg_branch *a, TCGCond cond)
> +static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>   {
>       TCGLabel *l = gen_new_label();
>       TCGv source1, source2;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index ece163e69f..e7fe8720ac 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -326,7 +326,7 @@ bool decode_insn32(DisasContext *ctx, uint32_t insn);
>   /* Include the auto-generated decoder for 32 bit insn */
>   #include "decode_insn32.inc.c"
>   
> -static bool gen_arith_imm(DisasContext *ctx, arg_arith_imm *a,
> +static bool gen_arith_imm(DisasContext *ctx, arg_i *a,
>                             void(*func)(TCGv, TCGv, TCGv))
>   {
>       TCGv source1, source2;
> @@ -344,7 +344,7 @@ static bool gen_arith_imm(DisasContext *ctx, arg_arith_imm *a,
>       return true;
>   }
>   
> -static bool gen_arith(DisasContext *ctx, arg_arith *a,
> +static bool gen_arith(DisasContext *ctx, arg_r *a,
>                         void(*func)(TCGv, TCGv, TCGv))
>   {
>       TCGv source1, source2;
> @@ -363,7 +363,7 @@ static bool gen_arith(DisasContext *ctx, arg_arith *a,
>   }
>   
>   #ifdef TARGET_RISCV64
> -static bool gen_arith_w(DisasContext *ctx, arg_arith *a,
> +static bool gen_arith_w(DisasContext *ctx, arg_r *a,
>                           void(*func)(TCGv, TCGv, TCGv))
>   {
>       TCGv source1, source2;
> @@ -384,8 +384,8 @@ static bool gen_arith_w(DisasContext *ctx, arg_arith *a,
>   }
>   #endif
>   
> -static bool gen_shift(DisasContext *ctx, arg_arith *a,
> -                        void(*func)(TCGv, TCGv, TCGv))
> +static bool gen_shift(DisasContext *ctx, arg_r *a,
> +                      void(*func)(TCGv, TCGv, TCGv))
>   {
>       TCGv source1 = tcg_temp_new();
>       TCGv source2 = tcg_temp_new();
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 1541c254df..77e093a060 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -38,17 +38,17 @@
>   %imm_u    12:s20                 !function=ex_shift_12
>   
>   # Argument sets:
> -&branch    imm rs2 rs1
> -&arith_imm imm rs1 rd
> -&arith     rd rs1 rs2
> +&b         imm rs2 rs1
> +&i         imm rs1 rd
> +&r         rd rs1 rs2
>   &shift     shamt rs1 rd
>   &atomic    aq rl rs2 rs1 rd
>   
>   # Formats 32:
>   
> -@r       .......   ..... ..... ... ..... ....... &arith            %rs2 %rs1 %rd
> -@i       ............    ..... ... ..... ....... &arith_imm imm=%imm_i  %rs1 %rd
> -@b       .......   ..... ..... ... ..... ....... &branch imm=%imm_b %rs2 %rs1
> +@r       .......   ..... ..... ... ..... ....... &r    %rs2 %rs1 %rd
> +@i       ............    ..... ... ..... ....... &i    imm=%imm_i %rs1 %rd
> +@b       .......   ..... ..... ... ..... ....... &b    imm=%imm_b %rs2 %rs1
>   @s       .......   ..... ..... ... ..... .......         imm=%imm_s %rs2 %rs1
>   @u       ....................      ..... .......         imm=%imm_u          %rd
>   @j       ....................      ..... .......         imm=%imm_j          %rd
> 

Re: [Qemu-devel] [PATCH 4/7] target/riscv: Rename some argument sets in insn32.decode
Posted by Richard Henderson 7 years ago
On 10/23/18 1:21 PM, Philippe Mathieu-Daudé wrote:
> On 23/10/18 14:04, Richard Henderson wrote:
>> For format x, use &x for the argument set and @x for the extract.
>> This is less confusing than e.g. "arith" for format R.
> 
> With your S-o-b:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Oh no.  I fully expect these to be merged back into the original patch set:
http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg04648.html

r~

Re: [Qemu-devel] [PATCH 4/7] target/riscv: Rename some argument sets in insn32.decode
Posted by Bastian Koppelmann 7 years ago
On 10/23/18 2:04 PM, Richard Henderson wrote:
> For format x, use &x for the argument set and @x for the extract.
> This is less confusing than e.g. "arith" for format R.
> ---
>   target/riscv/insn_trans/trans_rvi.inc.c |  2 +-
>   target/riscv/translate.c                | 10 +++++-----
>   target/riscv/insn32.decode              | 12 ++++++------
>   3 files changed, 12 insertions(+), 12 deletions(-)
>
Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian