diff mbox series

[15/28] target/riscv: Convert quadrant 0 of RVXC insns to decodetree

Message ID 20181012173047.25420-16-kbastian@mail.uni-paderborn.de (mailing list archive)
State New, archived
Headers show
Series target/riscv: Convert to decodetree | expand

Commit Message

Bastian Koppelmann Oct. 12, 2018, 5:30 p.m. UTC
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Peer Adelt <peer.adelt@hni.uni-paderborn.de>
---
 target/riscv/Makefile.objs              |  9 ++-
 target/riscv/insn16.decode              | 55 +++++++++++++++
 target/riscv/insn_trans/trans_rvc.inc.c | 89 +++++++++++++++++++++++++
 target/riscv/translate.c                | 88 +++++-------------------
 4 files changed, 168 insertions(+), 73 deletions(-)
 create mode 100644 target/riscv/insn16.decode
 create mode 100644 target/riscv/insn_trans/trans_rvc.inc.c

Comments

Richard Henderson Oct. 13, 2018, 6:18 p.m. UTC | #1
On 10/12/18 10:30 AM, Bastian Koppelmann wrote:
> +# Argument sets:
> +&cl               rs1 rd
> +&cl_dw     uimm   rs1 rd
> +&ciw       nzuimm     rd
> +&cs               rs1 rs2
> +&cs_dw     uimm   rs1 rs2

I guess this is good enough for now.
What I'd like to see is something like

&i	imm rs1 rd	!extern

which tells decodetree that the arg_i structure has been declared elsewhere.
Then you can write

> +@cl_w      ... ... ... .. ... .. &i  imm=%uimm_cl_w  rs1=%rs1_3  rd=%rs2_3

lw	010 ... ... .. ... 00	@cl_w

and then the same trans_lw can be used.

> +c_flw_ld          011  --- ... -- ... 00 @cl    #Note: Must parse uimm manually

Why?  It looks identical to @cl_w to me.

> +static bool trans_c_addi4spn(DisasContext *ctx, arg_c_addi4spn *a,
> +        uint16_t insn)
> +{
> +    if (a->nzuimm == 0) {
> +        /* Reserved in ISA */
> +        gen_exception_illegal(ctx);
> +        return true;

This should definitely be return false, as that kind of instruction overlap is
exactly what the return value is for.  (There is no current support for
overlapping insn patterns, but it is planned.)

> +static bool trans_c_flw_ld(DisasContext *ctx, arg_c_flw_ld *a, uint16_t insn)
> +{
> +#ifdef TARGET_RISCV32
> +    /* C.FLW ( RV32FC-only ) */
> +    arg_c_lw *tmp = g_new0(arg_c_lw, 1);
> +    extract_cl_w(tmp, insn);
> +    arg_flw arg = { .rd = tmp->rd, .rs1 = tmp->rs1, .imm = tmp->uimm };
> +    return trans_flw(ctx, &arg, insn);
> +#else
> +    /* C.LD ( RV64C/RV128C-only ) */
> +    arg_c_fld *tmp = g_new0(arg_c_fld, 1);
> +    extract_cl_d(tmp, insn);
> +    arg_ld arg = { .rd = tmp->rd, .rs1 = tmp->rs1, .imm = tmp->uimm };
> +    return trans_ld(ctx, &arg, insn);
> +#endif

Oh I see what you're after wrt manual parsing above.
But why g_new0 and missing free.  Why not stack allocation?

> +static int64_t ex_rvc_register(int reg)

return type should be int.


r~
Bastian Koppelmann Oct. 19, 2018, 12:49 p.m. UTC | #2
On 10/13/18 8:18 PM, Richard Henderson wrote:
> On 10/12/18 10:30 AM, Bastian Koppelmann wrote:
>> +# Argument sets:
>> +&cl               rs1 rd
>> +&cl_dw     uimm   rs1 rd
>> +&ciw       nzuimm     rd
>> +&cs               rs1 rs2
>> +&cs_dw     uimm   rs1 rs2
> I guess this is good enough for now.
> What I'd like to see is something like
>
> &i	imm rs1 rd	!extern


That looks so much better. Can you CC me when you send the patches for 
the extern support, so I can implement this for RISC-V?


Cheers,

Bastian
diff mbox series

Patch

diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs
index ea02f9b9ef..ec7326f1c7 100644
--- a/target/riscv/Makefile.objs
+++ b/target/riscv/Makefile.objs
@@ -8,4 +8,11 @@  target/riscv/decode_insn32.inc.c: \
 	  $(PYTHON) $(DECODETREE) -o $@ --decode decode_insn32 $<, \
 	  "GEN", $(TARGET_DIR)$@)
 
-target/riscv/translate.o: target/riscv/decode_insn32.inc.c
+target/riscv/decode_insn16.inc.c: \
+  $(SRC_PATH)/target/riscv/insn16.decode $(DECODETREE)
+	$(call quiet-command, \
+	  $(PYTHON) $(DECODETREE) -o $@ --decode decode_insn16 --insnwidth 16 $<, \
+	  "GEN", $(TARGET_DIR)$@)
+
+target/riscv/translate.o: target/riscv/decode_insn32.inc.c \
+	target/riscv/decode_insn16.inc.c
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
new file mode 100644
index 0000000000..558c0c41f0
--- /dev/null
+++ b/target/riscv/insn16.decode
@@ -0,0 +1,55 @@ 
+#
+# RISC-V translation routines for the RVXI Base Integer Instruction Set.
+#
+# Copyright (c) 2018 Peer Adelt, peer.adelt@hni.uni-paderborn.de
+#                    Bastian Koppelmann, kbastian@mail.uni-paderborn.de
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms and conditions of the GNU General Public License,
+# version 2 or later, as published by the Free Software Foundation.
+#
+# This program is distributed in the hope it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Fields:
+%rd        7:5
+%rs1_3     7:3                !function=ex_rvc_register
+%rs2_3     2:3                !function=ex_rvc_register
+
+# Immediates:
+%nzuimm_ciw    7:4 11:2 5:1 6:1   !function=ex_shift_2
+%uimm_cl_d     5:2 10:3           !function=ex_shift_3
+%uimm_cl_w     5:1 10:3 6:1       !function=ex_shift_2
+
+
+# Argument sets:
+&cl               rs1 rd
+&cl_dw     uimm   rs1 rd
+&ciw       nzuimm     rd
+&cs               rs1 rs2
+&cs_dw     uimm   rs1 rs2
+
+
+# Formats 16:
+@ciw       ...   ........ ... .. &ciw    nzuimm=%nzuimm_ciw           rd=%rs2_3
+@cl_d      ... ... ... .. ... .. &cl_dw  uimm=%uimm_cl_d  rs1=%rs1_3  rd=%rs2_3
+@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_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
+
+
+# *** RV64C Standard Extension (Quadrant 0) ***
+c_addi4spn        000    ........ ... 00 @ciw
+c_fld             001  ... ... .. ... 00 @cl_d
+c_lw              010  ... ... .. ... 00 @cl_w
+c_flw_ld          011  --- ... -- ... 00 @cl    #Note: Must parse uimm manually
+c_fsd             101  ... ... .. ... 00 @cs_d
+c_sw              110  ... ... .. ... 00 @cs_w
+c_fsw_sd          111  --- ... -- ... 00 @cs    #Note: Must parse uimm manually
diff --git a/target/riscv/insn_trans/trans_rvc.inc.c b/target/riscv/insn_trans/trans_rvc.inc.c
new file mode 100644
index 0000000000..f8ad2db527
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvc.inc.c
@@ -0,0 +1,89 @@ 
+/*
+ * RISC-V translation routines for the RVC Compressed Instruction Set.
+ *
+ * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
+ * Copyright (c) 2018 Peer Adelt, peer.adelt@hni.uni-paderborn.de
+ *                    Bastian Koppelmann, kbastian@mail.uni-paderborn.de
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+static bool trans_c_addi4spn(DisasContext *ctx, arg_c_addi4spn *a,
+        uint16_t insn)
+{
+    if (a->nzuimm == 0) {
+        /* Reserved in ISA */
+        gen_exception_illegal(ctx);
+        return true;
+    }
+    arg_addi arg = { .rd = a->rd, .rs1 = 2, .imm = a->nzuimm };
+    return trans_addi(ctx, &arg, insn);
+}
+
+static bool trans_c_fld(DisasContext *ctx, arg_c_fld *a, uint16_t insn)
+{
+    arg_fld arg = { .rd = a->rd, .rs1 = a->rs1, .imm = a->uimm };
+    return trans_fld(ctx, &arg, insn);
+}
+
+static bool trans_c_lw(DisasContext *ctx, arg_c_lw *a, uint16_t insn)
+{
+    arg_lw arg = { .rd = a->rd, .rs1 = a->rs1, .imm = a->uimm };
+    return trans_lw(ctx, &arg, insn);
+}
+
+static bool trans_c_flw_ld(DisasContext *ctx, arg_c_flw_ld *a, uint16_t insn)
+{
+#ifdef TARGET_RISCV32
+    /* C.FLW ( RV32FC-only ) */
+    arg_c_lw *tmp = g_new0(arg_c_lw, 1);
+    extract_cl_w(tmp, insn);
+    arg_flw arg = { .rd = tmp->rd, .rs1 = tmp->rs1, .imm = tmp->uimm };
+    return trans_flw(ctx, &arg, insn);
+#else
+    /* C.LD ( RV64C/RV128C-only ) */
+    arg_c_fld *tmp = g_new0(arg_c_fld, 1);
+    extract_cl_d(tmp, insn);
+    arg_ld arg = { .rd = tmp->rd, .rs1 = tmp->rs1, .imm = tmp->uimm };
+    return trans_ld(ctx, &arg, insn);
+#endif
+}
+
+static bool trans_c_fsd(DisasContext *ctx, arg_c_fsd *a, uint16_t insn)
+{
+    arg_fsd arg = { .rs1 = a->rs1, .rs2 = a->rs2, .imm = a->uimm };
+    return trans_fsd(ctx, &arg, insn);
+}
+
+static bool trans_c_sw(DisasContext *ctx, arg_c_sw *a, uint16_t insn)
+{
+    arg_sw arg = { .rs1 = a->rs1, .rs2 = a->rs2, .imm = a->uimm };
+    return trans_sw(ctx, &arg, insn);
+}
+
+static bool trans_c_fsw_sd(DisasContext *ctx, arg_c_fsw_sd *a, uint16_t insn)
+{
+#ifdef TARGET_RISCV32
+    /* C.FSW ( RV32FC-only ) */
+    arg_c_sw *tmp = g_new0(arg_c_sw, 1);
+    extract_cs_w(tmp, insn);
+    arg_fsw arg = { .rs1 = tmp->rs1, .rs2 = tmp->rs2, .imm = tmp->uimm };
+    return trans_fsw(ctx, &arg, insn);
+#else
+    /* C.SD ( RV64C/RV128C-only ) */
+    arg_c_fsd *tmp = g_new0(arg_c_fsd, 1);
+    extract_cs_d(tmp, insn);
+    arg_sd arg = { .rs1 = tmp->rs1, .rs2 = tmp->rs2, .imm = tmp->uimm };
+    return trans_sd(ctx, &arg, insn);
+#endif
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 49b1e9f8d7..a4bf7f98ab 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -713,74 +713,6 @@  static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc,
     }
 }
 
-static void decode_RV32_64C0(DisasContext *ctx)
-{
-    uint8_t funct3 = extract32(ctx->opcode, 13, 3);
-    uint8_t rd_rs2 = GET_C_RS2S(ctx->opcode);
-    uint8_t rs1s = GET_C_RS1S(ctx->opcode);
-
-    switch (funct3) {
-    case 0:
-        /* illegal */
-        if (ctx->opcode == 0) {
-            gen_exception_illegal(ctx);
-        } else {
-            /* C.ADDI4SPN -> addi rd', x2, zimm[9:2]*/
-            gen_arith_imm(ctx, OPC_RISC_ADDI, rd_rs2, 2,
-                          GET_C_ADDI4SPN_IMM(ctx->opcode));
-        }
-        break;
-    case 1:
-        /* C.FLD -> fld rd', offset[7:3](rs1')*/
-        gen_fp_load(ctx, OPC_RISC_FLD, rd_rs2, rs1s,
-                    GET_C_LD_IMM(ctx->opcode));
-        /* C.LQ(RV128) */
-        break;
-    case 2:
-        /* C.LW -> lw rd', offset[6:2](rs1') */
-        gen_load(ctx, OPC_RISC_LW, rd_rs2, rs1s,
-                 GET_C_LW_IMM(ctx->opcode));
-        break;
-    case 3:
-#if defined(TARGET_RISCV64)
-        /* C.LD(RV64/128) -> ld rd', offset[7:3](rs1')*/
-        gen_load(ctx, OPC_RISC_LD, rd_rs2, rs1s,
-                 GET_C_LD_IMM(ctx->opcode));
-#else
-        /* C.FLW (RV32) -> flw rd', offset[6:2](rs1')*/
-        gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
-                    GET_C_LW_IMM(ctx->opcode));
-#endif
-        break;
-    case 4:
-        /* reserved */
-        gen_exception_illegal(ctx);
-        break;
-    case 5:
-        /* C.FSD(RV32/64) -> fsd rs2', offset[7:3](rs1') */
-        gen_fp_store(ctx, OPC_RISC_FSD, rs1s, rd_rs2,
-                     GET_C_LD_IMM(ctx->opcode));
-        /* C.SQ (RV128) */
-        break;
-    case 6:
-        /* C.SW -> sw rs2', offset[6:2](rs1')*/
-        gen_store(ctx, OPC_RISC_SW, rs1s, rd_rs2,
-                  GET_C_LW_IMM(ctx->opcode));
-        break;
-    case 7:
-#if defined(TARGET_RISCV64)
-        /* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
-        gen_store(ctx, OPC_RISC_SD, rs1s, rd_rs2,
-                  GET_C_LD_IMM(ctx->opcode));
-#else
-        /* C.FSW (RV32) -> fsw rs2', offset[6:2](rs1')*/
-        gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
-                     GET_C_LW_IMM(ctx->opcode));
-#endif
-        break;
-    }
-}
-
 static void decode_RV32_64C1(CPURISCVState *env, DisasContext *ctx)
 {
     uint8_t funct3 = extract32(ctx->opcode, 13, 3);
@@ -978,9 +910,6 @@  static void decode_RV32_64C(CPURISCVState *env, DisasContext *ctx)
     uint8_t op = extract32(ctx->opcode, 0, 2);
 
     switch (op) {
-    case 0:
-        decode_RV32_64C0(ctx);
-        break;
     case 1:
         decode_RV32_64C1(env, ctx);
         break;
@@ -996,8 +925,15 @@  static void decode_RV32_64C(CPURISCVState *env, DisasContext *ctx)
         return imm << amount;                 \
     }
 EX_SH(1)
+EX_SH(2)
+EX_SH(3)
 EX_SH(12)
 
+static int64_t ex_rvc_register(int reg)
+{
+    return 8 + reg;
+}
+
 bool decode_insn32(DisasContext *ctx, uint32_t insn);
 /* Include the auto-generated decoder for 32 bit insn */
 #include "decode_insn32.inc.c"
@@ -1009,6 +945,11 @@  bool decode_insn32(DisasContext *ctx, uint32_t insn);
 #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_RV32_64G(CPURISCVState *env, DisasContext *ctx)
 {
     int rs1, rd;
@@ -1042,7 +983,10 @@  static void decode_opc(CPURISCVState *env, DisasContext *ctx)
             gen_exception_illegal(ctx);
         } else {
             ctx->pc_succ_insn = ctx->base.pc_next + 2;
-            decode_RV32_64C(env, ctx);
+            if (!decode_insn16(ctx, ctx->opcode)) {
+                /* fall back to old decoder */
+                decode_RV32_64C(env, ctx);
+            }
         }
     } else {
         ctx->pc_succ_insn = ctx->base.pc_next + 4;