From 6628a9c41277334d6ceb02db9c2bc187f5a21c10 Mon Sep 17 00:00:00 2001 From: Richard Qin Date: Mon, 16 Mar 2026 14:20:58 +0800 Subject: Naming convention --- rv64cpu/docs/naming_convention.md | 159 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 rv64cpu/docs/naming_convention.md (limited to 'rv64cpu/docs/naming_convention.md') diff --git a/rv64cpu/docs/naming_convention.md b/rv64cpu/docs/naming_convention.md new file mode 100644 index 0000000..c99d8a4 --- /dev/null +++ b/rv64cpu/docs/naming_convention.md @@ -0,0 +1,159 @@ +# RTL Signal Naming Convention + +This document defines the naming rules for all SystemVerilog signals, ports, types, +and modules in the Richard-CPU project. Follow these rules when writing new RTL or +modifying existing code. + +## 1. General Rules + +| Rule | Example | +|---|---| +| All names use `snake_case` (lowercase + underscores) | `alu_result`, `rs1_addr` | +| Names should be descriptive; avoid single-letter names except loop indices | `i` in `for(int i = ...)` is acceptable | +| Abbreviations are allowed **only** from the approved list (Section 7) | `addr` not `address`, `imm` not `immediate` | +| No trailing numbers for disambiguation; use descriptive suffixes instead | `read_port_a` not `read_port_1` | + +## 2. Module & Package Names + +| Item | Rule | Example | +|---|---|---| +| Module | `snake_case`, named after function | `alu`, `regfile`, `decoder`, `if_stage` | +| Package | `snake_case` ending in `_pkg` | `rv64_pkg` | +| Typedef | `snake_case` ending in `_t` | `alu_op_t`, `opcode_t` | +| Enum members | `UPPER_SNAKE_CASE` with a common prefix | `ALU_ADD`, `OPCODE_LUI` | +| Parameters / localparams | `UPPER_SNAKE_CASE` | `XLEN`, `INST_WIDTH` | + +## 3. Port & Signal Names + +### 3.1 Standard Suffixes + +Use these suffixes consistently to indicate the **role** of a signal: + +| Suffix | Meaning | Example | +|---|---|---| +| `_addr` | Address (register index or memory address) | `rs1_addr`, `rd_addr` | +| `_data` | Data bus (generic read/write payload) | `mem_rdata`, `mem_wdata` | +| `_rdata` | Read data (output from a storage element) | `rs1_rdata`, `rs2_rdata` | +| `_wdata` | Write data (input to a storage element) | `rd_wdata` | +| `_op` | Operation selector (typically an enum or funct3) | `alu_op`, `branch_op`, `csr_op` | +| `_sel` | Mux select signal | `alu_src1_sel`, `wb_sel` | +| `_en` | Enable (active-high gating signal) | `reg_write_en`, `mem_read_en` | +| `_val` | **Deprecated — do not use for new code.** Use `_rdata`/`_wdata`/`_data` instead | — | + +### 3.2 Standard Prefixes + +| Prefix | Meaning | Example | +|---|---|---| +| `is_` | Boolean flag indicating instruction type or state | `is_load`, `is_branch`, `is_csr` | +| `has_` | Boolean flag indicating a condition is met | `has_exception`, `has_interrupt` | + +### 3.3 Rules for Boolean / Control Signals + +All single-bit control signals should follow **one** of these patterns: + +| Pattern | When to Use | Example | +|---|---|---| +| `is_` | Classifying an instruction or state | `is_load`, `is_jump`, `is_ecall` | +| `_en` | Enabling a write or action | `reg_write_en`, `mem_write_en` | +| `_` | Describing a property | `illegal_instr`, `branch_taken` | + +> **Consistency rule**: For write-enable signals on storage elements (register file, +> memory, CSR), always use the `_en` suffix. Example: `reg_write_en`, not `reg_write`. + +### 3.4 Clock & Reset + +| Signal | Name | Description | +|---|---|---| +| Clock | `clk` | Single system clock | +| Reset | `rst_n` (preferred) or `rst` | Active-low (`rst_n`) is preferred; if active-high, use `rst` | + +> Current codebase uses active-high `rst`. If you later switch to active-low, rename +> to `rst_n` everywhere in a single commit. + +## 4. Pipeline Stage Prefixes + +When signals cross pipeline stages, prefix them with the stage abbreviation: + +| Stage | Prefix | Example | +|---|---|---| +| Instruction Fetch | `if_` | `if_pc`, `if_instr` | +| Instruction Decode | `id_` | `id_rs1_addr`, `id_imm` | +| Execute | `ex_` | `ex_alu_result`, `ex_branch_taken` | +| Memory Access | `mem_` | `mem_rdata`, `mem_addr` | +| Write Back | `wb_` | `wb_rd_wdata`, `wb_rd_addr` | + +Inter-stage pipeline registers use a combined prefix: `id_ex_reg_write_en`, +`ex_mem_alu_result`. + +## 5. ALU Port Naming + +The ALU is a **generic compute unit**. Its inputs are not always register values — +they may come from the PC or an immediate after mux selection. Use neutral names: + +| Port | Current (non-conforming) | Recommended | +|---|---|---| +| Operand 1 input | `rs1_val` | `operand_a` | +| Operand 2 input | `rs2_val` | `operand_b` | +| Result output | `alu_res` | `alu_result` | + +## 6. Register File Port Naming + +| Port | Name | Description | +|---|---|---| +| Read address 1 | `rs1_addr` | Source register 1 index | +| Read data 1 | `rs1_rdata` | Source register 1 data output | +| Read address 2 | `rs2_addr` | Source register 2 index | +| Read data 2 | `rs2_rdata` | Source register 2 data output | +| Write address | `rd_addr` | Destination register index | +| Write data | `rd_wdata` | Destination register data input | +| Write enable | `reg_write_en` | Write enable (active high) | + +## 7. Approved Abbreviations + +Only these abbreviations are permitted. All other words should be spelled in full. + +| Abbreviation | Full Form | +|---|---| +| `addr` | address | +| `alu` | arithmetic logic unit | +| `clk` | clock | +| `csr` | control and status register | +| `en` | enable | +| `ex` | execute (pipeline stage only) | +| `fpu` | floating-point unit | +| `id` | instruction decode (pipeline stage only) | +| `if` | instruction fetch (pipeline stage only) | +| `imm` | immediate | +| `instr` | instruction | +| `mem` | memory | +| `mmu` | memory management unit | +| `mux` | multiplexer | +| `op` | operation | +| `pc` | program counter | +| `rd` | destination register | +| `rdata` | read data | +| `reg` | register | +| `res` | **Deprecated — use `result`** | +| `rs1` / `rs2` | source register 1 / 2 | +| `rst` | reset | +| `sel` | select | +| `tlb` | translation lookaside buffer | +| `val` | **Deprecated — use `rdata`/`wdata`/`data`** | +| `wb` | write back (pipeline stage only) | +| `wdata` | write data | + +## 8. Current Codebase Non-Conformances + +The following table lists signals in the existing codebase that do not yet follow +this convention. These should be renamed during the next refactoring pass. + +| File | Current Name | Recommended Name | Reason | +|---|---|---|---| +| `alu.sv` | `rs1_val` | `operand_a` | ALU input is muxed, not always rs1 | +| `alu.sv` | `rs2_val` | `operand_b` | ALU input is muxed, not always rs2 | +| `alu.sv` | `alu_res` | `alu_result` | Avoid non-approved abbreviation `res` | +| `regfile.sv` | `rs1_val` | `rs1_rdata` | Use `_rdata` for read data ports | +| `regfile.sv` | `rs2_val` | `rs2_rdata` | Use `_rdata` for read data ports | +| `regfile.sv` | `rd_val` | `rd_wdata` | Use `_wdata` for write data ports | +| `regfile.sv` | `reg_write` | `reg_write_en` | Use `_en` suffix for enable signals | +| `decoder.sv` | `reg_write` | `reg_write_en` | Use `_en` suffix for enable signals | -- cgit v1.2.3 From 1554746fd79d51c6bd2281820520a4e2d2c16301 Mon Sep 17 00:00:00 2001 From: Richard Qin Date: Mon, 16 Mar 2026 14:40:52 +0800 Subject: fix naming problems --- rv64cpu/docs/naming_convention.md | 9 +-------- rv64cpu/rtl/core/alu.sv | 32 ++++++++++++++++---------------- rv64cpu/rtl/core/decoder.sv | 30 +++++++++++++++--------------- rv64cpu/rtl/core/regfile.sv | 16 ++++++++-------- rv64cpu/tb/unit/tb_alu.cc | 8 ++++---- rv64cpu/tb/unit/tb_decoder.cc | 36 ++++++++++++++++++------------------ rv64cpu/tb/unit/tb_regfile.cc | 30 +++++++++++++++--------------- 7 files changed, 77 insertions(+), 84 deletions(-) (limited to 'rv64cpu/docs/naming_convention.md') diff --git a/rv64cpu/docs/naming_convention.md b/rv64cpu/docs/naming_convention.md index c99d8a4..fea4844 100644 --- a/rv64cpu/docs/naming_convention.md +++ b/rv64cpu/docs/naming_convention.md @@ -149,11 +149,4 @@ this convention. These should be renamed during the next refactoring pass. | File | Current Name | Recommended Name | Reason | |---|---|---|---| -| `alu.sv` | `rs1_val` | `operand_a` | ALU input is muxed, not always rs1 | -| `alu.sv` | `rs2_val` | `operand_b` | ALU input is muxed, not always rs2 | -| `alu.sv` | `alu_res` | `alu_result` | Avoid non-approved abbreviation `res` | -| `regfile.sv` | `rs1_val` | `rs1_rdata` | Use `_rdata` for read data ports | -| `regfile.sv` | `rs2_val` | `rs2_rdata` | Use `_rdata` for read data ports | -| `regfile.sv` | `rd_val` | `rd_wdata` | Use `_wdata` for write data ports | -| `regfile.sv` | `reg_write` | `reg_write_en` | Use `_en` suffix for enable signals | -| `decoder.sv` | `reg_write` | `reg_write_en` | Use `_en` suffix for enable signals | +|None|None|None|None| diff --git a/rv64cpu/rtl/core/alu.sv b/rv64cpu/rtl/core/alu.sv index ac2e9c9..abfc238 100644 --- a/rv64cpu/rtl/core/alu.sv +++ b/rv64cpu/rtl/core/alu.sv @@ -21,44 +21,44 @@ module alu import rv64_pkg::*; ( input alu_op_t alu_op, // ALU operation select (from decoder) - input logic [XLEN-1:0] rs1_val, // Operand 1 (register rs1 value or PC) - input logic [XLEN-1:0] rs2_val, // Operand 2 (register rs2 value or immediate) + input logic [XLEN-1:0] operand_a, // Operand 1 (register rs1 value or PC) + input logic [XLEN-1:0] operand_b, // Operand 2 (register rs2 value or immediate) - output logic [XLEN-1:0] alu_res // ALU result + output logic [XLEN-1:0] alu_result // ALU result ); // Shift amount: RV64I uses lower 6 bits of rs2 (shamt[5:0]) logic [5:0] shamt; - assign shamt = rs2_val[5:0]; + assign shamt = operand_b[5:0]; always_comb begin - alu_res = '0; + alu_result = '0; case (alu_op) - ALU_ADD: alu_res = rs1_val + rs2_val; - ALU_SUB: alu_res = rs1_val - rs2_val; + ALU_ADD: alu_result = operand_a + operand_b; + ALU_SUB: alu_result = operand_a - operand_b; // Logical left shift: the shift amount is the lower 6 bits of rs2 - ALU_SLL: alu_res = rs1_val << shamt; + ALU_SLL: alu_result = operand_a << shamt; // Set less than (signed) - ALU_SLT: alu_res = {63'b0, $signed(rs1_val) < $signed(rs2_val)}; + ALU_SLT: alu_result = {63'b0, $signed(operand_a) < $signed(operand_b)}; // Set less than (unsigned) - ALU_SLTU: alu_res = {63'b0, rs1_val < rs2_val}; + ALU_SLTU: alu_result = {63'b0, operand_a < operand_b}; - ALU_XOR: alu_res = rs1_val ^ rs2_val; + ALU_XOR: alu_result = operand_a ^ operand_b; // Logical right shift: the shift amount is the lower 6 bits of rs2 - ALU_SRL: alu_res = rs1_val >> shamt; + ALU_SRL: alu_result = operand_a >> shamt; // Arithmetic right shift: the shift amount is the lower 6 bits of rs2 - ALU_SRA: alu_res = $unsigned($signed(rs1_val) >>> shamt); + ALU_SRA: alu_result = $unsigned($signed(operand_a) >>> shamt); - ALU_OR: alu_res = rs1_val | rs2_val; - ALU_AND: alu_res = rs1_val & rs2_val; + ALU_OR: alu_result = operand_a | operand_b; + ALU_AND: alu_result = operand_a & operand_b; - default: alu_res = '0; + default: alu_result = '0; endcase end diff --git a/rv64cpu/rtl/core/decoder.sv b/rv64cpu/rtl/core/decoder.sv index 8f35c47..585a018 100644 --- a/rv64cpu/rtl/core/decoder.sv +++ b/rv64cpu/rtl/core/decoder.sv @@ -28,7 +28,7 @@ module decoder output logic [4:0] rs1_addr, // Source register 1 address output logic [4:0] rs2_addr, // Source register 2 address output logic [4:0] rd_addr, // Destination register address - output logic reg_write, // Register file write enable + output logic reg_write_en, // Register file write enable // ALU Control output alu_op_t alu_op, // ALU operation select @@ -112,7 +112,7 @@ module decoder alu_op = ALU_ADD; alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b0; // rs2 - reg_write = 1'b0; + reg_write_en = 1'b0; imm = '0; is_load = 1'b0; is_store = 1'b0; @@ -139,7 +139,7 @@ module decoder // LUI — rd = imm_u OPCODE_LUI: begin - reg_write = 1'b1; + reg_write_en = 1'b1; alu_src1_sel = 1'b0; // doesn't matter, ALU_ADD(0, imm_u) alu_src2_sel = 1'b1; // immediate imm = imm_u; @@ -153,7 +153,7 @@ module decoder OPCODE_AUIPC: begin - reg_write = 1'b1; + reg_write_en = 1'b1; alu_src1_sel = 1'b1; // PC alu_src2_sel = 1'b1; // immediate imm = imm_u; @@ -163,7 +163,7 @@ module decoder // ----------------- // JAL — rd = PC+4; PC = PC + imm_j OPCODE_JAL: begin - reg_write = 1'b1; + reg_write_en = 1'b1; is_jump = 1'b1; alu_src1_sel = 1'b1; // PC alu_src2_sel = 1'b1; // immediate @@ -176,7 +176,7 @@ module decoder // JALR — rd = PC+4; PC = (rs1 + imm_i) & ~1 OPCODE_JALR: begin - reg_write = 1'b1; + reg_write_en = 1'b1; is_jump = 1'b1; alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b1; // immediate @@ -188,7 +188,7 @@ module decoder // ----------------- // Branch — compare rs1, rs2; if taken, PC = PC + imm_b OPCODE_BRANCH: begin - reg_write = 1'b0; // branches never write rd + reg_write_en = 1'b0; // branches never write rd is_branch = 1'b1; alu_src1_sel = 1'b1; // PC (ALU computes branch target) alu_src2_sel = 1'b1; // immediate @@ -200,7 +200,7 @@ module decoder // ----------------- // Load — rd = Mem[rs1 + imm_i] OPCODE_LOAD: begin - reg_write = 1'b1; + reg_write_en = 1'b1; is_load = 1'b1; alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b1; // immediate @@ -213,7 +213,7 @@ module decoder // ----------------- // Store — Mem[rs1 + imm_s] = rs2 OPCODE_STORE: begin - reg_write = 1'b0; + reg_write_en = 1'b0; is_store = 1'b1; alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b1; // immediate (NOT rs2 — rs2 is store data) @@ -226,7 +226,7 @@ module decoder // OP-IMM (I-type ALU) — rd = rs1 OP imm // ADDI, SLTI, SLTIU, XORI, ORI, ANDI, SLLI, SRLI, SRAI OPCODE_OP_IMM: begin - reg_write = 1'b1; + reg_write_en = 1'b1; alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b1; // immediate imm = imm_i; @@ -248,7 +248,7 @@ module decoder // OP-IMM-32 (I-type ALU, 32-bit word variants for RV64) // ADDIW, SLLIW, SRLIW, SRAIW OPCODE_OP_IMM_32: begin - reg_write = 1'b1; + reg_write_en = 1'b1; is_word_op = 1'b1; // EX stage: operate on lower 32 bits, sext result alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b1; // immediate @@ -269,7 +269,7 @@ module decoder // OP (R-type ALU) — rd = rs1 OP rs2 // ADD, SUB, SLL, SLT, SLTU, XOR, SRL, SRA, OR, AND OPCODE_OP: begin - reg_write = 1'b1; + reg_write_en = 1'b1; alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b0; // rs2 @@ -296,7 +296,7 @@ module decoder // OP-32 (R-type ALU, 32-bit word variants for RV64) // ADDW, SUBW, SLLW, SRLW, SRAW OPCODE_OP_32: begin - reg_write = 1'b1; + reg_write_en = 1'b1; is_word_op = 1'b1; alu_src1_sel = 1'b0; // rs1 alu_src2_sel = 1'b0; // rs2 @@ -333,7 +333,7 @@ module decoder OPCODE_SYSTEM: begin if (funct3 == 3'b000) begin // Privileged instructions (non-CSR) - reg_write = 1'b0; + reg_write_en = 1'b0; case ({funct7, instr[24:20]}) // {funct7, rs2} 12'b0000000_00000: is_ecall = 1'b1; // ECALL 12'b0000000_00001: is_ebreak = 1'b1; // EBREAK @@ -351,7 +351,7 @@ module decoder // CSR instructions // funct3: 001=CSRRW, 010=CSRRS, 011=CSRRC, // 101=CSRRWI, 110=CSRRSI, 111=CSRRCI - reg_write = 1'b1; + reg_write_en = 1'b1; is_csr = 1'b1; csr_op = funct3; imm = imm_i; // CSR address = instr[31:20] (reuse imm_i path) diff --git a/rv64cpu/rtl/core/regfile.sv b/rv64cpu/rtl/core/regfile.sv index 83e09b7..640890d 100644 --- a/rv64cpu/rtl/core/regfile.sv +++ b/rv64cpu/rtl/core/regfile.sv @@ -25,24 +25,24 @@ module regfile // Read port 1 input logic [4:0] rs1_addr, // Read address 1 - output logic [XLEN-1:0] rs1_val, // Read data 1 + output logic [XLEN-1:0] rs1_rdata, // Read data 1 // Read port 2 input logic [4:0] rs2_addr, // Read address 2 - output logic [XLEN-1:0] rs2_val, // Read data 2 + output logic [XLEN-1:0] rs2_rdata, // Read data 2 // Write port input logic [4:0] rd_addr, // Write address - input logic [XLEN-1:0] rd_val, // Write data - input logic we // Write enable signal + input logic [XLEN-1:0] rd_wdata, // Write data + input logic reg_write_en // Write enable signal ); // Register file storage logic [XLEN-1:0] regs [0:31]; // Read Logic (combinational logic) - assign rs1_val = (rs1_addr == 5'b0) ? '0 : regs[rs1_addr]; - assign rs2_val = (rs2_addr == 5'b0) ? '0 : regs[rs2_addr]; + assign rs1_rdata = (rs1_addr == 5'b0) ? '0 : regs[rs1_addr]; + assign rs2_rdata = (rs2_addr == 5'b0) ? '0 : regs[rs2_addr]; // Write Logic (sequential logic) always_ff @(posedge clk) begin @@ -51,8 +51,8 @@ module regfile regs[i] <= '0; end end else begin - if (we && rd_addr != 5'b0) begin - regs[rd_addr] <= rd_val; + if (reg_write_en && rd_addr != 5'b0) begin + regs[rd_addr] <= rd_wdata; end end end diff --git a/rv64cpu/tb/unit/tb_alu.cc b/rv64cpu/tb/unit/tb_alu.cc index a9f10b1..0d37b30 100644 --- a/rv64cpu/tb/unit/tb_alu.cc +++ b/rv64cpu/tb/unit/tb_alu.cc @@ -36,12 +36,12 @@ enum AluOp { void test_alu(Valu* alu, uint8_t op, uint64_t rs1, uint64_t rs2, uint64_t expected, const char* test_name) { alu->alu_op = op; - alu->rs1_val = rs1; - alu->rs2_val = rs2; + alu->operand_a = rs1; + alu->operand_b = rs2; alu->eval(); - if (alu->alu_res == expected) { + if (alu->alu_result == expected) { std::cout << "\033[32m"; std::cout << "[PASS] " << test_name << " | " << rs1 << " op " << rs2 << " = " << expected << std::endl; @@ -49,7 +49,7 @@ void test_alu(Valu* alu, uint8_t op, uint64_t rs1, uint64_t rs2, uint64_t expect } else { std::cout << "\033[31m"; std::cerr << "[FAIL] " << test_name << " | " - << "Expected: " << expected << ", Got: " << alu->alu_res << std::endl; + << "Expected: " << expected << ", Got: " << alu->alu_result << std::endl; std::cout << "\033[0m"; assert(false); } diff --git a/rv64cpu/tb/unit/tb_decoder.cc b/rv64cpu/tb/unit/tb_decoder.cc index b1784d5..e55bbb6 100644 --- a/rv64cpu/tb/unit/tb_decoder.cc +++ b/rv64cpu/tb/unit/tb_decoder.cc @@ -62,7 +62,7 @@ int main(int argc, char** argv) { CHECK(dut, rs1_addr, 2, "ADD"); CHECK(dut, rs2_addr, 3, "ADD"); CHECK(dut, rd_addr, 1, "ADD"); - CHECK(dut, reg_write, 1, "ADD"); // R-type writes rd + CHECK(dut, reg_write_en, 1, "ADD"); // R-type writes rd CHECK(dut, alu_op, 0, "ADD"); // ALU_ADD = 0b0000 CHECK(dut, alu_src1_sel, 0, "ADD"); // operand1 = rs1 CHECK(dut, alu_src2_sel, 0, "ADD"); // operand2 = rs2 @@ -74,7 +74,7 @@ int main(int argc, char** argv) { // Tests: funct7[5] distinguishes ADD vs SUB decode(dut, 0x402081B3, "SUB x3, x1, x2"); CHECK(dut, alu_op, 8, "SUB"); // ALU_SUB = 0b1000 - CHECK(dut, reg_write, 1, "SUB"); + CHECK(dut, reg_write_en, 1, "SUB"); CHECK(dut, alu_src2_sel, 0, "SUB"); // operand2 = rs2 // 3. R-type: SRA x1, x2, x3 (vs SRL) @@ -92,7 +92,7 @@ int main(int argc, char** argv) { // imm=000000101010 rs1=00000 funct3=000 rd=00001 op=0010011 // Tests: I-type sets alu_src2_sel=1 (immediate), reg_write=1 decode(dut, 0x02A00093, "ADDI x1, x0, 42"); - CHECK(dut, reg_write, 1, "ADDI"); + CHECK(dut, reg_write_en, 1, "ADDI"); CHECK(dut, alu_op, 0, "ADDI"); // ALU_ADD CHECK(dut, alu_src1_sel, 0, "ADDI"); // rs1 CHECK(dut, alu_src2_sel, 1, "ADDI"); // immediate @@ -118,7 +118,7 @@ int main(int argc, char** argv) { // Tests: is_load, mem_size, wb_sel=MEM, alu computes address decode(dut, 0x00853283, "LD x5, 8(x10)"); CHECK(dut, is_load, 1, "LD"); - CHECK(dut, reg_write, 1, "LD"); + CHECK(dut, reg_write_en, 1, "LD"); CHECK(dut, alu_src2_sel, 1, "LD"); // imm for address calc CHECK(dut, imm, 8, "LD"); CHECK(dut, mem_size, 3, "LD"); // funct3=011 → doubleword @@ -130,7 +130,7 @@ int main(int argc, char** argv) { // Tests: is_store, reg_write=0, S-type immediate extraction decode(dut, 0x00553823, "SD x5, 16(x10)"); CHECK(dut, is_store, 1, "SD"); - CHECK(dut, reg_write, 0, "SD"); // stores don't write rd + CHECK(dut, reg_write_en, 0, "SD"); // stores don't write rd CHECK(dut, alu_src2_sel, 1, "SD"); // imm for address calc (NOT rs2) CHECK(dut, imm, 16, "SD"); // S-type immediate = 16 CHECK(dut, mem_size, 3, "SD"); // funct3=011 → doubleword @@ -141,7 +141,7 @@ int main(int argc, char** argv) { // Tests: is_branch, branch_op=funct3, negative B-type immediate decode(dut, 0xFE208EE3, "BEQ x1, x2, -4"); CHECK(dut, is_branch, 1, "BEQ"); - CHECK(dut, reg_write, 0, "BEQ"); + CHECK(dut, reg_write_en, 0, "BEQ"); CHECK(dut, alu_src1_sel, 1, "BEQ"); // PC for target calc CHECK(dut, branch_op, 0, "BEQ"); // funct3=000 → BEQ CHECK(dut, imm, (uint64_t)(-4), "BEQ"); // sign-extended -4 @@ -159,7 +159,7 @@ int main(int argc, char** argv) { // imm[31:12]=0x12345 rd=00001 op=0110111 // Tests: U-type immediate, reg_write, alu_src2_sel=imm decode(dut, 0x123450B7, "LUI x1, 0x12345"); - CHECK(dut, reg_write, 1, "LUI"); + CHECK(dut, reg_write_en, 1, "LUI"); CHECK(dut, alu_src2_sel, 1, "LUI"); // immediate CHECK(dut, imm, 0x12345000ULL, "LUI"); // U-type: imm << 12 CHECK(dut, wb_sel, 0, "LUI"); // ALU result @@ -169,7 +169,7 @@ int main(int argc, char** argv) { // imm[31:12]=0x00001 rd=00010 op=0010111 // Tests: alu_src1_sel=PC (distinguishes from LUI) decode(dut, 0x00001117, "AUIPC x2, 0x1"); - CHECK(dut, reg_write, 1, "AUIPC"); + CHECK(dut, reg_write_en, 1, "AUIPC"); CHECK(dut, alu_src1_sel, 1, "AUIPC"); // PC ← key difference from LUI CHECK(dut, alu_src2_sel, 1, "AUIPC"); // immediate CHECK(dut, imm, 0x1000ULL, "AUIPC"); // 0x00001 << 12 = 0x1000 @@ -181,7 +181,7 @@ int main(int argc, char** argv) { // Tests: is_jump, wb_sel=PC+4, alu_src1=PC decode(dut, 0x008000EF, "JAL x1, +8"); CHECK(dut, is_jump, 1, "JAL"); - CHECK(dut, reg_write, 1, "JAL"); + CHECK(dut, reg_write_en, 1, "JAL"); CHECK(dut, alu_src1_sel, 1, "JAL"); // PC CHECK(dut, alu_src2_sel, 1, "JAL"); // immediate CHECK(dut, wb_sel, 2, "JAL"); // writeback PC+4 @@ -192,7 +192,7 @@ int main(int argc, char** argv) { // Tests: is_jump, alu_src1=rs1 (distinguishes from JAL) decode(dut, 0x004280E7, "JALR x1, x5, 4"); CHECK(dut, is_jump, 1, "JALR"); - CHECK(dut, reg_write, 1, "JALR"); + CHECK(dut, reg_write_en, 1, "JALR"); CHECK(dut, alu_src1_sel, 0, "JALR"); // rs1 ← key difference from JAL CHECK(dut, alu_src2_sel, 1, "JALR"); // immediate CHECK(dut, wb_sel, 2, "JALR"); // writeback PC+4 @@ -203,7 +203,7 @@ int main(int argc, char** argv) { // imm=000000001010 rs1=00010 funct3=000 rd=00001 op=0011011 // Tests: is_word_op flag for 32-bit operation decode(dut, 0x00A1009B, "ADDIW x1, x2, 10"); - CHECK(dut, reg_write, 1, "ADDIW"); + CHECK(dut, reg_write_en, 1, "ADDIW"); CHECK(dut, is_word_op, 1, "ADDIW"); // 32-bit + sext CHECK(dut, alu_op, 0, "ADDIW"); // ALU_ADD CHECK(dut, alu_src2_sel, 1, "ADDIW"); // immediate @@ -213,7 +213,7 @@ int main(int argc, char** argv) { // funct7=0000000 rs2=00010 rs1=00001 funct3=000 rd=00011 op=0111011 // Tests: OP_32 path, is_word_op + R-type (alu_src2=rs2) decode(dut, 0x002081BB, "ADDW x3, x1, x2"); - CHECK(dut, reg_write, 1, "ADDW"); + CHECK(dut, reg_write_en, 1, "ADDW"); CHECK(dut, is_word_op, 1, "ADDW"); CHECK(dut, alu_op, 0, "ADDW"); // ALU_ADD CHECK(dut, alu_src2_sel, 0, "ADDW"); // rs2 (R-type!) @@ -230,35 +230,35 @@ int main(int argc, char** argv) { decode(dut, 0x300110F3, "CSRRW x1, mstatus, x2"); CHECK(dut, is_csr, 1, "CSRRW"); CHECK(dut, csr_op, 1, "CSRRW"); // funct3=001 → CSRRW - CHECK(dut, reg_write, 1, "CSRRW"); + CHECK(dut, reg_write_en, 1, "CSRRW"); CHECK(dut, illegal_instr,0, "CSRRW"); // 20. ECALL // all-zero fields + opcode=1110011 decode(dut, 0x00000073, "ECALL"); CHECK(dut, is_ecall, 1, "ECALL"); - CHECK(dut, reg_write, 0, "ECALL"); + CHECK(dut, reg_write_en, 0, "ECALL"); CHECK(dut, illegal_instr,0, "ECALL"); // 21. EBREAK // funct7=0000000 rs2=00001 rs1=00000 funct3=000 rd=00000 op=1110011 decode(dut, 0x00100073, "EBREAK"); CHECK(dut, is_ebreak, 1, "EBREAK"); - CHECK(dut, reg_write, 0, "EBREAK"); + CHECK(dut, reg_write_en, 0, "EBREAK"); // 22. MRET // funct7=0011000 rs2=00010 rs1=00000 funct3=000 rd=00000 op=1110011 // Tests: privileged instruction decode from {funct7, rs2} decode(dut, 0x30200073, "MRET"); CHECK(dut, is_mret, 1, "MRET"); - CHECK(dut, reg_write, 0, "MRET"); + CHECK(dut, reg_write_en, 0, "MRET"); CHECK(dut, illegal_instr,0, "MRET"); // 23. SRET // funct7=0001000 rs2=00010 rs1=00000 funct3=000 rd=00000 op=1110011 decode(dut, 0x10200073, "SRET"); CHECK(dut, is_sret, 1, "SRET"); - CHECK(dut, reg_write, 0, "SRET"); + CHECK(dut, reg_write_en, 0, "SRET"); // 24. WFI // funct7=0001000 rs2=00101 rs1=00000 funct3=000 rd=00000 op=1110011 @@ -282,7 +282,7 @@ int main(int argc, char** argv) { // Tests: default case sets illegal_instr, reg_write stays 0 decode(dut, 0x00000000, "Illegal (all-zero)"); CHECK(dut, illegal_instr,1, "Illegal"); - CHECK(dut, reg_write, 0, "Illegal"); + CHECK(dut, reg_write_en, 0, "Illegal"); // Results std::cout << "═══════════════════════════════════════════════" << std::endl; diff --git a/rv64cpu/tb/unit/tb_regfile.cc b/rv64cpu/tb/unit/tb_regfile.cc index c96e395..eef92a2 100644 --- a/rv64cpu/tb/unit/tb_regfile.cc +++ b/rv64cpu/tb/unit/tb_regfile.cc @@ -36,9 +36,9 @@ int main(int argc, char** argv) { dut->clk = 0; dut->rst = 0; - dut->we = 0; + dut->reg_write_en = 0; dut->rd_addr = 0; - dut->rd_val = 0; + dut->rd_wdata = 0; dut->rs1_addr = 0; dut->rs2_addr = 0; @@ -51,45 +51,45 @@ int main(int argc, char** argv) { std::cout << "[PASS] Reset complete" << std::endl; // Test 2: Write and read - dut->we = 1; + dut->reg_write_en = 1; dut->rd_addr = 5; - dut->rd_val = 0xDEADBEEF; + dut->rd_wdata = 0xDEADBEEF; tick(dut); - dut->we = 0; + dut->reg_write_en = 0; dut->rs1_addr = 5; dut->eval(); - assert(dut->rs1_val == 0xDEADBEEF); + assert(dut->rs1_rdata == 0xDEADBEEF); std::cout << "[PASS] Write and read (x5) complete" << std::endl; // Test 3: x0 - dut->we = 1; + dut->reg_write_en = 1; dut->rd_addr = 0; - dut->rd_val = 0x12345678; + dut->rd_wdata = 0x12345678; tick(dut); - dut->we = 0; + dut->reg_write_en = 0; dut->rs2_addr = 0; dut->eval(); - assert(dut->rs2_val == 0); + assert(dut->rs2_rdata == 0); std::cout << "[PASS] Write and read (x0) complete" << std::endl; // Test 4: Dual-port simultaneous read - dut->we = 1; + dut->reg_write_en = 1; dut->rd_addr = 6; - dut->rd_val = 0x87654321; + dut->rd_wdata = 0x87654321; tick(dut); - dut->we = 1; + dut->reg_write_en = 1; dut->rd_addr = 7; - dut->rd_val = 0x11223344; + dut->rd_wdata = 0x11223344; tick(dut); dut->rs1_addr = 6; dut->rs2_addr = 7; dut->eval(); - assert(dut->rs1_val == 0x87654321 && dut->rs2_val == 0x11223344); + assert(dut->rs1_rdata == 0x87654321 && dut->rs2_rdata == 0x11223344); std::cout << "[PASS] Dual-port simultaneous read complete" << std::endl; std::cout << "Register file unit test complete" << std::endl; -- cgit v1.2.3 From 639212a826659c591db3202607c4a47e1049b92a Mon Sep 17 00:00:00 2001 From: Richard Qin Date: Tue, 7 Apr 2026 22:42:49 +0800 Subject: refactor: standardize interrupt signal naming across the codebase --- rv64cpu/README.md | 2 +- rv64cpu/docs/naming_convention.md | 45 ++++++++++++++++++++++++++ rv64cpu/rtl/core/csr_unit.sv | 52 +++++++++++++++--------------- rv64cpu/rtl/core/include/csr_pkg.sv | 18 +++++------ rv64cpu/rtl/core/richard_core.sv | 38 +++++++++++----------- rv64cpu/rtl/core/trap_ctrl.sv | 24 +++++++------- rv64cpu/tb/compliance/compliance_runner.cc | 6 ++-- rv64cpu/tb/core/tb_core.cc | 24 +++++++------- rv64cpu/tb/unit/tb_trap_ctrl.cc | 16 ++++----- 9 files changed, 135 insertions(+), 90 deletions(-) (limited to 'rv64cpu/docs/naming_convention.md') diff --git a/rv64cpu/README.md b/rv64cpu/README.md index 72b3546..a7e62a5 100644 --- a/rv64cpu/README.md +++ b/rv64cpu/README.md @@ -130,7 +130,7 @@ richard-cpu/ |---|---| | P1 | RV64I five-stage pipeline + hazard handling (Done) | | P2 | CSR + M/S/U privilege levels + Trap (Done) | -| P3 | MMU (Sv39 TLB + PTW) | +| P3 | MMU (TLB + PTW) | | P4 | Cache + AXI4-Full Burst | | P5 | Peripherals + AXI4-Lite bridge + SoC integration | | P6 | Pass riscv-arch-test (In Progress: strict suites above all pass) | diff --git a/rv64cpu/docs/naming_convention.md b/rv64cpu/docs/naming_convention.md index fea4844..830c5e5 100644 --- a/rv64cpu/docs/naming_convention.md +++ b/rv64cpu/docs/naming_convention.md @@ -13,6 +13,20 @@ modifying existing code. | Abbreviations are allowed **only** from the approved list (Section 7) | `addr` not `address`, `imm` not `immediate` | | No trailing numbers for disambiguation; use descriptive suffixes instead | `read_port_a` not `read_port_1` | +### 1.1 Semantic Clarity Priority + +When multiple names are technically valid, prefer the name with the clearest +semantic meaning: + +1. Prefer full words over unclear abbreviations unless listed in Section 7. +2. Prefer domain-accurate nouns over generic placeholders. +3. Prefer names that communicate role and timing (source, destination, stage). + +Examples: +- `interrupt_cause` is preferred over `int_cause` +- `trap_target_pc` is preferred over `trap_next_value` +- `csr_write_data` is preferred over `csr_write_val` + ## 2. Module & Package Names | Item | Rule | Example | @@ -40,6 +54,14 @@ Use these suffixes consistently to indicate the **role** of a signal: | `_en` | Enable (active-high gating signal) | `reg_write_en`, `mem_read_en` | | `_val` | **Deprecated — do not use for new code.** Use `_rdata`/`_wdata`/`_data` instead | — | +Additional clarity rules: +- `*_value` is allowed for architectural/state snapshots when `*_data` would be + ambiguous. Example: `csr_mstatus_value`. +- For storage read/write payloads, always prefer `_rdata`/`_wdata`/`_data` over + `_value`. +- `_flag` is deprecated for new boolean signals. Use `is_`, `has_`, + or `_`. + ### 3.2 Standard Prefixes | Prefix | Meaning | Example | @@ -60,6 +82,11 @@ All single-bit control signals should follow **one** of these patterns: > **Consistency rule**: For write-enable signals on storage elements (register file, > memory, CSR), always use the `_en` suffix. Example: `reg_write_en`, not `reg_write`. +Additional boolean rules: +- Avoid suffix-only boolean naming such as `*_flag` and `*_bool` in new code. +- `has_` is for condition presence (`has_interrupt`), `is_` is for classification + (`is_interrupt` for a classified event or type). + ### 3.4 Clock & Reset | Signal | Name | Description | @@ -142,6 +169,24 @@ Only these abbreviations are permitted. All other words should be spelled in ful | `wb` | write back (pipeline stage only) | | `wdata` | write data | +Not approved for new code: +- `int` (use `interrupt`) +- `flag` suffix for booleans (use Section 3.3 patterns) + +## 7.1 Architecture-Defined Bit Names (Exception) + +Some names come directly from ISA-defined bit semantics and are allowed in narrow +scopes even if they are single-letter: + +- PTE-like permission bits: `u`, `r`, `w`, `x`, `a`, `d`, `g` + +Scope limits: +1. Allowed only inside tightly scoped structs, packed fields, or helper function + parameters tied to ISA semantics. +2. For module ports and cross-module buses, prefer descriptive names such as + `pte_user`, `pte_read`, `pte_write`, `pte_exec`, `pte_accessed`, `pte_dirty`, + `pte_global`. + ## 8. Current Codebase Non-Conformances The following table lists signals in the existing codebase that do not yet follow diff --git a/rv64cpu/rtl/core/csr_unit.sv b/rv64cpu/rtl/core/csr_unit.sv index 0427eeb..3aefe10 100644 --- a/rv64cpu/rtl/core/csr_unit.sv +++ b/rv64cpu/rtl/core/csr_unit.sv @@ -201,10 +201,10 @@ module csr_unit if (csr_write_en && csr_is_writable(csr_addr_canonical)) begin unique case (csr_addr_canonical) CSR_MSTATUS: begin - logic [XLEN-1:0] next_val; - next_val = csr_write_next(mstatus_reg, csr_rs1_data, csr_op); - mstatus_reg <= next_val; - sstatus_reg <= mirror_mstatus_to_sstatus(sstatus_reg, next_val); + logic [XLEN-1:0] next_data; + next_data = csr_write_next(mstatus_reg, csr_rs1_data, csr_op); + mstatus_reg <= next_data; + sstatus_reg <= mirror_mstatus_to_sstatus(sstatus_reg, next_data); end CSR_MEDELEG: medeleg_reg <= csr_write_next(medeleg_reg, csr_rs1_data, csr_op); CSR_MIDELEG: mideleg_reg <= csr_write_next(mideleg_reg, csr_rs1_data, csr_op); @@ -321,18 +321,18 @@ module csr_unit // Apply the CSR op semantics (write/set/clear) for a single register. function automatic logic [XLEN-1:0] csr_write_next( - input logic [XLEN-1:0] current_val, - input logic [XLEN-1:0] write_val, + input logic [XLEN-1:0] current_data, + input logic [XLEN-1:0] write_data, input logic [2:0] op_code ); - logic [XLEN-1:0] next_val; + logic [XLEN-1:0] next_data; unique case (op_code) - 3'b001, 3'b101: next_val = write_val; // CSRRW / CSRRWI - 3'b010, 3'b110: next_val = current_val | write_val; // CSRRS / CSRRSI - 3'b011, 3'b111: next_val = current_val & ~write_val; // CSRRC / CSRRCI - default: next_val = current_val; + 3'b001, 3'b101: next_data = write_data; // CSRRW / CSRRWI + 3'b010, 3'b110: next_data = current_data | write_data; // CSRRS / CSRRSI + 3'b011, 3'b111: next_data = current_data & ~write_data; // CSRRC / CSRRCI + default: next_data = current_data; endcase - return next_val; + return next_data; endfunction function automatic bit csr_is_writable(input logic [11:0] addr); @@ -393,26 +393,26 @@ module csr_unit function automatic logic [XLEN-1:0] apply_sstatus_to_mstatus( input logic [XLEN-1:0] mstatus_cur, - input logic [XLEN-1:0] sstatus_val + input logic [XLEN-1:0] sstatus_data ); - logic [XLEN-1:0] next_val; - next_val = mstatus_cur; - next_val[SSTATUS_SIE_BIT] = sstatus_val[SSTATUS_SIE_BIT]; - next_val[SSTATUS_SPIE_BIT] = sstatus_val[SSTATUS_SPIE_BIT]; - next_val[SSTATUS_SPP_BIT] = sstatus_val[SSTATUS_SPP_BIT]; - return next_val; + logic [XLEN-1:0] next_data; + next_data = mstatus_cur; + next_data[SSTATUS_SIE_BIT] = sstatus_data[SSTATUS_SIE_BIT]; + next_data[SSTATUS_SPIE_BIT] = sstatus_data[SSTATUS_SPIE_BIT]; + next_data[SSTATUS_SPP_BIT] = sstatus_data[SSTATUS_SPP_BIT]; + return next_data; endfunction function automatic logic [XLEN-1:0] mirror_mstatus_to_sstatus( input logic [XLEN-1:0] sstatus_cur, - input logic [XLEN-1:0] mstatus_val + input logic [XLEN-1:0] mstatus_data ); - logic [XLEN-1:0] next_val; - next_val = sstatus_cur; - next_val[SSTATUS_SIE_BIT] = mstatus_val[SSTATUS_SIE_BIT]; - next_val[SSTATUS_SPIE_BIT] = mstatus_val[SSTATUS_SPIE_BIT]; - next_val[SSTATUS_SPP_BIT] = mstatus_val[SSTATUS_SPP_BIT]; - return next_val; + logic [XLEN-1:0] next_data; + next_data = sstatus_cur; + next_data[SSTATUS_SIE_BIT] = mstatus_data[SSTATUS_SIE_BIT]; + next_data[SSTATUS_SPIE_BIT] = mstatus_data[SSTATUS_SPIE_BIT]; + next_data[SSTATUS_SPP_BIT] = mstatus_data[SSTATUS_SPP_BIT]; + return next_data; endfunction // Pack the discrete FP status bits into the architectural FCSR layout. diff --git a/rv64cpu/rtl/core/include/csr_pkg.sv b/rv64cpu/rtl/core/include/csr_pkg.sv index a225da2..a908538 100644 --- a/rv64cpu/rtl/core/include/csr_pkg.sv +++ b/rv64cpu/rtl/core/include/csr_pkg.sv @@ -102,16 +102,16 @@ package csr_pkg; // Interrupt Cause Codes (mcause.Interrupt == 1) // --------------------------------- // User Mode Interrupts - localparam logic [XLEN-1:0] INT_U_SOFTWARE = 64'd0; // User Mode Software Interrupt - localparam logic [XLEN-1:0] INT_U_TIMER = 64'd4; // User Mode Timer Interrupt - localparam logic [XLEN-1:0] INT_U_EXTERNAL = 64'd8; // User Mode External Interrupt + localparam logic [XLEN-1:0] INTERRUPT_U_SOFTWARE = 64'd0; // User Mode Software Interrupt + localparam logic [XLEN-1:0] INTERRUPT_U_TIMER = 64'd4; // User Mode Timer Interrupt + localparam logic [XLEN-1:0] INTERRUPT_U_EXTERNAL = 64'd8; // User Mode External Interrupt // Supervisor Mode Interrupts - localparam logic [XLEN-1:0] INT_S_SOFTWARE = 64'd1; // Supervisor Mode Software Interrupt - localparam logic [XLEN-1:0] INT_S_TIMER = 64'd5; // Supervisor Mode Timer Interrupt - localparam logic [XLEN-1:0] INT_S_EXTERNAL = 64'd9; // Supervisor Mode External Interrupt + localparam logic [XLEN-1:0] INTERRUPT_S_SOFTWARE = 64'd1; // Supervisor Mode Software Interrupt + localparam logic [XLEN-1:0] INTERRUPT_S_TIMER = 64'd5; // Supervisor Mode Timer Interrupt + localparam logic [XLEN-1:0] INTERRUPT_S_EXTERNAL = 64'd9; // Supervisor Mode External Interrupt // Machine Mode Interrupts - localparam logic [XLEN-1:0] INT_M_SOFTWARE = 64'd3; // Machine Mode Software Interrupt - localparam logic [XLEN-1:0] INT_M_TIMER = 64'd7; // Machine Mode Timer Interrupt - localparam logic [XLEN-1:0] INT_M_EXTERNAL = 64'd11; // Machine Mode External Interrupt + localparam logic [XLEN-1:0] INTERRUPT_M_SOFTWARE = 64'd3; // Machine Mode Software Interrupt + localparam logic [XLEN-1:0] INTERRUPT_M_TIMER = 64'd7; // Machine Mode Timer Interrupt + localparam logic [XLEN-1:0] INTERRUPT_M_EXTERNAL = 64'd11; // Machine Mode External Interrupt endpackage diff --git a/rv64cpu/rtl/core/richard_core.sv b/rv64cpu/rtl/core/richard_core.sv index ad75cc8..a841253 100644 --- a/rv64cpu/rtl/core/richard_core.sv +++ b/rv64cpu/rtl/core/richard_core.sv @@ -37,9 +37,9 @@ module richard_core input logic [63:0] dmem_rdata, // Interface 3: External Interrupt Sources - input logic ext_timer_int, - input logic ext_software_int, - input logic ext_external_int + input logic ext_timer_interrupt, + input logic ext_software_interrupt, + input logic ext_external_interrupt ); localparam int CSR_MSTATUS_MIE_BIT = 3; @@ -159,7 +159,7 @@ module richard_core logic [4:0] wb_rs1_addr_in; logic [XLEN-1:0] wb_imm_in; logic wb_illegal_instr_in, wb_is_ecall_in, wb_is_ebreak_in, wb_is_mret_in, wb_is_sret_in; - logic wb_load_fault_flag, wb_store_fault_flag; + logic wb_load_fault, wb_store_fault; logic [XLEN-1:0] wb_fault_addr_in; // (10) WB Stage Outputs (to RegFile & CSR Unit) @@ -182,8 +182,8 @@ module richard_core logic [XLEN-1:0] csr_sepc_value; logic [XLEN-1:0] csr_satp_value; logic [XLEN-1:0] csr_trap_vector_value; - logic csr_trap_to_s_mode_flag; - logic csr_illegal_access_flag; + logic csr_trap_to_s_mode; + logic csr_illegal_access; // (11) Exception / Trap bus logic trap_illegal_instr, trap_is_ecall, trap_is_ebreak, trap_is_mret, trap_is_sret; @@ -203,7 +203,7 @@ module richard_core logic [XLEN-1:0] trap_ctrl_mepc; logic [XLEN-1:0] trap_ctrl_mcause; logic [XLEN-1:0] trap_ctrl_mtval; - logic trap_ctrl_to_s_mode_flag; + logic trap_ctrl_to_s_mode; logic trap_ctrl_mret_en; logic trap_ctrl_sret_en; logic csr_mstatus_mie; @@ -591,8 +591,8 @@ module richard_core .wb_is_ebreak (wb_is_ebreak_in), .wb_is_mret (wb_is_mret_in), .wb_is_sret (wb_is_sret_in), - .wb_load_fault (wb_load_fault_flag), - .wb_store_fault (wb_store_fault_flag), + .wb_load_fault (wb_load_fault), + .wb_store_fault (wb_store_fault), .wb_fault_addr (wb_fault_addr_in) ); @@ -618,11 +618,11 @@ module richard_core .mem_wb_is_ebreak (wb_is_ebreak_in), .mem_wb_is_mret (wb_is_mret_in), .mem_wb_is_sret (wb_is_sret_in), - .mem_wb_load_fault (wb_load_fault_flag), - .mem_wb_store_fault (wb_store_fault_flag), + .mem_wb_load_fault (wb_load_fault), + .mem_wb_store_fault (wb_store_fault), .mem_wb_fault_addr (wb_fault_addr_in), .csr_rdata (csr_rdata), - .csr_illegal_access (csr_illegal_access_flag), + .csr_illegal_access (csr_illegal_access), .wb_rd_wdata (wb_rd_wdata), .wb_rd_addr (wb_rd_addr), .wb_reg_write_en (wb_reg_write_en_out), @@ -650,7 +650,7 @@ module richard_core .csr_addr (csr_addr), .csr_rs1_data (csr_rs1_data), .trap_ctrl_trigger (trap_ctrl_trigger), - .trap_ctrl_to_s_mode(trap_ctrl_to_s_mode_flag), + .trap_ctrl_to_s_mode(trap_ctrl_to_s_mode), .trap_ctrl_mepc (trap_ctrl_mepc), .trap_ctrl_mcause (trap_ctrl_mcause), .trap_ctrl_mtval (trap_ctrl_mtval), @@ -670,8 +670,8 @@ module richard_core .csr_trap_vector (), .csr_trap_to_s_mode (), .csr_trap_vector_next (csr_trap_vector_value), - .csr_trap_to_s_mode_next(csr_trap_to_s_mode_flag), - .csr_illegal_access (csr_illegal_access_flag) + .csr_trap_to_s_mode_next(csr_trap_to_s_mode), + .csr_illegal_access (csr_illegal_access) ); trap_ctrl u_trap_ctrl ( @@ -688,9 +688,9 @@ module richard_core .trap_store_page_fault(trap_store_page_fault), .trap_bad_addr (trap_bad_addr), .trap_bad_instr (trap_bad_instr), - .ext_timer_int (ext_timer_int), - .ext_software_int (ext_software_int), - .ext_external_int (ext_external_int), + .ext_timer_interrupt (ext_timer_interrupt), + .ext_software_interrupt (ext_software_interrupt), + .ext_external_interrupt (ext_external_interrupt), .csr_mstatus_mie (csr_mstatus_mie), .csr_mtvec (csr_mtvec_base), .csr_stvec (csr_stvec_base), @@ -703,7 +703,7 @@ module richard_core .trap_mepc (trap_ctrl_mepc), .trap_mcause (trap_ctrl_mcause), .trap_mtval (trap_ctrl_mtval), - .trap_to_s_mode (trap_ctrl_to_s_mode_flag), + .trap_to_s_mode (trap_ctrl_to_s_mode), .mret_en (trap_ctrl_mret_en), .sret_en (trap_ctrl_sret_en), .trap_flush_req (trap_flush_req), diff --git a/rv64cpu/rtl/core/trap_ctrl.sv b/rv64cpu/rtl/core/trap_ctrl.sv index c5593ea..55278d9 100644 --- a/rv64cpu/rtl/core/trap_ctrl.sv +++ b/rv64cpu/rtl/core/trap_ctrl.sv @@ -38,9 +38,9 @@ module trap_ctrl input logic [INST_WIDTH-1:0] trap_bad_instr, // Interrupt pin from external source (asynchronous signal) - input logic ext_timer_int, // M-Mode Machine Timer Interrupt (MTIP) - input logic ext_software_int, // M-Mode Software Interrupt (MSIP) - input logic ext_external_int, // M-Mode External Interrupt (MEIP) + input logic ext_timer_interrupt, // M-Mode Machine Timer Interrupt (MTIP) + input logic ext_software_interrupt, // M-Mode Software Interrupt (MSIP) + input logic ext_external_interrupt, // M-Mode External Interrupt (MEIP) // Status read from CSR Unit input logic csr_mstatus_mie, // Global interrupt enable (MIE) @@ -128,22 +128,22 @@ module trap_ctrl // Interrupt Detection - Asynchronous, Protected by MIE logic has_interrupt; - logic [XLEN-1:0] int_cause; + logic [XLEN-1:0] interrupt_cause; always_comb begin has_interrupt = 1'b0; - int_cause = '0; + interrupt_cause = '0; if (csr_mstatus_mie) begin - if (ext_external_int) begin + if (ext_external_interrupt) begin has_interrupt = 1'b1; - int_cause = INTERRUPT_BIT | INT_M_EXTERNAL; - end else if (ext_software_int) begin + interrupt_cause = INTERRUPT_BIT | INTERRUPT_M_EXTERNAL; + end else if (ext_software_interrupt) begin has_interrupt = 1'b1; - int_cause = INTERRUPT_BIT | INT_M_SOFTWARE; - end else if (ext_timer_int) begin + interrupt_cause = INTERRUPT_BIT | INTERRUPT_M_SOFTWARE; + end else if (ext_timer_interrupt) begin has_interrupt = 1'b1; - int_cause = INTERRUPT_BIT | INT_M_TIMER; + interrupt_cause = INTERRUPT_BIT | INTERRUPT_M_TIMER; end end end @@ -153,7 +153,7 @@ module trap_ctrl logic trap_is_interrupt; assign trap_is_interrupt = has_interrupt; - assign trap_cause_value = has_interrupt ? int_cause : sync_cause; + assign trap_cause_value = has_interrupt ? interrupt_cause : sync_cause; assign trap_mtval_value = has_interrupt ? '0 : sync_mtval; assign trap_trigger = has_interrupt | has_sync_exception; diff --git a/rv64cpu/tb/compliance/compliance_runner.cc b/rv64cpu/tb/compliance/compliance_runner.cc index 5cc4709..b29662a 100644 --- a/rv64cpu/tb/compliance/compliance_runner.cc +++ b/rv64cpu/tb/compliance/compliance_runner.cc @@ -306,9 +306,9 @@ bool load_elf64(const std::string& path, ByteMemory* memory, Symbols* symbols) { void tick(Vrichard_core* top, ByteMemory* memory) { top->imem_rdata = memory->read_u32(top->imem_addr); top->dmem_rdata = top->dmem_read_en ? memory->read_u64(top->dmem_addr & ~0x7ULL) : 0; - top->ext_timer_int = 0; - top->ext_software_int = 0; - top->ext_external_int = 0; + top->ext_timer_interrupt = 0; + top->ext_software_interrupt = 0; + top->ext_external_interrupt = 0; top->clk = 0; top->eval(); diff --git a/rv64cpu/tb/core/tb_core.cc b/rv64cpu/tb/core/tb_core.cc index 39d4f19..b915df4 100644 --- a/rv64cpu/tb/core/tb_core.cc +++ b/rv64cpu/tb/core/tb_core.cc @@ -352,17 +352,17 @@ public: Vrichard_core* top; MemoryModel mem; uint64_t sim_time; - bool ext_timer_int_level; - bool ext_software_int_level; - bool ext_external_int_level; + bool ext_timer_interrupt_level; + bool ext_software_interrupt_level; + bool ext_external_interrupt_level; #if VM_TRACE VerilatedVcdC* tfp; #endif CoreSim() : sim_time(0), - ext_timer_int_level(false), - ext_software_int_level(false), - ext_external_int_level(false) { + ext_timer_interrupt_level(false), + ext_software_interrupt_level(false), + ext_external_interrupt_level(false) { top = new Vrichard_core; #if VM_TRACE Verilated::traceEverOn(true); @@ -401,9 +401,9 @@ public: top->dmem_rdata = 0; } - top->ext_timer_int = ext_timer_int_level; - top->ext_software_int = ext_software_int_level; - top->ext_external_int = ext_external_int_level; + top->ext_timer_interrupt = ext_timer_interrupt_level; + top->ext_software_interrupt = ext_software_interrupt_level; + top->ext_external_interrupt = ext_external_interrupt_level; // Rising edge top->clk = 0; @@ -432,9 +432,9 @@ public: } void set_interrupt_lines(bool timer, bool software, bool external) { - ext_timer_int_level = timer; - ext_software_int_level = software; - ext_external_int_level = external; + ext_timer_interrupt_level = timer; + ext_software_interrupt_level = software; + ext_external_interrupt_level = external; } void clear_interrupts() { diff --git a/rv64cpu/tb/unit/tb_trap_ctrl.cc b/rv64cpu/tb/unit/tb_trap_ctrl.cc index c57e6da..29d3439 100644 --- a/rv64cpu/tb/unit/tb_trap_ctrl.cc +++ b/rv64cpu/tb/unit/tb_trap_ctrl.cc @@ -56,9 +56,9 @@ void drive_defaults(Vtrap_ctrl* dut) { dut->trap_bad_addr = 0; dut->trap_bad_instr = 0; - dut->ext_timer_int = 0; - dut->ext_software_int = 0; - dut->ext_external_int = 0; + dut->ext_timer_interrupt = 0; + dut->ext_software_interrupt = 0; + dut->ext_external_interrupt = 0; dut->csr_mstatus_mie = 0; dut->csr_mtvec = 0x800ULL; @@ -157,9 +157,9 @@ int main(int argc, char** argv) { dut->csr_priv_mode = PRIV_MODE_M; dut->csr_mtvec = 0x300ULL | 0x1ULL; // vectored mode dut->wb_pc = 0x4000ULL; - dut->ext_timer_int = 1; - dut->ext_software_int = 1; - dut->ext_external_int = 1; + dut->ext_timer_interrupt = 1; + dut->ext_software_interrupt = 1; + dut->ext_external_interrupt = 1; dut->eval(); const uint64_t expected_vectored_pc = 0x300ULL + (INT_M_EXTERNAL << 2); check(dut->trap_trigger, "InterruptPriority_Triggers"); @@ -169,7 +169,7 @@ int main(int argc, char** argv) { drive_defaults(dut); dut->csr_mstatus_mie = 0; - dut->ext_external_int = 1; + dut->ext_external_interrupt = 1; dut->eval(); check(!dut->trap_trigger, "InterruptMasked_NoTrigger"); @@ -179,7 +179,7 @@ int main(int argc, char** argv) { dut->csr_stvec = 0x880ULL | 0x1ULL; // vectored supervisor mode dut->csr_mideleg = (1ULL << INT_M_TIMER); dut->wb_pc = 0xABCDULL; - dut->ext_timer_int = 1; + dut->ext_timer_interrupt = 1; dut->eval(); const uint64_t expected_sv_pc = 0x880ULL + (INT_M_TIMER << 2); check(dut->trap_to_s_mode, "InterruptDelegation_ToSupervisor"); -- cgit v1.2.3