* [PATCH v5 0/2] sim: riscv: Compressed instruction simulation
@ 2023-12-22 5:26 jaydeep.patil
2023-12-22 5:26 ` [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding jaydeep.patil
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: jaydeep.patil @ 2023-12-22 5:26 UTC (permalink / raw)
To: gdb-patches
Cc: aburgess, vapier, joseph.faulls, bhushan.attarde, jaydeep.patil
From: Jaydeep Patil <jaydeep.patil@imgtec.com>
Hi Mike, Andrew,
Addressed review comments.
- Simplified the checking for C extension
- Simplified the .exp file for tests. Options moved to individual .s files.
Jaydeep Patil (2):
[sim/riscv] Fix crash during instruction decoding
[sim/riscv] Add support for compressed integer instructions
opcodes/riscv-dis.c | 2 +-
sim/riscv/model_list.def | 9 +
sim/riscv/sim-main.c | 339 ++++++++++++++++++++++++++++++--
sim/testsuite/riscv/allinsn.exp | 2 +-
sim/testsuite/riscv/c-ext.s | 95 +++++++++
sim/testsuite/riscv/jalr.s | 2 +-
sim/testsuite/riscv/m-ext.s | 2 +-
sim/testsuite/riscv/pass.s | 2 +-
8 files changed, 437 insertions(+), 16 deletions(-)
create mode 100644 sim/testsuite/riscv/c-ext.s
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding
2023-12-22 5:26 [PATCH v5 0/2] sim: riscv: Compressed instruction simulation jaydeep.patil
@ 2023-12-22 5:26 ` jaydeep.patil
2024-01-10 10:22 ` Andrew Burgess
2023-12-22 5:26 ` [PATCH v5 2/2] [sim/riscv] Add support for compressed integer instructions jaydeep.patil
2024-01-04 4:24 ` [PATCH v5 0/2] sim: riscv: Compressed instruction simulation Jaydeep Patil
2 siblings, 1 reply; 7+ messages in thread
From: jaydeep.patil @ 2023-12-22 5:26 UTC (permalink / raw)
To: gdb-patches
Cc: aburgess, vapier, joseph.faulls, bhushan.attarde, jaydeep.patil
From: Jaydeep Patil <jaydeep.patil@imgtec.com>
The match_never() function has been removed and thus step_once() crashes
during instruction decoding. Fixed it by checking for null pointer before
invoking function attached to match_func member of riscv_opcode structure.
---
opcodes/riscv-dis.c | 2 +-
sim/riscv/sim-main.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 68674380797..a89ebdd32ac 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -818,7 +818,7 @@ riscv_disassemble_insn (bfd_vma memaddr,
if (op->pinfo == INSN_MACRO)
continue;
/* Does the opcode match? */
- if (! (op->match_func) (op, word))
+ if (! op->match_func || ! (op->match_func) (op, word))
continue;
/* Is this a pseudo-instruction and may we print it as such? */
if (no_aliases && (op->pinfo & INSN_ALIAS))
diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index 4d205345395..65c0ea245b2 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -1042,7 +1042,7 @@ void step_once (SIM_CPU *cpu)
for (; op->name; op++)
{
/* Does the opcode match? */
- if (! op->match_func (op, iw))
+ if (! op->match_func || ! op->match_func (op, iw))
continue;
/* Is this a pseudo-instruction and may we print it as such? */
if (op->pinfo & INSN_ALIAS)
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] [sim/riscv] Add support for compressed integer instructions
2023-12-22 5:26 [PATCH v5 0/2] sim: riscv: Compressed instruction simulation jaydeep.patil
2023-12-22 5:26 ` [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding jaydeep.patil
@ 2023-12-22 5:26 ` jaydeep.patil
2024-01-04 4:24 ` [PATCH v5 0/2] sim: riscv: Compressed instruction simulation Jaydeep Patil
2 siblings, 0 replies; 7+ messages in thread
From: jaydeep.patil @ 2023-12-22 5:26 UTC (permalink / raw)
To: gdb-patches
Cc: aburgess, vapier, joseph.faulls, bhushan.attarde, jaydeep.patil
From: Jaydeep Patil <jaydeep.patil@imgtec.com>
Added support for simulation of compressed integer instruction set ("c").
Added test file sim/testsuite/riscv/c-ext.s to test compressed instructions.
The compressed instructions are available for models implementing C extension.
Such as RV32IC, RV64IC, RV32GC, RV64GC etc.
---
sim/riscv/model_list.def | 9 +
sim/riscv/sim-main.c | 337 +++++++++++++++++++++++++++++++-
sim/testsuite/riscv/allinsn.exp | 2 +-
sim/testsuite/riscv/c-ext.s | 95 +++++++++
sim/testsuite/riscv/jalr.s | 2 +-
sim/testsuite/riscv/m-ext.s | 2 +-
sim/testsuite/riscv/pass.s | 2 +-
7 files changed, 435 insertions(+), 14 deletions(-)
create mode 100644 sim/testsuite/riscv/c-ext.s
diff --git a/sim/riscv/model_list.def b/sim/riscv/model_list.def
index 5efd85ab280..b83557e5539 100644
--- a/sim/riscv/model_list.def
+++ b/sim/riscv/model_list.def
@@ -3,7 +3,16 @@ M(I)
M(IM)
M(IMA)
M(IA)
+M(GC)
+M(IC)
+M(IMC)
+M(IMAC)
+M(IAC)
M(E)
M(EM)
M(EMA)
M(EA)
+M(EC)
+M(EMC)
+M(EMAC)
+M(EAC)
diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index 65c0ea245b2..7e3ffb0aa24 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -975,6 +975,320 @@ execute_a (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
return pc;
}
+static sim_cia
+execute_c (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
+{
+ SIM_DESC sd = CPU_STATE (cpu);
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+ int rd = (iw >> OP_SH_RD) & OP_MASK_RD;
+ int rs1_c = ((iw >> OP_SH_CRS1S) & OP_MASK_CRS1S) + 8;
+ int rs2 = (iw >> OP_SH_CRS2) & OP_MASK_CRS2;
+ int rs2_c = ((iw >> OP_SH_CRS2S) & OP_MASK_CRS2S) + 8;
+ const char *rd_name = riscv_gpr_names_abi[rd];
+ const char *rs1_c_name = riscv_gpr_names_abi[rs1_c];
+ const char *rs2_name = riscv_gpr_names_abi[rs2];
+ const char *rs2_c_name = riscv_gpr_names_abi[rs2_c];
+ signed_word imm;
+ unsigned_word tmp;
+ sim_cia pc = riscv_cpu->pc + 2;
+
+ switch (op->match)
+ {
+ case MATCH_C_JR | MATCH_C_MV:
+ switch (op->mask)
+ {
+ case MASK_C_MV:
+ TRACE_INSN (cpu, "c.mv %s, %s; // %s = %s",
+ rd_name, rs2_name, rd_name, rs2_name);
+ store_rd (cpu, rd, riscv_cpu->regs[rs2]);
+ break;
+ case MASK_C_JR:
+ TRACE_INSN (cpu, "c.jr %s;",
+ rd_name);
+ pc = riscv_cpu->regs[rd];
+ TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
+ break;
+ }
+ break;
+ case MATCH_C_J:
+ imm = EXTRACT_CJTYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.j %" PRIxTW,
+ imm);
+ pc = riscv_cpu->pc + imm;
+ TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
+ break;
+ case MATCH_C_JAL | MATCH_C_ADDIW:
+ /* JAL and ADDIW have the same mask, so switch based on op name. */
+ switch (op->name[2])
+ {
+ case 'j':
+ imm = EXTRACT_CJTYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.jal %" PRIxTW,
+ imm);
+ store_rd (cpu, SIM_RISCV_RA_REGNUM, riscv_cpu->pc + 2);
+ pc = riscv_cpu->pc + imm;
+ TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
+ break;
+ case 'a':
+ imm = EXTRACT_CITYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.addiw %s, %s, %#" PRIxTW "; // %s += %#" PRIxTW,
+ rd_name, rd_name, imm, rd_name, imm);
+ RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
+ store_rd (cpu, rd, EXTEND32 (riscv_cpu->regs[rd] + imm));
+ break;
+ default:
+ TRACE_INSN (cpu, "UNHANDLED INSN: %s", op->name);
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_signalled,
+ SIM_SIGILL);
+ }
+ break;
+ case MATCH_C_JALR | MATCH_C_ADD | MATCH_C_EBREAK:
+ switch (op->mask)
+ {
+ case MASK_C_ADD:
+ TRACE_INSN (cpu, "c.add %s, %s; // %s += %s",
+ rd_name, rs2_name, rd_name, rs2_name);
+ store_rd (cpu, rd, riscv_cpu->regs[rd] + riscv_cpu->regs[rs2]);
+ break;
+ case MASK_C_JALR:
+ TRACE_INSN (cpu, "c.jalr %s, %s;",
+ riscv_gpr_names_abi[SIM_RISCV_RA_REGNUM], rd_name);
+ store_rd (cpu, SIM_RISCV_RA_REGNUM, riscv_cpu->pc + 2);
+ pc = riscv_cpu->regs[rd];
+ TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
+ break;
+ case MASK_C_EBREAK:
+ TRACE_INSN (cpu, "ebreak");
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_stopped,
+ SIM_SIGTRAP);
+ }
+ break;
+ case MATCH_C_BEQZ:
+ imm = EXTRACT_CBTYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.beqz %s, %#" PRIxTW "; "
+ "// if (%s == 0) goto %#" PRIxTW,
+ rs1_c_name, imm, rs1_c_name, riscv_cpu->pc + imm);
+ if (riscv_cpu->regs[rs1_c] == riscv_cpu->regs[0])
+ {
+ pc = riscv_cpu->pc + imm;
+ TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
+ }
+ break;
+ case MATCH_C_BNEZ:
+ imm = EXTRACT_CBTYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.bnez %s, %#" PRIxTW "; "
+ "// if (%s != 0) goto %#" PRIxTW,
+ rs1_c_name, imm, rs1_c_name, riscv_cpu->pc + imm);
+ if (riscv_cpu->regs[rs1_c] != riscv_cpu->regs[0])
+ {
+ pc = riscv_cpu->pc + imm;
+ TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
+ }
+ break;
+ case MATCH_C_LWSP:
+ imm = EXTRACT_CITYPE_LWSP_IMM (iw);
+ TRACE_INSN (cpu, "c.lwsp %s, %" PRIiTW "(sp);",
+ rd_name, imm);
+ store_rd (cpu, rd, EXTEND32 (
+ sim_core_read_unaligned_4 (cpu, riscv_cpu->pc, read_map,
+ riscv_cpu->regs[SIM_RISCV_SP_REGNUM]
+ + imm)));
+ break;
+ case MATCH_C_LW:
+ imm = EXTRACT_CLTYPE_LW_IMM (iw);
+ TRACE_INSN (cpu, "c.lw %s, %" PRIiTW "(%s);",
+ rs2_c_name, imm, rs1_c_name);
+ store_rd (cpu, rs2_c, EXTEND32 (
+ sim_core_read_unaligned_4 (cpu, riscv_cpu->pc, read_map,
+ riscv_cpu->regs[rs1_c] + imm)));
+ break;
+ case MATCH_C_SWSP:
+ imm = EXTRACT_CSSTYPE_SWSP_IMM (iw);
+ TRACE_INSN (cpu, "c.swsp %s, %" PRIiTW "(sp);",
+ rs2_name, imm);
+ sim_core_write_unaligned_4 (cpu, riscv_cpu->pc, write_map,
+ riscv_cpu->regs[SIM_RISCV_SP_REGNUM] + imm,
+ riscv_cpu->regs[rs2]);
+ break;
+ case MATCH_C_SW:
+ imm = EXTRACT_CLTYPE_LW_IMM (iw);
+ TRACE_INSN (cpu, "c.sw %s, %" PRIiTW "(%s);",
+ rs2_c_name, imm, rs1_c_name);
+ sim_core_write_unaligned_4 (cpu, riscv_cpu->pc, write_map,
+ riscv_cpu->regs[rs1_c] + (imm),
+ riscv_cpu->regs[rs2_c]);
+ break;
+ case MATCH_C_ADDI:
+ imm = EXTRACT_CITYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.addi %s, %s, %#" PRIxTW "; // %s += %#" PRIxTW,
+ rd_name, rd_name, imm, rd_name, imm);
+ store_rd (cpu, rd, riscv_cpu->regs[rd] + imm);
+ break;
+ case MATCH_C_LUI:
+ imm = EXTRACT_CITYPE_LUI_IMM (iw);
+ TRACE_INSN (cpu, "c.lui %s, %#" PRIxTW ";",
+ rd_name, imm);
+ store_rd (cpu, rd, imm);
+ break;
+ case MATCH_C_LI:
+ imm = EXTRACT_CITYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.li %s, %#" PRIxTW "; // %s = %#" PRIxTW,
+ rd_name, imm, rd_name, imm);
+ store_rd (cpu, rd, imm);
+ break;
+ case MATCH_C_ADDI4SPN:
+ imm = EXTRACT_CIWTYPE_ADDI4SPN_IMM (iw);
+ TRACE_INSN (cpu, "c.addi4spn %s, %" PRIiTW "; // %s = sp + %" PRIiTW,
+ rs2_c_name, imm, rs2_c_name, imm);
+ store_rd (cpu, rs2_c, riscv_cpu->regs[SIM_RISCV_SP_REGNUM] + (imm));
+ break;
+ case MATCH_C_ADDI16SP:
+ imm = EXTRACT_CITYPE_ADDI16SP_IMM (iw);
+ TRACE_INSN (cpu, "c.addi16sp %s, %" PRIiTW "; // %s = sp + %" PRIiTW,
+ rd_name, imm, rd_name, imm);
+ store_rd (cpu, rd, riscv_cpu->regs[SIM_RISCV_SP_REGNUM] + imm);
+ break;
+ case MATCH_C_SUB:
+ TRACE_INSN (cpu, "c.sub %s, %s; // %s = %s - %s",
+ rs1_c_name, rs2_c_name, rs1_c_name, rs1_c_name, rs2_c_name);
+ store_rd (cpu, rs1_c, riscv_cpu->regs[rs1_c] - riscv_cpu->regs[rs2_c]);
+ break;
+ case MATCH_C_AND:
+ TRACE_INSN (cpu, "c.and %s, %s; // %s = %s & %s",
+ rs1_c_name, rs2_c_name, rs1_c_name, rs1_c_name, rs2_c_name);
+ store_rd (cpu, rs1_c, riscv_cpu->regs[rs1_c] & riscv_cpu->regs[rs2_c]);
+ break;
+ case MATCH_C_OR:
+ TRACE_INSN (cpu, "c.or %s, %s; // %s = %s | %s",
+ rs1_c_name, rs2_c_name, rs1_c_name, rs1_c_name, rs2_c_name);
+ store_rd (cpu, rs1_c, riscv_cpu->regs[rs1_c] | riscv_cpu->regs[rs2_c]);
+ break;
+ case MATCH_C_XOR:
+ TRACE_INSN (cpu, "c.xor %s, %s; // %s = %s ^ %s",
+ rs1_c_name, rs2_c_name, rs1_c_name, rs1_c_name, rs2_c_name);
+ store_rd (cpu, rs1_c, riscv_cpu->regs[rs1_c] ^ riscv_cpu->regs[rs2_c]);
+ break;
+ case MATCH_C_SLLI | MATCH_C_SLLI64:
+ if (op->mask == MASK_C_SLLI64)
+ {
+ /* Reserved for custom use. */
+ TRACE_INSN (cpu, "UNHANDLED INSN: %s", op->name);
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_signalled,
+ SIM_SIGILL);
+ break;
+ }
+ imm = EXTRACT_CITYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.slli %s, %" PRIiTW "; // %s = %s << %#" PRIxTW,
+ rd_name, imm, rd_name, rd_name, imm);
+ store_rd (cpu, rd, riscv_cpu->regs[rd] << imm);
+ break;
+ case MATCH_C_SRLI | MATCH_C_SRLI64:
+ if (op->mask == MASK_C_SRLI64)
+ {
+ /* Reserved for custom use. */
+ TRACE_INSN (cpu, "UNHANDLED INSN: %s", op->name);
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_signalled,
+ SIM_SIGILL);
+ break;
+ }
+ imm = EXTRACT_CITYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.srli %s, %" PRIiTW "; // %s = %s >> %#" PRIxTW,
+ rs1_c_name, imm, rs1_c_name, rs1_c_name, imm);
+ if (RISCV_XLEN (cpu) == 32)
+ store_rd (cpu, rs1_c,
+ EXTEND32 ((uint32_t) riscv_cpu->regs[rs1_c] >> imm));
+ else
+ store_rd (cpu, rs1_c, riscv_cpu->regs[rs1_c] >> imm);
+ break;
+ case MATCH_C_SRAI | MATCH_C_SRAI64:
+ if (op->mask == MASK_C_SRAI64)
+ {
+ /* Reserved for custom use. */
+ TRACE_INSN (cpu, "UNHANDLED INSN: %s", op->name);
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_signalled,
+ SIM_SIGILL);
+ break;
+ }
+ imm = EXTRACT_CITYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.srai %s, %" PRIiTW "; // %s = %s >> %#" PRIxTW,
+ rs1_c_name, imm, rs1_c_name, rs1_c_name, imm);
+ if (RISCV_XLEN (cpu) == 32)
+ {
+ if (imm > 0x1f)
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_signalled,
+ SIM_SIGILL);
+ tmp = ashiftrt (riscv_cpu->regs[rs1_c], imm);
+ }
+ else
+ tmp = ashiftrt64 (riscv_cpu->regs[rs1_c], imm);
+ store_rd (cpu, rd, tmp);
+ break;
+ case MATCH_C_ANDI:
+ imm = EXTRACT_CITYPE_IMM (iw);
+ TRACE_INSN (cpu, "c.andi %s, %" PRIiTW "; // %s = %s & %#" PRIxTW,
+ rs1_c_name, imm, rs1_c_name, rs1_c_name, imm);
+ store_rd (cpu, rs1_c, riscv_cpu->regs[rs1_c] & imm);
+ break;
+ case MATCH_C_ADDW:
+ TRACE_INSN (cpu, "c.addw %s, %s; // %s = %s + %s",
+ rs1_c_name, rs2_c_name, rs1_c_name, rs1_c_name, rs2_c_name);
+ RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
+ store_rd (cpu, rs1_c,
+ EXTEND32 (riscv_cpu->regs[rs1_c] + riscv_cpu->regs[rs2_c]));
+ break;
+ case MATCH_C_SUBW:
+ TRACE_INSN (cpu, "c.subw %s, %s; // %s = %s - %s",
+ rs1_c_name, rs2_c_name, rs1_c_name, rs1_c_name, rs2_c_name);
+ RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
+ store_rd (cpu, rs1_c,
+ EXTEND32 (riscv_cpu->regs[rs1_c] - riscv_cpu->regs[rs2_c]));
+ break;
+ case MATCH_C_LDSP:
+ imm = EXTRACT_CITYPE_LDSP_IMM (iw);
+ TRACE_INSN (cpu, "c.ldsp %s, %" PRIiTW "(sp);",
+ rd_name, imm);
+ RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
+ store_rd (cpu, rd,
+ sim_core_read_unaligned_8 (cpu, riscv_cpu->pc, read_map,
+ riscv_cpu->regs[SIM_RISCV_SP_REGNUM]
+ + imm));
+ break;
+ case MATCH_C_LD:
+ imm = EXTRACT_CLTYPE_LD_IMM (iw);
+ TRACE_INSN (cpu, "c.ld %s, %" PRIiTW "(%s);",
+ rs1_c_name, imm, rs2_c_name);
+ RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
+ store_rd (cpu, rs2_c,
+ sim_core_read_unaligned_8 (cpu, riscv_cpu->pc, read_map,
+ riscv_cpu->regs[rs1_c] + imm));
+ break;
+ case MATCH_C_SDSP:
+ imm = EXTRACT_CSSTYPE_SDSP_IMM (iw);
+ TRACE_INSN (cpu, "c.sdsp %s, %" PRIiTW "(sp);",
+ rs2_name, imm);
+ RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
+ sim_core_write_unaligned_8 (cpu, riscv_cpu->pc, write_map,
+ riscv_cpu->regs[SIM_RISCV_SP_REGNUM] + imm,
+ riscv_cpu->regs[rs2]);
+ break;
+ case MATCH_C_SD:
+ imm = EXTRACT_CLTYPE_LD_IMM (iw);
+ TRACE_INSN (cpu, "c.sd %s, %" PRIiTW "(%s);",
+ rs2_c_name, imm, rs1_c_name);
+ RISCV_ASSERT_RV64 (cpu, "insn: %s", op->name);
+ sim_core_write_unaligned_8 (cpu, riscv_cpu->pc, write_map,
+ riscv_cpu->regs[rs1_c] + imm,
+ riscv_cpu->regs[rs2_c]);
+ break;
+ default:
+ TRACE_INSN (cpu, "UNHANDLED INSN: %s", op->name);
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_signalled,
+ SIM_SIGILL);
+ }
+
+ return pc;
+}
+
static sim_cia
execute_one (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
{
@@ -990,6 +1304,16 @@ execute_one (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
{
case INSN_CLASS_A:
return execute_a (cpu, iw, op);
+ case INSN_CLASS_C:
+ /* Check whether model with C extension is selected. */
+ if (riscv_cpu->csr.misa & 4)
+ return execute_c (cpu, iw, op);
+ else
+ {
+ TRACE_INSN (cpu, "UNHANDLED EXTENSION: %d", op->insn_class);
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_signalled,
+ SIM_SIGILL);
+ }
case INSN_CLASS_I:
return execute_i (cpu, iw, op);
case INSN_CLASS_M:
@@ -1020,17 +1344,10 @@ void step_once (SIM_CPU *cpu)
iw = sim_core_read_aligned_2 (cpu, pc, exec_map, pc);
- /* Reject non-32-bit opcodes first. */
len = riscv_insn_length (iw);
- if (len != 4)
- {
- sim_io_printf (sd, "sim: bad insn len %#x @ %#" PRIxTA ": %#" PRIxTW "\n",
- len, pc, iw);
- sim_engine_halt (sd, cpu, NULL, pc, sim_signalled, SIM_SIGILL);
- }
-
- iw |= ((unsigned_word) sim_core_read_aligned_2 (
- cpu, pc, exec_map, pc + 2) << 16);
+ if (len == 4)
+ iw |= ((unsigned_word) sim_core_read_aligned_2
+ (cpu, pc, exec_map, pc + 2) << 16);
TRACE_CORE (cpu, "0x%08" PRIxTW, iw);
diff --git a/sim/testsuite/riscv/allinsn.exp b/sim/testsuite/riscv/allinsn.exp
index 972edf4d5ec..9d454039467 100644
--- a/sim/testsuite/riscv/allinsn.exp
+++ b/sim/testsuite/riscv/allinsn.exp
@@ -3,7 +3,7 @@
sim_init
# all machines
-set all_machs "riscv"
+set all_machs "riscv32 riscv64"
foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.s]] {
# If we're only testing specific files and this isn't one of them, skip it.
diff --git a/sim/testsuite/riscv/c-ext.s b/sim/testsuite/riscv/c-ext.s
new file mode 100644
index 00000000000..e9d6ab70111
--- /dev/null
+++ b/sim/testsuite/riscv/c-ext.s
@@ -0,0 +1,95 @@
+# Basic Tests for C extension.
+# mach: all
+# sim(riscv32): --model RV32IC
+# sim(riscv64): --model RV64IC
+# ld(riscv32): -m elf32lriscv
+# ld(riscv64): -m elf64lriscv
+# as(riscv32): -march=rv32ic
+# as(riscv64): -march=rv64ic
+
+.include "testutils.inc"
+
+ .data
+ .align 4
+_data:
+ .word 1234
+ .word 0
+
+ start
+ la a0, _data
+
+ # Test load-store instructions.
+ c.lw a1,0(a0)
+ c.sw a1,4(a0)
+ c.lw a2,4(a0)
+
+ li a5,1234
+ bne a1,a5,test_fail
+ bne a2,a5,test_fail
+
+ # Test basic arithmetic.
+ c.li a0,0
+ c.li a1,1
+ c.addi a0,1
+ c.addi a0,-1
+ c.add a0,a1
+ c.sub a0,a1
+
+ li a5,1
+ bne a0,x0,test_fail
+ bne a1,a5,test_fail
+
+ # Test logical operations.
+ c.li a0,7
+ c.li a1,7
+ c.li a2,4
+ c.li a3,3
+ c.li a4,3
+ c.andi a0,3
+ c.and a1,a0
+ c.or a2,a3
+ c.xor a4,a4
+
+ li a5,3
+ bne a0,a5,test_fail
+ bne a1,a5,test_fail
+ bne a4,x0,test_fail
+ li a5,7
+ bne a2,a5,test_fail
+
+ # Test shift operations.
+ c.li a0,4
+ c.li a1,4
+ c.slli a0,1
+ c.srli a1,1
+
+ li a5,8
+ bne a0,a5,test_fail
+ li a5,2
+ bne a1,a5,test_fail
+
+ # Test jump instruction.
+ c.j 1f
+
+ j test_fail
+1:
+ la a0,2f
+
+ # Test jump register instruction.
+ c.jr a0
+
+ j test_fail
+
+2:
+ # Test branch instruction.
+ c.li a0,1
+ c.beqz a0,test_fail
+ c.li a0,0
+ c.bnez a0,test_fail
+
+test_pass:
+ pass
+ fail
+
+test_fail:
+ fail
diff --git a/sim/testsuite/riscv/jalr.s b/sim/testsuite/riscv/jalr.s
index daccf4fb5a0..f08575b4185 100644
--- a/sim/testsuite/riscv/jalr.s
+++ b/sim/testsuite/riscv/jalr.s
@@ -1,5 +1,5 @@
# Basic jalr tests.
-# mach: riscv
+# mach: all
.include "testutils.inc"
diff --git a/sim/testsuite/riscv/m-ext.s b/sim/testsuite/riscv/m-ext.s
index b80bd140e76..ef917184a58 100644
--- a/sim/testsuite/riscv/m-ext.s
+++ b/sim/testsuite/riscv/m-ext.s
@@ -1,5 +1,5 @@
# Check that the RV32M instructions run without any faults.
-# mach: riscv
+# mach: all
.include "testutils.inc"
diff --git a/sim/testsuite/riscv/pass.s b/sim/testsuite/riscv/pass.s
index bd428ca1677..01cedf2d44f 100644
--- a/sim/testsuite/riscv/pass.s
+++ b/sim/testsuite/riscv/pass.s
@@ -1,5 +1,5 @@
# check that the sim doesn't die immediately.
-# mach: riscv
+# mach: all
.include "testutils.inc"
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v5 0/2] sim: riscv: Compressed instruction simulation
2023-12-22 5:26 [PATCH v5 0/2] sim: riscv: Compressed instruction simulation jaydeep.patil
2023-12-22 5:26 ` [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding jaydeep.patil
2023-12-22 5:26 ` [PATCH v5 2/2] [sim/riscv] Add support for compressed integer instructions jaydeep.patil
@ 2024-01-04 4:24 ` Jaydeep Patil
2024-01-05 5:41 ` Mike Frysinger
2 siblings, 1 reply; 7+ messages in thread
From: Jaydeep Patil @ 2024-01-04 4:24 UTC (permalink / raw)
To: gdb-patches; +Cc: aburgess, vapier, Joseph Faulls, Bhushan Attarde
Hi Mike, Andrew,
Could you please review this patch?
Regards,
Jaydeep
-----Original Message-----
From: Jaydeep Patil <Jaydeep.Patil@imgtec.com>
Sent: Friday, December 22, 2023 10:57 AM
To: gdb-patches@sourceware.org
Cc: aburgess@redhat.com; vapier@gentoo.org; Joseph Faulls <Joseph.Faulls@imgtec.com>; Bhushan Attarde <Bhushan.Attarde@imgtec.com>; Jaydeep Patil <Jaydeep.Patil@imgtec.com>
Subject: [PATCH v5 0/2] sim: riscv: Compressed instruction simulation
*** NOTE: This is an internal email from Imagination Technologies ***
From: Jaydeep Patil <jaydeep.patil@imgtec.com>
Hi Mike, Andrew,
Addressed review comments.
- Simplified the checking for C extension
- Simplified the .exp file for tests. Options moved to individual .s files.
Jaydeep Patil (2):
[sim/riscv] Fix crash during instruction decoding
[sim/riscv] Add support for compressed integer instructions
opcodes/riscv-dis.c | 2 +-
sim/riscv/model_list.def | 9 +
sim/riscv/sim-main.c | 339 ++++++++++++++++++++++++++++++--
sim/testsuite/riscv/allinsn.exp | 2 +-
sim/testsuite/riscv/c-ext.s | 95 +++++++++
sim/testsuite/riscv/jalr.s | 2 +-
sim/testsuite/riscv/m-ext.s | 2 +-
sim/testsuite/riscv/pass.s | 2 +-
8 files changed, 437 insertions(+), 16 deletions(-) create mode 100644 sim/testsuite/riscv/c-ext.s
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 0/2] sim: riscv: Compressed instruction simulation
2024-01-04 4:24 ` [PATCH v5 0/2] sim: riscv: Compressed instruction simulation Jaydeep Patil
@ 2024-01-05 5:41 ` Mike Frysinger
0 siblings, 0 replies; 7+ messages in thread
From: Mike Frysinger @ 2024-01-05 5:41 UTC (permalink / raw)
To: Jaydeep Patil; +Cc: gdb-patches, aburgess, Joseph Faulls, Bhushan Attarde
[-- Attachment #1: Type: text/plain, Size: 461 bytes --]
On 04 Jan 2024 04:24, Jaydeep Patil wrote:
> Could you please review this patch?
i was mostly being lazy and waiting on the first one to merge. i assume
Andrew needs to sign off on that as i don't usually touch opcodes/.
also, can you stick to normal "tag:" commit summary style, not "[tag]" ?
git treats "[...]" text specially and likes to discard it in some cases
which makes merging your patches difficult. so use "sim: riscv:", not
"[sim/riscv]".
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding
2023-12-22 5:26 ` [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding jaydeep.patil
@ 2024-01-10 10:22 ` Andrew Burgess
2024-01-10 11:34 ` [EXTERNAL] " Jaydeep Patil
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2024-01-10 10:22 UTC (permalink / raw)
To: jaydeep.patil, gdb-patches
Cc: vapier, joseph.faulls, bhushan.attarde, jaydeep.patil
<jaydeep.patil@imgtec.com> writes:
> From: Jaydeep Patil <jaydeep.patil@imgtec.com>
>
> The match_never() function has been removed and thus step_once() crashes
> during instruction decoding. Fixed it by checking for null pointer before
> invoking function attached to match_func member of riscv_opcode structure.
> ---
> opcodes/riscv-dis.c | 2 +-
> sim/riscv/sim-main.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 68674380797..a89ebdd32ac 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -818,7 +818,7 @@ riscv_disassemble_insn (bfd_vma memaddr,
> if (op->pinfo == INSN_MACRO)
> continue;
> /* Does the opcode match? */
> - if (! (op->match_func) (op, word))
> + if (! op->match_func || ! (op->match_func) (op, word))
> continue;
> /* Is this a pseudo-instruction and may we print it as such? */
> if (no_aliases && (op->pinfo & INSN_ALIAS))
Sorry to be a pain, but changes in opcodes/ need to go through the
binutils@sourceware.org mailing list.
I took a little dive into the history of the match_never() removal, and
found these two commits:
commit 2ec31e54dff83130fbde8d2f674469078ee203d5
Date: Fri Nov 24 10:15:59 2023 +0100
RISC-V: drop leftover match_never() references
And maybe more interesting:
commit 27b33966b18ed8bf1701a60999448224b1d28273
Date: Fri Nov 24 09:53:15 2023 +0100
RISC-V: disallow x0 with certain macro-insns
This second commit talks about treating a NULL as actually meaning
match_always. I've not dug into this beyond looking at those commits,
but that second commit does include some code similar to yours, except
in that case they've gone with something like:
if (op->match_func && !(op->match_func) (op, word))
continue;
I'd like to see a commit message that references this history, and
explains why this commit does something different.
Also, does your change indicate that there exists an instruction
encoding which, if we try to disassemble it, will cause the disassembler
to segfault? That would be a good candidate for making into a test,
maybe in the gas testsuite?
Thanks,
Andrew
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 4d205345395..65c0ea245b2 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -1042,7 +1042,7 @@ void step_once (SIM_CPU *cpu)
> for (; op->name; op++)
> {
> /* Does the opcode match? */
> - if (! op->match_func (op, iw))
> + if (! op->match_func || ! op->match_func (op, iw))
> continue;
> /* Is this a pseudo-instruction and may we print it as such? */
> if (op->pinfo & INSN_ALIAS)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding
2024-01-10 10:22 ` Andrew Burgess
@ 2024-01-10 11:34 ` Jaydeep Patil
0 siblings, 0 replies; 7+ messages in thread
From: Jaydeep Patil @ 2024-01-10 11:34 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches; +Cc: vapier, Joseph Faulls, Bhushan Attarde
<aburgess@redhat.com> writes:
>This second commit talks about treating a NULL as actually meaning match_always. I've not dug into this beyond looking at those commits, but that second commit does include some code similar to yours, except in that case they've gone with something like:
>
> if (op->match_func && !(op->match_func) (op, word))
> continue;
>
>I'd like to see a commit message that references this history, and explains why this commit does something different.
>
>Also, does your change indicate that there exists an instruction encoding which, if we try to disassemble it, will cause the disassembler to segfault? That would be a good candidate for making into a test, maybe in the gas testsuite?
>
Looking at the riscv_opcodes[] table in opcodes/riscv-opc.c, we don't need to modify opcodes/riscv-dis.c.
The match_never() has been replaced with NULL for INSN_MACRO types. Thus disassembler will never segfault because of following code:
@@ -818,7 +818,7 @@ riscv_disassemble_insn (bfd_vma memaddr,
if (op->pinfo == INSN_MACRO)
continue;
The segfault happens only in step_once() of sim/riscv/sim-main.c.
Regards,
Jaydeep
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-10 11:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-22 5:26 [PATCH v5 0/2] sim: riscv: Compressed instruction simulation jaydeep.patil
2023-12-22 5:26 ` [PATCH v5 1/2] [sim/riscv] Fix crash during instruction decoding jaydeep.patil
2024-01-10 10:22 ` Andrew Burgess
2024-01-10 11:34 ` [EXTERNAL] " Jaydeep Patil
2023-12-22 5:26 ` [PATCH v5 2/2] [sim/riscv] Add support for compressed integer instructions jaydeep.patil
2024-01-04 4:24 ` [PATCH v5 0/2] sim: riscv: Compressed instruction simulation Jaydeep Patil
2024-01-05 5:41 ` Mike Frysinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).