diff mbox series

[16/28] target/riscv: Convert quadrant 1 of RVXC insns to decodetree

Message ID 20181012173047.25420-17-kbastian@mail.uni-paderborn.de
State New
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/insn16.decode              |  43 +++++++
 target/riscv/insn_trans/trans_rvc.inc.c | 150 ++++++++++++++++++++++++
 target/riscv/translate.c                | 118 +------------------
 3 files changed, 194 insertions(+), 117 deletions(-)

Comments

Richard Henderson Oct. 13, 2018, 6:53 p.m. UTC | #1 
On 10/12/18 10:30 AM, Bastian Koppelmann wrote:
> +static bool trans_c_addi(DisasContext *ctx, arg_c_addi *a, uint16_t insn)
> +{
> +    if (a->imm == 0) {
> +        return true;
> +    }

return false, I think.

> +static bool trans_c_jal_addiw(DisasContext *ctx, arg_c_jal_addiw *a,
> +        uint16_t insn)
> +{
> +#ifdef TARGET_RISCV32
> +    /* C.JAL */
> +    arg_c_j *tmp = g_new0(arg_c_j, 1);
> +    extract_cj(tmp, insn);

Again with the g_new0 without free, which should use stack.

> +    arg_jal arg = { .rd = 1, .imm = a->imm };
> +    return trans_jal(ctx, &arg, insn);
> +#else
> +    /* C.ADDIW */
> +    arg_addiw arg = { .rd = a->rd, .rs1 = a->rd, .imm = a->imm };
> +    return trans_addiw(ctx, &arg, insn);
> +#endif
> +}
> +
> +static bool trans_c_li(DisasContext *ctx, arg_c_li *a, uint16_t insn)
> +{
> +    if (a->rd == 0) {
> +        return true;
> +    }

return false.

> +static bool trans_c_addi16sp_lui(DisasContext *ctx, arg_c_addi16sp_lui *a,
> +        uint16_t insn)
> +{
> +    if (a->rd == 2) {
> +        /* C.ADDI16SP */
> +        arg_addi arg = { .rd = 2, .rs1 = 2, .imm = a->imm_addi16sp };
> +        return trans_addi(ctx, &arg, insn);
> +    } else if (a->imm_lui != 0) {
> +        if (a->rd == 0) {
> +            return true;
> +        }

I think it should be

  } else if (a->imm_lui != 0 && a->rd != 0) {

> +        /* C.LUI */
> +        arg_lui arg = { .rd = a->rd, .imm = a->imm_lui };
> +        return trans_lui(ctx, &arg, insn);
> +    }
> +    return false;
> +}
> +
> +static bool trans_c_srli(DisasContext *ctx, arg_c_srli *a, uint16_t insn)
> +{
> +    if (a->shamt == 0) {
> +        /* Reserved in ISA */
> +        gen_exception_illegal(ctx);
> +        return true;
> +    }

Choose return false or raise exception.  Except...
I wonder if we might write this as

    int shamt = a->shamt;
    if (shamt == 0) {
        shamt = 64;
    }

> +#ifdef TARGET_RISCV32
> +    /* Ensure, that shamt[5] is zero for RV32 */
> +    if (a->shamt >= 32) {
> +        gen_exception_illegal(ctx);
> +        return true;
> +    }
> +#endif

then this is unconditional as

    if (a->shamt >= TARGET_LONG_BITS)

which makes it clear that "reserved in isa" already has a meaning.

> +
> +    arg_srli arg = { .rd = a->rd, .rs1 = a->rd, .shamt = a->shamt };
> +    return trans_srli(ctx, &arg, insn);

Although I do wonder about moving that check into trans_srli et al, rather than
bend over backward parsing 6-bit shift amount rather just using @i format.


r~
Bastian Koppelmann Oct. 19, 2018, 1:20 p.m. UTC | #2 
On 10/13/18 8:53 PM, Richard Henderson wrote:
> Choose return false or raise exception. Except...
> I wonder if we might write this as
>
>      int shamt = a->shamt;
>      if (shamt == 0) {
>          shamt = 64;
>      }


Good catch. I'll add a comment, that a shamt of 0 is intended for RV128.


Cheers,

Bastian
Bastian Koppelmann Oct. 19, 2018, 3:28 p.m. UTC | #3 
On 10/13/18 8:53 PM, Richard Henderson wrote:
> On 10/12/18 10:30 AM, Bastian Koppelmann wrote:
>> +static bool trans_c_addi(DisasContext *ctx, arg_c_addi *a, uint16_t insn)
>> +{
>> +    if (a->imm == 0) {
>> +        return true;
>> +    }
> return false, I think.


Those are HINTS, which means the instruction in valid, but does not 
affect state, so true is correct. If I do return false, then Linux does 
not boot anymore :)


>
>> +    arg_jal arg = { .rd = 1, .imm = a->imm };
>> +    return trans_jal(ctx, &arg, insn);
>> +#else
>> +    /* C.ADDIW */
>> +    arg_addiw arg = { .rd = a->rd, .rs1 = a->rd, .imm = a->imm };
>> +    return trans_addiw(ctx, &arg, insn);
>> +#endif
>> +}
>> +
>> +static bool trans_c_li(DisasContext *ctx, arg_c_li *a, uint16_t insn)
>> +{
>> +    if (a->rd == 0) {
>> +        return true;
>> +    }
> return false.


Likewise.


>
>> +static bool trans_c_addi16sp_lui(DisasContext *ctx, arg_c_addi16sp_lui *a,
>> +        uint16_t insn)
>> +{
>> +    if (a->rd == 2) {
>> +        /* C.ADDI16SP */
>> +        arg_addi arg = { .rd = 2, .rs1 = 2, .imm = a->imm_addi16sp };
>> +        return trans_addi(ctx, &arg, insn);
>> +    } else if (a->imm_lui != 0) {
>> +        if (a->rd == 0) {
>> +            return true;
>> +        }
> I think it should be
>
>    } else if (a->imm_lui != 0 && a->rd != 0) {

Likewise.


Cheers,

Bastian
Richard Henderson Oct. 19, 2018, 3:38 p.m. UTC | #4 
On 10/19/18 8:28 AM, Bastian Koppelmann wrote:
> 
> On 10/13/18 8:53 PM, Richard Henderson wrote:
>> On 10/12/18 10:30 AM, Bastian Koppelmann wrote:
>>> +static bool trans_c_addi(DisasContext *ctx, arg_c_addi *a, uint16_t insn)
>>> +{
>>> +    if (a->imm == 0) {
>>> +        return true;
>>> +    }
>> return false, I think.
> 
> 
> Those are HINTS, which means the instruction in valid, but does not affect
> state, so true is correct. If I do return false, then Linux does not boot
> anymore :)

Ah, this information is not present in the main body of the text, only in table
12.5.  It might be good to add comments to the code here.

Thanks.


r~
Palmer Dabbelt Oct. 19, 2018, 6:49 p.m. UTC | #5 
On Fri, 19 Oct 2018 08:28:38 PDT (-0700), kbastian@mail.uni-paderborn.de wrote:
> On 10/13/18 8:53 PM, Richard Henderson wrote:
>> On 10/12/18 10:30 AM, Bastian Koppelmann wrote:
>>> +static bool trans_c_addi(DisasContext *ctx, arg_c_addi *a, uint16_t insn)
>>> +{
>>> +    if (a->imm == 0) {
>>> +        return true;
>>> +    }
>> return false, I think.
>
>
> Those are HINTS, which means the instruction in valid, but does not
> affect state, so true is correct. If I do return false, then Linux does
> not boot anymore :)

Technically, "c.addi x0, 0" is an illegal instruction.  It just so happens, 
however, that the encoding that would arise from "c.addi x0, 0" is instead the 
legal "c.nop" instruction, which happens to have exactly the same effect as a 
"c.addi x0, 0".  No idea why the spec is written this way.

So I guess you're both correct: "trans_c_addi" should treat this as invalid, as 
it's not an addi.  The processor's behavior will still be correct with this 
implementation, though, so I don't see this as a distinction worth worrying 
about.

>>> +    arg_jal arg = { .rd = 1, .imm = a->imm };
>>> +    return trans_jal(ctx, &arg, insn);
>>> +#else
>>> +    /* C.ADDIW */
>>> +    arg_addiw arg = { .rd = a->rd, .rs1 = a->rd, .imm = a->imm };
>>> +    return trans_addiw(ctx, &arg, insn);
>>> +#endif
>>> +}
>>> +
>>> +static bool trans_c_li(DisasContext *ctx, arg_c_li *a, uint16_t insn)
>>> +{
>>> +    if (a->rd == 0) {
>>> +        return true;
>>> +    }
>> return false.
>
>
> Likewise.

In this case I believe "li x0, *" should be invalid.  According to v2.2

    C.LI loads the sign-extended 6-bit immediate, imm, into register rd. C.LI 
    is only valid when rd6 = x0.
    C.LI expands into addi rd, x0, imm[5:0].

>>> +static bool trans_c_addi16sp_lui(DisasContext *ctx, arg_c_addi16sp_lui *a,
>>> +        uint16_t insn)
>>> +{
>>> +    if (a->rd == 2) {
>>> +        /* C.ADDI16SP */
>>> +        arg_addi arg = { .rd = 2, .rs1 = 2, .imm = a->imm_addi16sp };
>>> +        return trans_addi(ctx, &arg, insn);
>>> +    } else if (a->imm_lui != 0) {
>>> +        if (a->rd == 0) {
>>> +            return true;
>>> +        }
>> I think it should be
>>
>>    } else if (a->imm_lui != 0 && a->rd != 0) {
>
> Likewise.

Yes, c.lui is invalid with a non-zero immediate.  Again, according to v2.2

    C.LUI loads the non-zero 6-bit immediate field into bits 17–12 of the 
    destination register, clears the bottom 12 bits, and sign-extends bit 17 
    into all higher bits of the destination. C.LUI is only valid when rd6 = 
    {x0, x2}, and when the immediate is not equal to zero.  C.LUI expands into 
    lui rd, nzuimm[17:12].
diff mbox series

Patch

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 558c0c41f0..29dade0fa1 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -22,28 +22,53 @@ 
 %rs2_3     2:3                !function=ex_rvc_register
 
 # Immediates:
+%imm_ci        12:s1 2:5
 %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
+%imm_cb        12:s1 5:2 2:1 10:2 3:2 !function=ex_shift_1
+%imm_cj        12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1
+
+%nzuimm_6bit   12:1 2:5
+
+%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:
 &cl               rs1 rd
 &cl_dw     uimm   rs1 rd
+&ci        imm        rd
 &ciw       nzuimm     rd
 &cs               rs1 rs2
 &cs_dw     uimm   rs1 rs2
+&cb        imm    rs1
+&cr               rd  rs2
+&c_j       imm
+&c_shift   shamt      rd
+
 
+&c_addi16sp_lui  imm_lui imm_addi16sp rd
 
 # Formats 16:
+@ci        ... . ..... .....  .. &ci     imm=%imm_ci                  %rd
 @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_2      ... ... ... .. ... .. &cr                      rd=%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
+@cj        ...    ........... .. &c_j    imm=%imm_cj
 
+@c_addi16sp_lui ... .  ..... ..... .. &c_addi16sp_lui %imm_lui %imm_addi16sp %rd
+
+@c_shift        ... . .. ... ..... .. &c_shift rd=%rs1_3 shamt=%nzuimm_6bit
+
+@c_andi         ... . .. ... ..... .. &ci imm=%imm_ci rd=%rs1_3
 
 # *** RV64C Standard Extension (Quadrant 0) ***
 c_addi4spn        000    ........ ... 00 @ciw
@@ -53,3 +78,21 @@  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
+
+# *** RV64C Standard Extension (Quadrant 1) ***
+c_addi            000 .  .....  ..... 01 @ci
+c_jal_addiw       001 .  .....  ..... 01 @ci #Note: parse rd and/or imm manually
+c_li              010 .  .....  ..... 01 @ci
+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
+c_j               101     ........... 01 @cj
+c_beqz            110  ... ...  ..... 01 @cb
+c_bnez            111  ... ...  ..... 01 @cb
diff --git a/target/riscv/insn_trans/trans_rvc.inc.c b/target/riscv/insn_trans/trans_rvc.inc.c
index f8ad2db527..74cb4dad0a 100644
--- a/target/riscv/insn_trans/trans_rvc.inc.c
+++ b/target/riscv/insn_trans/trans_rvc.inc.c
@@ -87,3 +87,153 @@  static bool trans_c_fsw_sd(DisasContext *ctx, arg_c_fsw_sd *a, uint16_t insn)
     return trans_sd(ctx, &arg, insn);
 #endif
 }
+
+static bool trans_c_addi(DisasContext *ctx, arg_c_addi *a, uint16_t insn)
+{
+    if (a->imm == 0) {
+        return true;
+    }
+    arg_addi arg = { .rd = a->rd, .rs1 = a->rd, .imm = a->imm };
+    return trans_addi(ctx, &arg, insn);
+}
+
+static bool trans_c_jal_addiw(DisasContext *ctx, arg_c_jal_addiw *a,
+        uint16_t insn)
+{
+#ifdef TARGET_RISCV32
+    /* C.JAL */
+    arg_c_j *tmp = g_new0(arg_c_j, 1);
+    extract_cj(tmp, insn);
+    arg_jal arg = { .rd = 1, .imm = a->imm };
+    return trans_jal(ctx, &arg, insn);
+#else
+    /* C.ADDIW */
+    arg_addiw arg = { .rd = a->rd, .rs1 = a->rd, .imm = a->imm };
+    return trans_addiw(ctx, &arg, insn);
+#endif
+}
+
+static bool trans_c_li(DisasContext *ctx, arg_c_li *a, uint16_t insn)
+{
+    if (a->rd == 0) {
+        return true;
+    }
+    arg_addi arg = { .rd = a->rd, .rs1 = 0, .imm = a->imm };
+    return trans_addi(ctx, &arg, insn);
+}
+
+static bool trans_c_addi16sp_lui(DisasContext *ctx, arg_c_addi16sp_lui *a,
+        uint16_t insn)
+{
+    if (a->rd == 2) {
+        /* C.ADDI16SP */
+        arg_addi arg = { .rd = 2, .rs1 = 2, .imm = a->imm_addi16sp };
+        return trans_addi(ctx, &arg, insn);
+    } else if (a->imm_lui != 0) {
+        if (a->rd == 0) {
+            return true;
+        }
+        /* C.LUI */
+        arg_lui arg = { .rd = a->rd, .imm = a->imm_lui };
+        return trans_lui(ctx, &arg, insn);
+    }
+    return false;
+}
+
+static bool trans_c_srli(DisasContext *ctx, arg_c_srli *a, uint16_t insn)
+{
+    if (a->shamt == 0) {
+        /* Reserved in ISA */
+        gen_exception_illegal(ctx);
+        return true;
+    }
+#ifdef TARGET_RISCV32
+    /* Ensure, that shamt[5] is zero for RV32 */
+    if (a->shamt >= 32) {
+        gen_exception_illegal(ctx);
+        return true;
+    }
+#endif
+
+    arg_srli arg = { .rd = a->rd, .rs1 = a->rd, .shamt = a->shamt };
+    return trans_srli(ctx, &arg, insn);
+}
+
+static bool trans_c_srai(DisasContext *ctx, arg_c_srai *a, uint16_t insn)
+{
+    if (a->shamt == 0) {
+        /* Reserved in ISA */
+        gen_exception_illegal(ctx);
+        return true;
+    }
+#ifdef TARGET_RISCV32
+    /* Ensure, that shamt[5] is zero for RV32 */
+    if (a->shamt >= 32) {
+        gen_exception_illegal(ctx);
+        return true;
+    }
+#endif
+
+    arg_srai arg = { .rd = a->rd, .rs1 = a->rd, .shamt = a->shamt };
+    return trans_srai(ctx, &arg, insn);
+}
+
+static bool trans_c_andi(DisasContext *ctx, arg_c_andi *a, uint16_t insn)
+{
+    arg_andi arg = { .rd = a->rd, .rs1 = a->rd, .imm = a->imm };
+    return trans_andi(ctx, &arg, insn);
+}
+
+static bool trans_c_sub(DisasContext *ctx, arg_c_sub *a, uint16_t insn)
+{
+    arg_sub arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
+    return trans_sub(ctx, &arg, insn);
+}
+
+static bool trans_c_xor(DisasContext *ctx, arg_c_xor *a, uint16_t insn)
+{
+    arg_xor arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
+    return trans_xor(ctx, &arg, insn);
+}
+
+static bool trans_c_or(DisasContext *ctx, arg_c_or *a, uint16_t insn)
+{
+    arg_or arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
+    return trans_or(ctx, &arg, insn);
+}
+
+static bool trans_c_and(DisasContext *ctx, arg_c_and *a, uint16_t insn)
+{
+    arg_and arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
+    return trans_and(ctx, &arg, insn);
+}
+
+static bool trans_c_subw(DisasContext *ctx, arg_c_subw *a, uint16_t insn)
+{
+    arg_subw arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
+    return trans_subw(ctx, &arg, insn);
+}
+
+static bool trans_c_addw(DisasContext *ctx, arg_c_addw *a, uint16_t insn)
+{
+    arg_addw arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
+    return trans_addw(ctx, &arg, insn);
+}
+
+static bool trans_c_j(DisasContext *ctx, arg_c_j *a, uint16_t insn)
+{
+    arg_jal arg = { .rd = 0, .imm = a->imm };
+    return trans_jal(ctx, &arg, insn);
+}
+
+static bool trans_c_beqz(DisasContext *ctx, arg_c_beqz *a, uint16_t insn)
+{
+    arg_beq arg = { .rs1 = a->rs1, .rs2 = 0, .imm = a->imm };
+    return trans_beq(ctx, &arg, insn);
+}
+
+static bool trans_c_bnez(DisasContext *ctx, arg_c_bnez *a, uint16_t insn)
+{
+    arg_bne arg = { .rs1 = a->rs1, .rs2 = 0, .imm = a->imm };
+    return trans_bne(ctx, &arg, insn);
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index a4bf7f98ab..3d2146c9b2 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -713,120 +713,6 @@  static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc,
     }
 }
 
-static void decode_RV32_64C1(CPURISCVState *env, DisasContext *ctx)
-{
-    uint8_t funct3 = extract32(ctx->opcode, 13, 3);
-    uint8_t rd_rs1 = GET_C_RS1(ctx->opcode);
-    uint8_t rs1s, rs2s;
-    uint8_t funct2;
-
-    switch (funct3) {
-    case 0:
-        /* C.ADDI -> addi rd, rd, nzimm[5:0] */
-        gen_arith_imm(ctx, OPC_RISC_ADDI, rd_rs1, rd_rs1,
-                      GET_C_IMM(ctx->opcode));
-        break;
-    case 1:
-#if defined(TARGET_RISCV64)
-        /* C.ADDIW (RV64/128) -> addiw rd, rd, imm[5:0]*/
-        gen_arith_imm(ctx, OPC_RISC_ADDIW, rd_rs1, rd_rs1,
-                      GET_C_IMM(ctx->opcode));
-#else
-        /* C.JAL(RV32) -> jal x1, offset[11:1] */
-        gen_jal(env, ctx, 1, GET_C_J_IMM(ctx->opcode));
-#endif
-        break;
-    case 2:
-        /* C.LI -> addi rd, x0, imm[5:0]*/
-        gen_arith_imm(ctx, OPC_RISC_ADDI, rd_rs1, 0, GET_C_IMM(ctx->opcode));
-        break;
-    case 3:
-        if (rd_rs1 == 2) {
-            /* C.ADDI16SP -> addi x2, x2, nzimm[9:4]*/
-            gen_arith_imm(ctx, OPC_RISC_ADDI, 2, 2,
-                          GET_C_ADDI16SP_IMM(ctx->opcode));
-        } else if (rd_rs1 != 0) {
-            /* C.LUI (rs1/rd =/= {0,2}) -> lui rd, nzimm[17:12]*/
-            tcg_gen_movi_tl(cpu_gpr[rd_rs1],
-                            GET_C_IMM(ctx->opcode) << 12);
-        }
-        break;
-    case 4:
-        funct2 = extract32(ctx->opcode, 10, 2);
-        rs1s = GET_C_RS1S(ctx->opcode);
-        switch (funct2) {
-        case 0: /* C.SRLI(RV32) -> srli rd', rd', shamt[5:0] */
-            gen_arith_imm(ctx, OPC_RISC_SHIFT_RIGHT_I, rs1s, rs1s,
-                               GET_C_ZIMM(ctx->opcode));
-            /* C.SRLI64(RV128) */
-            break;
-        case 1:
-            /* C.SRAI -> srai rd', rd', shamt[5:0]*/
-            gen_arith_imm(ctx, OPC_RISC_SHIFT_RIGHT_I, rs1s, rs1s,
-                            GET_C_ZIMM(ctx->opcode) | 0x400);
-            /* C.SRAI64(RV128) */
-            break;
-        case 2:
-            /* C.ANDI -> andi rd', rd', imm[5:0]*/
-            gen_arith_imm(ctx, OPC_RISC_ANDI, rs1s, rs1s,
-                          GET_C_IMM(ctx->opcode));
-            break;
-        case 3:
-            funct2 = extract32(ctx->opcode, 5, 2);
-            rs2s = GET_C_RS2S(ctx->opcode);
-            switch (funct2) {
-            case 0:
-                /* C.SUB -> sub rd', rd', rs2' */
-                if (extract32(ctx->opcode, 12, 1) == 0) {
-                    gen_arith(ctx, OPC_RISC_SUB, rs1s, rs1s, rs2s);
-                }
-#if defined(TARGET_RISCV64)
-                else {
-                    gen_arith(ctx, OPC_RISC_SUBW, rs1s, rs1s, rs2s);
-                }
-#endif
-                break;
-            case 1:
-                /* C.XOR -> xor rs1', rs1', rs2' */
-                if (extract32(ctx->opcode, 12, 1) == 0) {
-                    gen_arith(ctx, OPC_RISC_XOR, rs1s, rs1s, rs2s);
-                }
-#if defined(TARGET_RISCV64)
-                else {
-                    /* C.ADDW (RV64/128) */
-                    gen_arith(ctx, OPC_RISC_ADDW, rs1s, rs1s, rs2s);
-                }
-#endif
-                break;
-            case 2:
-                /* C.OR -> or rs1', rs1', rs2' */
-                gen_arith(ctx, OPC_RISC_OR, rs1s, rs1s, rs2s);
-                break;
-            case 3:
-                /* C.AND -> and rs1', rs1', rs2' */
-                gen_arith(ctx, OPC_RISC_AND, rs1s, rs1s, rs2s);
-                break;
-            }
-            break;
-        }
-        break;
-    case 5:
-        /* C.J -> jal x0, offset[11:1]*/
-        gen_jal(env, ctx, 0, GET_C_J_IMM(ctx->opcode));
-        break;
-    case 6:
-        /* C.BEQZ -> beq rs1', x0, offset[8:1]*/
-        rs1s = GET_C_RS1S(ctx->opcode);
-        gen_branch(env, ctx, OPC_RISC_BEQ, rs1s, 0, GET_C_B_IMM(ctx->opcode));
-        break;
-    case 7:
-        /* C.BNEZ -> bne rs1', x0, offset[8:1]*/
-        rs1s = GET_C_RS1S(ctx->opcode);
-        gen_branch(env, ctx, OPC_RISC_BNE, rs1s, 0, GET_C_B_IMM(ctx->opcode));
-        break;
-    }
-}
-
 static void decode_RV32_64C2(CPURISCVState *env, DisasContext *ctx)
 {
     uint8_t rd, rs2;
@@ -910,9 +796,6 @@  static void decode_RV32_64C(CPURISCVState *env, DisasContext *ctx)
     uint8_t op = extract32(ctx->opcode, 0, 2);
 
     switch (op) {
-    case 1:
-        decode_RV32_64C1(env, ctx);
-        break;
     case 2:
         decode_RV32_64C2(env, ctx);
         break;
@@ -927,6 +810,7 @@  static void decode_RV32_64C(CPURISCVState *env, DisasContext *ctx)
 EX_SH(1)
 EX_SH(2)
 EX_SH(3)
+EX_SH(4)
 EX_SH(12)
 
 static int64_t ex_rvc_register(int reg)