[PATCH v6 0/2] sim: riscv: Compressed instruction simulation

[PATCH v6 0/2] sim: riscv: Compressed instruction simulation

[jaydeep.patil]

From: Jaydeep Patil <jaydeep.patil@imgtec.com>

Hi Mike, Andrew,

Addressed review comments.
 - Restored opcodes/riscv-dis.c
 - Commit message changed to "sim: riscv:"

Jaydeep Patil (2):
  sim: riscv: Fix crash during instruction decoding
  sim: riscv: Add support for compressed integer instructions

 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 +-
 7 files changed, 436 insertions(+), 15 deletions(-)
 create mode 100644 sim/testsuite/riscv/c-ext.s

-- 
2.25.1

[PATCH v6 1/2] sim: riscv: Fix crash during instruction decoding

[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
---
 sim/riscv/sim-main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index b04d12c0de1..25dc9b41412 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -1040,7 +1040,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

Re: [PATCH v6 1/2] sim: riscv: Fix crash during instruction decoding

[Andrew Burgess]

<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
> ---
>  sim/riscv/sim-main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thank you for continuing to work on these patches.

For patch 1/2:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index b04d12c0de1..25dc9b41412 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -1040,7 +1040,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

Re: [PATCH v6 1/2] sim: riscv: Fix crash during instruction decoding

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

On 11 Jan 2024 05:28, jaydeep.patil@imgtec.com wrote:
> 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

pushed now, thx
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v6 2/2] sim: riscv: Add support for compressed integer instructions

[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 25dc9b41412..865c2c0e7fd 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -973,6 +973,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)
 {
@@ -988,6 +1302,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:
@@ -1018,17 +1342,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

Re: [PATCH v6 2/2] sim: riscv: Add support for compressed integer instructions

[Andrew Burgess]

<jaydeep.patil@imgtec.com> writes:

> 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.

Thanks for working on these patches.  This mostly looks fine, but I do
have a couple of questions, see below.

> ---
>  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 25dc9b41412..865c2c0e7fd 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -973,6 +973,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)
>  {
> @@ -988,6 +1302,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:
> @@ -1018,17 +1342,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);
> -    }

riscv_insn_length() can return lengths other than 2 or 4.  I think we
should retain this error block but change the condition to:

  if (len != 4 && len != 2)

Anything else isn't going to work, so we shouldn't even try beyond this point.

> -
> -  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

I notice 'mach: all' here, and you've changed all the other tests to use
'all' too.  This doesn't seem right.  What's going on here?

Thanks,
Andrew

> +# 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

RE: [EXTERNAL] Re: [PATCH v6 2/2] sim: riscv: Add support for compressed integer instructions

[Jaydeep Patil]

<aburgess@redhat.com> writes:

> riscv_insn_length() can return lengths other than 2 or 4.  I think we should retain this error block but change the condition to:
> 
>   if (len != 4 && len != 2)
> 
> Anything else isn't going to work, so we shouldn't even try beyond this point.

Ok, I will retain the error block.

> I notice 'mach: all' here, and you've changed all the other tests to use 'all' too.  This doesn't seem right.  What's going on here?

I think this should be 'mach: riscv32 riscv64'.

Regards,
Jaydeep