Add Zcb and Zcmp extensions

doc/01_overview/compliance.rst

@@ -47,6 +47,14 @@ In addition, the following instruction set extensions are available.
      - 2.0
      - always enabled
 
+   * - **Zcb**: Simple Code-Size Saving Instructions
+     - 1.0.0
+     - optional
+
+   * - **Zcmp**: Push/Pop/Move Code-Size Saving Instructions
+     - 1.0.0
+     - optional
+
    * - **Smepmp** - PMP Enhancements for memory access and execution prevention on Machine mode
      - 1.0
      - always enabled in configurations with PMP see :ref:`PMP Enhancements<pmp-enhancements>`

doc/03_reference/pipeline_details.rst

@@ -93,3 +93,13 @@ Read the description for more information.
 |                       |                                      | jump (which does the required flushing) so it has the same  |
 |                       |                                      | stall characteristics (see above).                          |
 +-----------------------+--------------------------------------+-------------------------------------------------------------+
+| Zcmp Push/Pop         | 2 - N                                | The cm.push/pop instructions as defined in 'Zcmp' of the    |
+|                       |                                      | RISC-V specification. Internally they expand to multiple    |
+|                       |                                      | register load/store operations combined with stack pointer  |
+|                       |                                      | adjustments, so their latency corresponds to the total      |
+|                       |                                      | number of instructions issued and the memory latency.       |
++-----------------------+--------------------------------------+-------------------------------------------------------------+
+| Zcmp Move             | 2                                    | The `cm.mvsa01` and `cm.mva01s` instruction as defined in   |
+|                       |                                      | 'Zcmp' of the RISC-V specification. Internally they are     |
+|                       |                                      | implemented as two `addi rd, rs1, 0` instructions.          |
++-----------------------+--------------------------------------+-------------------------------------------------------------+

dv/cosim/cosim.h

@@ -74,9 +74,12 @@ class Cosim {
   // In this case the instruction doesn't retire so no register write occurs (so
   // `write_reg` must be 0).
   //
+  // `expanded_insn` is the 32-bit instruction that is being expanded or zero.
+  //
   // Returns false if there are any errors; use `get_errors` to obtain details
   virtual bool step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
-                    bool sync_trap, bool suppress_reg_write) = 0;
+                    bool sync_trap, bool suppress_reg_write,
+                    uint32_t expanded_insn) = 0;
 
   // When more than one of `set_mip`, `set_nmi` or `set_debug_req` is called
   // before `step` which one takes effect is chosen by the co-simulator. Which

dv/cosim/cosim_dpi.cc

@@ -12,11 +12,12 @@
 
 int riscv_cosim_step(Cosim *cosim, const svBitVecVal *write_reg,
                      const svBitVecVal *write_reg_data, const svBitVecVal *pc,
-                     svBit sync_trap, svBit suppress_reg_write) {
+                     svBit sync_trap, svBit suppress_reg_write,
+                     const svBitVecVal *expanded_insn) {
   assert(cosim);
 
   return cosim->step(write_reg[0], write_reg_data[0], pc[0], sync_trap,
-                     suppress_reg_write)
+                     suppress_reg_write, expanded_insn[0])
              ? 1
              : 0;
 }

dv/cosim/cosim_dpi.h

@@ -16,7 +16,8 @@
 extern "C" {
 int riscv_cosim_step(Cosim *cosim, const svBitVecVal *write_reg,
                      const svBitVecVal *write_reg_data, const svBitVecVal *pc,
-                     svBit sync_trap, svBit suppress_reg_write);
+                     svBit sync_trap, svBit suppress_reg_write,
+                     const svBitVecVal *expanded_insn);
 void riscv_cosim_set_mip(Cosim *cosim, const svBitVecVal *pre_mip,
                          const svBitVecVal *post_mip);
 void riscv_cosim_set_nmi(Cosim *cosim, svBit nmi);

dv/cosim/cosim_dpi.svh

@@ -11,7 +11,8 @@
 `define COSIM_DPI_SVH
 
 import "DPI-C" function int riscv_cosim_step(chandle cosim_handle, bit [4:0] write_reg,
-  bit [31:0] write_reg_data, bit [31:0] pc, bit sync_trap, bit suppress_reg_write);
+  bit [31:0] write_reg_data, bit [31:0] pc, bit sync_trap, bit suppress_reg_write,
+  bit [31:0] expanded_insn);
 import "DPI-C" function void riscv_cosim_set_mip(chandle cosim_handle, bit [31:0] pre_mip,
   bit [31:0] post_mip);
 import "DPI-C" function void riscv_cosim_set_nmi(chandle cosim_handle, bit nmi);

dv/cosim/spike_cosim.cc

@@ -39,7 +39,8 @@ SpikeCosim::SpikeCosim(const std::string &isa_string, uint32_t start_pc,
                        uint32_t pmp_num_regions, uint32_t pmp_granularity,
                        uint32_t mhpm_counter_num, uint32_t dm_start_addr,
                        uint32_t dm_end_addr)
-    : nmi_mode(false), pending_iside_error(false), insn_cnt(0) {
+    : nmi_mode(false), pending_iside_error(false), insn_cnt(0),
+    pending_expanded_insn(0) {
   FILE *log_file = nullptr;
   if (trace_log_path.length() != 0) {
     log = std::make_unique<log_file_t>(trace_log_path.c_str());
@@ -170,7 +171,8 @@ bool SpikeCosim::backdoor_read_mem(uint32_t addr, size_t len,
 //   processor, and when we call step() again we start executing in the new
 //   context of the trap (trap handler, new MSTATUS, debug rom, etc. etc.)
 bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
-                      bool sync_trap, bool suppress_reg_write) {
+                      bool sync_trap, bool suppress_reg_write,
+                      uint32_t expanded_insn) {
   assert(write_reg < 32);
 
   // The DUT has just produced an RVFI item
@@ -214,7 +216,12 @@ bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
   // (If the current step causes a synchronous trap, it will be
   //  recorded against the current pc)
   initial_spike_pc = (processor->get_state()->pc & 0xffffffff);
-  processor->step(1);
+  // Only step Spike if the current instruction is not an expanded one. Spike
+  // does not expand instructions, so we have to consume a single Spike step
+  // over multiple steps of Ibex.
+  if (pending_expanded_insn == 0) {
+    processor->step(1);
+  }
 
   // ISS
   // - If encountered an async trap,
@@ -309,18 +316,102 @@ bool SpikeCosim::step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
                                       suppressed_write_reg_data);
   }
 
-  if (!check_retired_instr(write_reg, write_reg_data, pc, suppress_reg_write)) {
-    return false;
+  if (expanded_insn) {
+    if (!check_expanded_instr(write_reg, write_reg_data, pc, suppress_reg_write,
+                              expanded_insn)) {
+      return false;
+    }
+  } else {
+    if (!check_retired_instr(write_reg, write_reg_data, pc, suppress_reg_write,
+                             expanded_insn)) {
+      return false;
+    }
   }
 
   // Only increment insn_cnt and return true if there are no errors
   insn_cnt++;
   return true;
 }
 
+bool SpikeCosim::check_expanded_instr(uint32_t write_reg,
+                                     uint32_t write_reg_data, uint32_t dut_pc,
+                                     bool suppress_reg_write,
+                                     uint32_t expanded_insn) {
+  // If this is the first step of an expanded instruction, set up our expectations.
+  if (!pending_expanded_insn) {
+    pending_expanded_insn = expanded_insn;
+    expanded_insn_pc = dut_pc;
+    expanded_reg_changes.clear();
+
+    // The ISS has just stepped once for the entire expanded instruction.
+    // We now collect all the GPR changes it produced and store them in a map
+    auto &reg_changes = processor->get_state()->log_reg_write;
+    for (auto reg_change : reg_changes) {
+      // Check if it's a GPR write (type 0)
+      if ((reg_change.first & 0xf) == 0) {
+        uint32_t iss_write_reg = (reg_change.first >> 4) & 0x1f;
+        // Ignore writes to x0
+        if (iss_write_reg == 0) {
+          continue;
+        }
+        uint32_t iss_write_data = reg_change.second.v[0];
+        expanded_reg_changes[iss_write_reg] = iss_write_data;
+      }
+    }
+  }
+
+  // TODO: We have to deal with suppressed writes here too
+  if (suppress_reg_write) {
+    return true;
+  }
+
+  // If the DUT did not write a register, this is a valid micro-op (e.g., a
+  // memory access or intermediate calculation). There is nothing to check.
+  if (write_reg == 0) {
+    return true;
+  }
+
+  // The DUT wrote a register. Check if it matches one of our expectations.
+  auto reg_change = expanded_reg_changes.find(write_reg);
+
+  // DUT wrote, but ISS didn't
+  if (reg_change == expanded_reg_changes.end()) {
+    std::stringstream err_str;
+    err_str << "Expanded instruction at PC 0x" << std::hex << expanded_insn_pc
+            << ": DUT wrote to x" << std::dec << write_reg
+            << ", which was not an expected register write for this sequence.";
+    errors.emplace_back(err_str.str());
+    return false;
+  }
+
+  // The register index matches. Now check if the data matches.
+  uint32_t expected_data = reg_change->second;
+  if (write_reg_data != expected_data) {
+    std::stringstream err_str;
+    err_str << "Expanded instruction at PC 0x" << std::hex << expanded_insn_pc
+            << ": Data mismatch for register x" << std::dec << write_reg
+            << ". DUT wrote: 0x" << std::hex << write_reg_data
+            << ", but ISS expected: 0x" << expected_data;
+    errors.emplace_back(err_str.str());
+    return false;
+  }
+
+  // Match is perfect. Remove this from our set of expectations.
+  expanded_reg_changes.erase(reg_change);
+
+  if(expanded_reg_changes.empty()) {
+    // All expected register writes have been matched. Clear the pending state.
+    pending_expanded_insn = 0;
+    expanded_insn_pc = 0;
+  }
+
+  return true;
+}
+
 bool SpikeCosim::check_retired_instr(uint32_t write_reg,
                                      uint32_t write_reg_data, uint32_t dut_pc,
-                                     bool suppress_reg_write) {
+                                     bool suppress_reg_write,
+                                     uint32_t expanded_insn) {
   // Check the retired instruction and all of its side-effects match those from
   // the DUT
 
@@ -333,6 +424,11 @@ bool SpikeCosim::check_retired_instr(uint32_t write_reg,
             << " , but the ISS retired: " << std::hex
             << (processor->get_state()->last_inst_pc & 0xffffffff);
     errors.emplace_back(err_str.str());
+    if (pending_expanded_insn) {
+      err_str << " (while processing expanded instruction at PC 0x"
+              << std::hex << expanded_insn_pc << ")";
+      errors.emplace_back(err_str.str());
+    }
     return false;
   }
 
@@ -355,7 +451,7 @@ bool SpikeCosim::check_retired_instr(uint32_t write_reg,
       assert(!gpr_write_seen);
 
       if (!suppress_reg_write &&
-          !check_gpr_write(reg_change, write_reg, write_reg_data)) {
+          !check_gpr_write(reg_change, write_reg, write_reg_data, expanded_insn)) {
         return false;
       }
 
@@ -433,7 +529,8 @@ bool SpikeCosim::check_sync_trap(uint32_t write_reg, uint32_t dut_pc,
 }
 
 bool SpikeCosim::check_gpr_write(const commit_log_reg_t::value_type &reg_change,
-                                 uint32_t write_reg, uint32_t write_reg_data) {
+                                 uint32_t write_reg, uint32_t write_reg_data,
+                                 uint32_t expanded_insn) {
   uint32_t cosim_write_reg = (reg_change.first >> 4) & 0x1f;
 
   if (write_reg == 0) {

dv/cosim/spike_cosim.h

@@ -77,7 +77,8 @@ class SpikeCosim : public simif_t, public Cosim {
   bool check_debug_ebreak(uint32_t write_reg, uint32_t pc, bool sync_trap);
 
   bool check_gpr_write(const commit_log_reg_t::value_type &reg_change,
-                       uint32_t write_reg, uint32_t write_reg_data);
+                       uint32_t write_reg, uint32_t write_reg_data,
+                       uint32_t expanded_insn);
 
   bool check_suppress_reg_write(uint32_t write_reg, uint32_t pc,
                                 uint32_t &suppressed_write_reg);
@@ -99,6 +100,11 @@ class SpikeCosim : public simif_t, public Cosim {
 
   unsigned int insn_cnt;
 
+  // Handle expanded insn
+  uint32_t pending_expanded_insn;
+  uint32_t expanded_insn_pc;
+  std::map<uint32_t, uint32_t> expanded_reg_changes;
+
  public:
   SpikeCosim(const std::string &isa_string, uint32_t start_pc,
              uint32_t start_mtvec, const std::string &trace_log_path,
@@ -120,10 +126,15 @@ class SpikeCosim : public simif_t, public Cosim {
                           const uint8_t *data_in) override;
   bool backdoor_read_mem(uint32_t addr, size_t len, uint8_t *data_out) override;
   bool step(uint32_t write_reg, uint32_t write_reg_data, uint32_t pc,
-            bool sync_trap, bool suppress_reg_write) override;
+            bool sync_trap, bool suppress_reg_write,
+            uint32_t expanded_insn) override;
 
   bool check_retired_instr(uint32_t write_reg, uint32_t write_reg_data,
-                           uint32_t dut_pc, bool suppress_reg_write);
+                           uint32_t dut_pc, bool suppress_reg_write,
+                           uint32_t expanded_insn);
+  bool check_expanded_instr(uint32_t write_reg, uint32_t write_reg_data,
+                            uint32_t dut_pc, bool suppress_reg_write,
+                            uint32_t expanded_insn);
   bool check_sync_trap(uint32_t write_reg, uint32_t pc,
                        uint32_t initial_spike_pc);
   void set_mip(uint32_t pre_mip, uint32_t post_mip) override;

dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv

@@ -163,7 +163,7 @@
       riscv_cosim_set_ic_scr_key_valid(cosim_handle, rvfi_instr.ic_scr_key_valid);
 
       if (!riscv_cosim_step(cosim_handle, rvfi_instr.rd_addr, rvfi_instr.rd_wdata, rvfi_instr.pc,
-                            rvfi_instr.trap, rvfi_instr.rf_wr_suppress)) begin
+                            rvfi_instr.trap, rvfi_instr.rf_wr_suppress, rvfi_instr.expanded_insn)) begin
         // cosim instruction step doesn't match rvfi captured instruction, report a fatal error
         // with the details
         if (cfg.relax_cosim_check) begin

dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_monitor.sv

@@ -45,6 +45,7 @@ class ibex_rvfi_monitor extends uvm_monitor;
       trans_collected.rf_wr_suppress   = vif.monitor_cb.ext_rf_wr_suppress;
       trans_collected.mcycle           = vif.monitor_cb.ext_mcycle;
       trans_collected.ic_scr_key_valid = vif.monitor_cb.ext_ic_scr_key_valid;
+      trans_collected.expanded_insn    = vif.monitor_cb.ext_expanded_insn;
 
       for (int i=0; i < 10; i++) begin
        trans_collected.mhpmcounters[i]  = vif.monitor_cb.ext_mhpmcounters[i];

dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_rvfi_seq_item.sv

@@ -21,6 +21,8 @@ class ibex_rvfi_seq_item extends uvm_sequence_item;
   bit [31:0] mhpmcountersh [10];
   bit        ic_scr_key_valid;
 
+  bit [31:0] expanded_insn;
+
   `uvm_object_utils_begin(ibex_rvfi_seq_item)
     `uvm_field_int (trap, UVM_DEFAULT)
     `uvm_field_int (pc, UVM_DEFAULT)
@@ -37,6 +39,7 @@ class ibex_rvfi_seq_item extends uvm_sequence_item;
     `uvm_field_sarray_int (mhpmcounters, UVM_DEFAULT)
     `uvm_field_sarray_int (mhpmcountersh, UVM_DEFAULT)
     `uvm_field_int (ic_scr_key_valid, UVM_DEFAULT)
+    `uvm_field_int (expanded_insn, UVM_DEFAULT)
   `uvm_object_utils_end
 
   `uvm_object_new

dv/uvm/core_ibex/env/core_ibex_rvfi_if.sv

@@ -40,6 +40,8 @@ interface core_ibex_rvfi_if(input logic clk);
 
   logic        ext_ic_scr_key_valid;
 
+  logic [31:0] ext_expanded_insn;
+
   clocking monitor_cb @(posedge clk);
     input reset;
     input valid;
@@ -74,6 +76,7 @@ interface core_ibex_rvfi_if(input logic clk);
     input ext_mhpmcountersh;
     input ext_ic_scr_key_valid;
     input ext_irq_valid;
+    input ext_expanded_insn;
   endclocking
 
   task automatic wait_clks(input int num);

dv/uvm/core_ibex/riscv_dv_extension/riscv_core_setting.tpl.sv

@@ -53,7 +53,7 @@ bit support_unaligned_load_store = 1'b1;
 
 // ISA supported by the processor
 // TODO: Determine how Ibex RV32B types map to RISCV-DV ISA names
-riscv_instr_group_t supported_isa[$] = {RV32I, RV32M, RV32C
+riscv_instr_group_t supported_isa[$] = {RV32I, RV32M, RV32C, RV32ZCB, RV32ZCMP
 % if ibex_config['RV32B'] == 'ibex_pkg::RV32BNone':
     };
 % else:

dv/uvm/core_ibex/riscv_dv_extension/testlist.yaml

@@ -1157,3 +1157,13 @@
   rtl_test: core_ibex_base_test
   rtl_params:
     RV32B: ["ibex_pkg::RV32BFull", "ibex_pkg::RV32BOTEarlGrey", "ibex_pkg::RV32BBalanced"]
+
+- test: riscv_zcb_balanced_test
+  desc: >
+    Random instruction test with zcb instructions in balanced configuration
+  iterations: 10
+  gen_test: riscv_rand_instr_test
+  gen_opts: >
+    +enable_zcb_extension=1
+  rtl_test: core_ibex_base_test
+

dv/uvm/core_ibex/scripts/ibex_cmd.py

@@ -114,8 +114,9 @@ def get_isas_for_config(cfg: Config) -> Tuple[str, str]:
 
     has_bitmanip = cfg.rv32b != 'ibex_pkg::RV32BNone'
     toolchain_isa = base_isa + ('b' if has_bitmanip else '')
+    toolchain_isa = toolchain_isa + ('_zicsr_zifencei_zcb_zcmp')
 
-    return (toolchain_isa, '_'.join([base_isa] + bitmanip_isa))
+    return (toolchain_isa, '_'.join([base_isa] + ['Zicsr','Zifencei','Zcb','Zcmp'] + bitmanip_isa))
 
 
 _TestEntry = Dict[str, object]

dv/uvm/core_ibex/tb/core_ibex_tb_top.sv

@@ -239,6 +239,7 @@ module core_ibex_tb_top;
   assign rvfi_if.ext_mhpmcountersh    = dut.rvfi_ext_mhpmcountersh;
   assign rvfi_if.ext_ic_scr_key_valid = dut.rvfi_ext_ic_scr_key_valid;
   assign rvfi_if.ext_irq_valid        = dut.rvfi_ext_irq_valid;
+  assign rvfi_if.ext_expanded_insn    = dut.rvfi_ext_expanded_insn;
   // Irq interface connections
   assign irq_vif.reset = ~rst_n;
   // Dut_if interface connections

dv/uvm/core_ibex/tests/core_ibex_base_test.sv

@@ -70,6 +70,10 @@ class core_ibex_base_test extends uvm_test;
     isa = {"rv32", RV32E ? "e" : "i"};
     if (RV32M != RV32MNone) isa = {isa, "m"};
     isa = {isa, "c"};
+    isa = {isa, "_Zicsr"};
+    isa = {isa, "_Zifencei"};
+    isa = {isa, "_Zcb"};
+    isa = {isa, "_Zcmp"};
     case (RV32B)
       RV32BNone:
         ;