[Qemu-devel] [PATCH] target/riscv: Fix manually parsed 16 bit insn

Bastian Koppelmann posted 1 patch 5 years, 1 month ago
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190315135140.10507-1-kbastian@mail.uni-paderborn.de
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Palmer Dabbelt <palmer@sifive.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
target/riscv/insn_trans/trans_rvc.inc.c | 30 ++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
[Qemu-devel] [PATCH] target/riscv: Fix manually parsed 16 bit insn
Posted by Bastian Koppelmann 5 years, 1 month ago
during the refactor to decodetree we removed the manual decoding that is
necessary for c.jal/c.addiw and removed the translation of c.flw/c.ld
and c.fsw/c.sd. This reintroduces the manual parsing and the
omited implementation.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 target/riscv/insn_trans/trans_rvc.inc.c | 30 ++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvc.inc.c b/target/riscv/insn_trans/trans_rvc.inc.c
index bcdf64d3b7..5819f53f90 100644
--- a/target/riscv/insn_trans/trans_rvc.inc.c
+++ b/target/riscv/insn_trans/trans_rvc.inc.c
@@ -44,10 +44,19 @@ static bool trans_c_flw_ld(DisasContext *ctx, arg_c_flw_ld *a)
 {
 #ifdef TARGET_RISCV32
     /* C.FLW ( RV32FC-only ) */
-    return false;
+    REQUIRE_FPU;
+    REQUIRE_EXT(ctx, RVF);
+
+    arg_c_lw tmp;
+    decode_insn16_extract_cl_w(&tmp, ctx->opcode);
+    arg_flw arg = { .rd = tmp.rd, .rs1 = tmp.rs1, .imm = tmp.uimm };
+    return trans_flw(ctx, &arg);
 #else
     /* C.LD ( RV64C/RV128C-only ) */
-    return false;
+    arg_c_fld tmp;
+    decode_insn16_extract_cl_d(&tmp, ctx->opcode);
+    arg_ld arg = { .rd = tmp.rd, .rs1 = tmp.rs1, .imm = tmp.uimm };
+    return trans_ld(ctx, &arg);
 #endif
 }
 
@@ -67,10 +76,19 @@ static bool trans_c_fsw_sd(DisasContext *ctx, arg_c_fsw_sd *a)
 {
 #ifdef TARGET_RISCV32
     /* C.FSW ( RV32FC-only ) */
-    return false;
+    REQUIRE_FPU;
+    REQUIRE_EXT(ctx, RVF);
+
+    arg_c_sw tmp;
+    decode_insn16_extract_cs_w(&tmp, ctx->opcode);
+    arg_fsw arg = { .rs1 = tmp.rs1, .rs2 = tmp.rs2, .imm = tmp.uimm };
+    return trans_fsw(ctx, &arg);
 #else
     /* C.SD ( RV64C/RV128C-only ) */
-    return false;
+    arg_c_fsd tmp;
+    decode_insn16_extract_cs_d(&tmp, ctx->opcode);
+    arg_sd arg = { .rs1 = tmp.rs1, .rs2 = tmp.rs2, .imm = tmp.uimm };
+    return trans_sd(ctx, &arg);
 #endif
 }
 
@@ -88,7 +106,9 @@ static bool trans_c_jal_addiw(DisasContext *ctx, arg_c_jal_addiw *a)
 {
 #ifdef TARGET_RISCV32
     /* C.JAL */
-    arg_jal arg = { .rd = 1, .imm = a->imm };
+    arg_c_j tmp;
+    decode_insn16_extract_cj(&tmp, ctx->opcode);
+    arg_jal arg = { .rd = 1, .imm = tmp.imm };
     return trans_jal(ctx, &arg);
 #else
     /* C.ADDIW */
-- 
2.21.0


Re: [Qemu-devel] [PATCH] target/riscv: Fix manually parsed 16 bit insn
Posted by Palmer Dabbelt 5 years, 1 month ago
On Fri, 15 Mar 2019 06:51:40 PDT (-0700), Bastian Koppelmann wrote:
> during the refactor to decodetree we removed the manual decoding that is
> necessary for c.jal/c.addiw and removed the translation of c.flw/c.ld
> and c.fsw/c.sd. This reintroduces the manual parsing and the
> omited implementation.
>
> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> ---
>  target/riscv/insn_trans/trans_rvc.inc.c | 30 ++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvc.inc.c b/target/riscv/insn_trans/trans_rvc.inc.c
> index bcdf64d3b7..5819f53f90 100644
> --- a/target/riscv/insn_trans/trans_rvc.inc.c
> +++ b/target/riscv/insn_trans/trans_rvc.inc.c
> @@ -44,10 +44,19 @@ static bool trans_c_flw_ld(DisasContext *ctx, arg_c_flw_ld *a)
>  {
>  #ifdef TARGET_RISCV32
>      /* C.FLW ( RV32FC-only ) */
> -    return false;
> +    REQUIRE_FPU;
> +    REQUIRE_EXT(ctx, RVF);
> +
> +    arg_c_lw tmp;
> +    decode_insn16_extract_cl_w(&tmp, ctx->opcode);
> +    arg_flw arg = { .rd = tmp.rd, .rs1 = tmp.rs1, .imm = tmp.uimm };
> +    return trans_flw(ctx, &arg);
>  #else
>      /* C.LD ( RV64C/RV128C-only ) */
> -    return false;
> +    arg_c_fld tmp;
> +    decode_insn16_extract_cl_d(&tmp, ctx->opcode);
> +    arg_ld arg = { .rd = tmp.rd, .rs1 = tmp.rs1, .imm = tmp.uimm };
> +    return trans_ld(ctx, &arg);
>  #endif
>  }
>
> @@ -67,10 +76,19 @@ static bool trans_c_fsw_sd(DisasContext *ctx, arg_c_fsw_sd *a)
>  {
>  #ifdef TARGET_RISCV32
>      /* C.FSW ( RV32FC-only ) */
> -    return false;
> +    REQUIRE_FPU;
> +    REQUIRE_EXT(ctx, RVF);
> +
> +    arg_c_sw tmp;
> +    decode_insn16_extract_cs_w(&tmp, ctx->opcode);
> +    arg_fsw arg = { .rs1 = tmp.rs1, .rs2 = tmp.rs2, .imm = tmp.uimm };
> +    return trans_fsw(ctx, &arg);
>  #else
>      /* C.SD ( RV64C/RV128C-only ) */
> -    return false;
> +    arg_c_fsd tmp;
> +    decode_insn16_extract_cs_d(&tmp, ctx->opcode);
> +    arg_sd arg = { .rs1 = tmp.rs1, .rs2 = tmp.rs2, .imm = tmp.uimm };
> +    return trans_sd(ctx, &arg);
>  #endif
>  }
>
> @@ -88,7 +106,9 @@ static bool trans_c_jal_addiw(DisasContext *ctx, arg_c_jal_addiw *a)
>  {
>  #ifdef TARGET_RISCV32
>      /* C.JAL */
> -    arg_jal arg = { .rd = 1, .imm = a->imm };
> +    arg_c_j tmp;
> +    decode_insn16_extract_cj(&tmp, ctx->opcode);
> +    arg_jal arg = { .rd = 1, .imm = tmp.imm };
>      return trans_jal(ctx, &arg);
>  #else
>      /* C.ADDIW */

Thanks!  This fixes my simple test case, so I'm going to pick it up for the 
next PR.  It also sounds like it's time for me to start testing rv32 Linux 
boots, so I'll go do that next...

Re: [Qemu-devel] [PATCH] target/riscv: Fix manually parsed 16 bit insn
Posted by Alistair Francis 5 years, 1 month ago
On Fri, Mar 15, 2019 at 7:15 AM Bastian Koppelmann
<kbastian@mail.uni-paderborn.de> wrote:
>
> during the refactor to decodetree we removed the manual decoding that is
> necessary for c.jal/c.addiw and removed the translation of c.flw/c.ld
> and c.fsw/c.sd. This reintroduces the manual parsing and the
> omited implementation.
>
> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Tested-by: Alistair Francis <alistair.francis@wdc.com>

This fixes the 32-bit failures I was seeing.

Alistair

> ---
>  target/riscv/insn_trans/trans_rvc.inc.c | 30 ++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvc.inc.c b/target/riscv/insn_trans/trans_rvc.inc.c
> index bcdf64d3b7..5819f53f90 100644
> --- a/target/riscv/insn_trans/trans_rvc.inc.c
> +++ b/target/riscv/insn_trans/trans_rvc.inc.c
> @@ -44,10 +44,19 @@ static bool trans_c_flw_ld(DisasContext *ctx, arg_c_flw_ld *a)
>  {
>  #ifdef TARGET_RISCV32
>      /* C.FLW ( RV32FC-only ) */
> -    return false;
> +    REQUIRE_FPU;
> +    REQUIRE_EXT(ctx, RVF);
> +
> +    arg_c_lw tmp;
> +    decode_insn16_extract_cl_w(&tmp, ctx->opcode);
> +    arg_flw arg = { .rd = tmp.rd, .rs1 = tmp.rs1, .imm = tmp.uimm };
> +    return trans_flw(ctx, &arg);
>  #else
>      /* C.LD ( RV64C/RV128C-only ) */
> -    return false;
> +    arg_c_fld tmp;
> +    decode_insn16_extract_cl_d(&tmp, ctx->opcode);
> +    arg_ld arg = { .rd = tmp.rd, .rs1 = tmp.rs1, .imm = tmp.uimm };
> +    return trans_ld(ctx, &arg);
>  #endif
>  }
>
> @@ -67,10 +76,19 @@ static bool trans_c_fsw_sd(DisasContext *ctx, arg_c_fsw_sd *a)
>  {
>  #ifdef TARGET_RISCV32
>      /* C.FSW ( RV32FC-only ) */
> -    return false;
> +    REQUIRE_FPU;
> +    REQUIRE_EXT(ctx, RVF);
> +
> +    arg_c_sw tmp;
> +    decode_insn16_extract_cs_w(&tmp, ctx->opcode);
> +    arg_fsw arg = { .rs1 = tmp.rs1, .rs2 = tmp.rs2, .imm = tmp.uimm };
> +    return trans_fsw(ctx, &arg);
>  #else
>      /* C.SD ( RV64C/RV128C-only ) */
> -    return false;
> +    arg_c_fsd tmp;
> +    decode_insn16_extract_cs_d(&tmp, ctx->opcode);
> +    arg_sd arg = { .rs1 = tmp.rs1, .rs2 = tmp.rs2, .imm = tmp.uimm };
> +    return trans_sd(ctx, &arg);
>  #endif
>  }
>
> @@ -88,7 +106,9 @@ static bool trans_c_jal_addiw(DisasContext *ctx, arg_c_jal_addiw *a)
>  {
>  #ifdef TARGET_RISCV32
>      /* C.JAL */
> -    arg_jal arg = { .rd = 1, .imm = a->imm };
> +    arg_c_j tmp;
> +    decode_insn16_extract_cj(&tmp, ctx->opcode);
> +    arg_jal arg = { .rd = 1, .imm = tmp.imm };
>      return trans_jal(ctx, &arg);
>  #else
>      /* C.ADDIW */
> --
> 2.21.0
>
>