fix(tomasulo): MMIO cache bypass, ICARUS/Yosys compat, AMO test coverage

adambagley · adambagley · commit e746d9aca576 · 2026-03-05T23:18:33.000-05:00
- Gate cache_fill_valid with !lq_is_mmio to prevent MMIO loads from
    filling L0 cache (DESIGN.md spec drift)
  - Fix sc_pending always_ff to separate async reset from sync flush for
    Yosys formal compatibility
  - Add !sq_committed guard to SC discard to prevent invalidating
    committed SQ entries (formal assertion fix)
  - Replace `inside {}` with explicit comparisons for Yosys compatibility
  - Rewrite amo_compute function to use assignment instead of return for
    Yosys compatibility
  - Add ICARUS guards: `ifndef ICARUS` on concurrent assertion, forward-
    declare sq_effective_addr, stub mem_rs_next_is_sc
  - Add missing AMO ports to tomasulo_wrapper_tb and o_next_issue_is_sc
    to reservation_station_tb for ICARUS elaboration
  - Mark load_queue as Verilator-only (ICARUS VPI crash on wide structs)
  - Add 9 wrapper-level integration tests: MMIO store and all 8 AMO ops
    (AMOADD, AMOXOR, AMOAND, AMOOR, AMOMIN, AMOMAX, AMOMINU, AMOMAXU)
diff --git a/hw/rtl/cpu_and_mem/cpu/tomasulo/DESIGN.md b/hw/rtl/cpu_and_mem/cpu/tomasulo/DESIGN.md
@@ -1366,12 +1366,12 @@ The existing FROST front-end optimizations are preserved:
     |    6. MEM (load results)                                               |
     |    7. ALU (shortest latency, can tolerate delay)                       |
     |                                                                        |
-    |  DESIGN PARAMETER: NUM_CDB_LANES                                       |
+    |  DESIGN NOTE: SINGLE CDB LANE                                          |
     |                                                                        |
-    |    - Initially NUM_CDB_LANES = 1 (single CDB)                          |
-    |    - Parameterized for future expansion                                |
-    |    - 2 lanes (INT + FP) would reduce contention significantly          |
-    |    - Trade-off: 2x wakeup comparators in RS, 2x ROB write ports        |
+    |    - Single CDB lane (one broadcast per cycle)                         |
+    |    - Multi-lane would require duplicating snoop comparators in every   |
+    |      RS entry and ROB slot, plus multi-port RAT — not justified for   |
+    |      this design's 7-FU configuration with fixed-priority arbitration  |
     |                                                                        |
     +------------------------------------------------------------------------+
 ```
@@ -1970,7 +1970,7 @@ existing `cpu_and_mem.sv` interface expectations.
     |                                                                        |
     |  PARAMETERS:                                                           |
     |    NUM_FUS = 7 (ALU, MUL, DIV, MEM, FP_ADD, FP_MUL, FP_DIV)            |
-    |    NUM_CDB_LANES = 1 (parameterized for future multi-bus expansion)    |
+    |    NUM_CDB_LANES = 1 (single lane; see design note above)             |
     |    ROB_TAG_WIDTH = 5 bits                                              |
     |    VALUE_WIDTH = max(XLEN, FLEN) = 64 bits                             |
     |                                                                        |
@@ -2220,7 +2220,7 @@ The schedule incorporates all Tomasulo components plus FROST-specific integratio
 | Week | Dates | Deliverable | Key Tasks |
 |------|-------|-------------|-----------|
 | 1 | 1/20 | **F/D Extensions + Architecture** | FPU integration, F/D instruction support, cocotb tests passing; high-level Tomasulo block diagrams; review existing FROST pipeline for integration points |
-| 2 | 1/27 | **Package Definition + D Timing** | Define tomasulo_pkg.sv (all types, structs, parameters); instruction routing table; ROB/RS entry structures; finish D extension timing closure (~300ps slack) |
+| 2 | 1/27 | **Package Definition + D Timing** | Define Tomasulo types/structs/parameters in riscv_pkg.sv; instruction routing table; ROB/RS entry structures; finish D extension timing closure (~300ps slack) |
 | 3 | 2/3 | **ROB Core Structure** | ROB circular buffer, allocation/deallocation logic, head/tail pointers, unified INT/FP entry fields (dest_rf, fp_flags), basic valid/done tracking |
 | 4 | 2/10 | **ROB Commit + Serialization** | ROB commit logic (INT/FP writeback, FP flag accumulation); exception handling; WFI stall-at-head; CSR execute-at-commit; FENCE/FENCE.I handling |
 | 5 | 2/17 | **RAT + Checkpointing** | INT RAT (x0-x31 mapping), FP RAT (f0-f31 mapping); checkpoint storage (4 checkpoints); checkpoint allocation on branch; restore on misprediction; RAS pointer in checkpoint |
@@ -2326,8 +2326,8 @@ The schedule incorporates all Tomasulo components plus FROST-specific integratio
 - **Core Tomasulo**: ROB, INT RAT, FP RAT, Dispatch unit
 - **Reservation Stations**: INT_RS, MUL_RS, MEM_RS, FP_RS, FMUL_RS, FDIV_RS (generic + instances)
 - **Memory Subsystem**: Load Queue (with L0 cache integration, MMIO, FP64), Store Queue (with forwarding, MMIO, FP64)
-- **Result Broadcast**: CDB arbiter (7 FUs, FLEN-wide, lane-parameterized)
-- **Package**: tomasulo_pkg.sv with all types, structs, parameters
+- **Result Broadcast**: CDB arbiter (7 FUs, FLEN-wide, single-lane)
+- **Package**: All Tomasulo types, structs, and parameters live in riscv_pkg.sv
 
 ### Integration
 - Integrated FROST-Tomasulo CPU with out-of-order execution for integer, memory, and floating-point operations
diff --git a/hw/rtl/cpu_and_mem/cpu/tomasulo/fu_shims/int_muldiv_shim.sv b/hw/rtl/cpu_and_mem/cpu/tomasulo/fu_shims/int_muldiv_shim.sv
@@ -102,17 +102,21 @@ module int_muldiv_shim (
   // ---------------------------------------------------------------------------
   // MUL in-flight + flush tracking (single in-flight, unchanged)
   // ---------------------------------------------------------------------------
-  logic mul_in_flight;
-  logic mul_flushed;
+  logic                                                        mul_in_flight;
+  logic                                                        mul_flushed;
 
   // Forward declarations for valid signals from FUs
-  logic multiplier_valid_input;
-  logic multiplier_valid_output;
-  logic divider_valid_input;
-  logic divider_valid_output;
+  logic                                                        multiplier_valid_input;
+  logic                                                        multiplier_valid_output;
+  logic                                                        divider_valid_input;
+  logic                                                        divider_valid_output;
 
-  logic mul_flush_inflight;
-  logic mul_flush_launching;
+  // Forward declarations for signals used before their primary declaration
+  logic                 [riscv_pkg::ReorderBufferTagWidth-1:0] mul_tag_reg;
+  riscv_pkg::instr_op_e                                        mul_op_reg;
+
+  logic                                                        mul_flush_inflight;
+  logic                                                        mul_flush_launching;
 
   assign mul_flush_inflight = mul_in_flight & (i_flush | (i_flush_en & is_younger(
       mul_tag_reg, i_flush_tag, i_rob_head_tag
@@ -173,8 +177,7 @@ module int_muldiv_shim (
       .o_completing_next_cycle(mul_completing_next_cycle)
   );
 
-  logic                 [riscv_pkg::ReorderBufferTagWidth-1:0] mul_tag_reg;
-  riscv_pkg::instr_op_e                                        mul_op_reg;
+  // mul_tag_reg and mul_op_reg declared above (forward declaration for ICARUS)
 
   always_ff @(posedge i_clk or negedge i_rst_n) begin
     if (!i_rst_n) begin
diff --git a/hw/rtl/cpu_and_mem/cpu/tomasulo/load_queue/README.md b/hw/rtl/cpu_and_mem/cpu/tomasulo/load_queue/README.md
@@ -127,9 +127,10 @@ total). Rationale:
 - **Formal**: `ifdef FORMAL` block with BMC (depth 12) and cover (depth 20).
   Assertions check pointer/count consistency, issue prerequisites, MMIO
   ordering, CDB back-pressure, and flush behavior.
-- **Cocotb**: 21 unit tests covering reset, allocation, address update,
+- **Cocotb**: 38 unit tests covering reset, allocation, address update,
   LW/LB/LBU/LH/LHU, SQ forwarding, MMIO, FLD two-phase, FLW NaN-boxing,
-  flush, ordering, back-pressure, and constrained random.
+  flush, ordering, back-pressure, constrained random, LR/SC reservation,
+  and AMO read-modify-write operations.
 
 ## Files
 
diff --git a/hw/rtl/cpu_and_mem/cpu/tomasulo/load_queue/load_queue.sv b/hw/rtl/cpu_and_mem/cpu/tomasulo/load_queue/load_queue.sv
@@ -497,7 +497,7 @@ module load_queue #(
   end
 
   assign cache_fill_valid = i_mem_read_valid && mem_outstanding && lq_valid[issued_idx]
-      && !lq_is_lr[issued_idx] && !lq_is_amo[issued_idx];
+      && !lq_is_mmio[issued_idx] && !lq_is_lr[issued_idx] && !lq_is_amo[issued_idx];
   assign cache_fill_addr = cache_fill_actual_addr;
   assign cache_fill_data = i_mem_read_data;
 
diff --git a/hw/rtl/cpu_and_mem/cpu/tomasulo/reorder_buffer/reorder_buffer.sv b/hw/rtl/cpu_and_mem/cpu/tomasulo/reorder_buffer/reorder_buffer.sv
@@ -182,6 +182,12 @@ module reorder_buffer (
     end
   endfunction
 
+  // Forward declarations for ICARUS (used in debug assigns before main decl)
+  logic [ReorderBufferTagWidth:0] head_ptr;
+  logic [ReorderBufferTagWidth:0] tail_ptr;
+  logic full;
+  logic empty;
+
   // ===========================================================================
   // Debug Signals (for verification)
   // ===========================================================================
@@ -250,17 +256,13 @@ module reorder_buffer (
   logic [ReorderBufferDepth-1:0] rob_is_sc;
   logic [ReorderBufferDepth-1:0] rob_is_compressed;
 
-  // Head and tail pointers (with extra bit for full/empty detection)
-  logic [ReorderBufferTagWidth:0] head_ptr;
-  logic [ReorderBufferTagWidth:0] tail_ptr;
+  // Head and tail pointers (declared above for ICARUS forward ref)
 
   // Derived pointer values (without wrap bit)
   logic [ReorderBufferTagWidth-1:0] head_idx;
   logic [ReorderBufferTagWidth-1:0] tail_idx;
 
-  // Status signals
-  logic full;
-  logic empty;
+  // Status signals (full and empty declared above for ICARUS forward ref)
   logic [ReorderBufferTagWidth:0] count;
 
   // Head entry fields for commit — RAM-backed fields are driven by RAM
diff --git a/hw/rtl/cpu_and_mem/cpu/tomasulo/tomasulo_wrapper/tomasulo_wrapper.sv b/hw/rtl/cpu_and_mem/cpu/tomasulo/tomasulo_wrapper/tomasulo_wrapper.sv
@@ -371,6 +371,15 @@ module tomasulo_wrapper (
   // ===========================================================================
   riscv_pkg::cdb_broadcast_t cdb_bus;
 
+  // Forward declarations: adapter→arbiter signals (used here, defined below)
+  riscv_pkg::fu_complete_t   alu_adapter_to_arbiter;
+  riscv_pkg::fu_complete_t   mul_adapter_to_arbiter;
+  riscv_pkg::fu_complete_t   div_adapter_to_arbiter;
+  riscv_pkg::fu_complete_t   mem_adapter_to_arbiter;
+  riscv_pkg::fu_complete_t   fp_add_adapter_to_arbiter;
+  riscv_pkg::fu_complete_t   fp_mul_adapter_to_arbiter;
+  riscv_pkg::fu_complete_t   fp_div_adapter_to_arbiter;
+
 `ifdef VERILATOR
   // Override slots 0-6: internal FU pipelines replace external i_fu_complete.
   // Slots 4-6 use a priority mux: internal FP adapter takes precedence,
@@ -486,7 +495,7 @@ module tomasulo_wrapper (
   // ===========================================================================
   riscv_pkg::rs_issue_t    int_rs_issue_w;  // INT_RS issue output
   riscv_pkg::fu_complete_t alu_shim_out;  // ALU shim → adapter
-  riscv_pkg::fu_complete_t alu_adapter_to_arbiter;  // adapter → arbiter
+  // alu_adapter_to_arbiter declared above (forward declaration)
   logic                    alu_adapter_result_pending;
   logic                    alu_fu_busy;  // always 0 for single-cycle ALU
   logic                    int_rs_fu_ready;
@@ -499,8 +508,7 @@ module tomasulo_wrapper (
   riscv_pkg::rs_issue_t    mul_rs_issue_w;  // MUL_RS issue output (internal)
   riscv_pkg::fu_complete_t mul_shim_out;  // shim MUL → adapter
   riscv_pkg::fu_complete_t div_shim_out;  // shim DIV → adapter
-  riscv_pkg::fu_complete_t mul_adapter_to_arbiter;  // adapter → arbiter slot 1
-  riscv_pkg::fu_complete_t div_adapter_to_arbiter;  // adapter → arbiter slot 2
+  // mul/div_adapter_to_arbiter declared above (forward declaration)
   logic                    mul_adapter_result_pending;
   logic                    div_adapter_result_pending;
   logic                    muldiv_busy;
@@ -521,7 +529,7 @@ module tomasulo_wrapper (
   // MEM (Load) Pipeline: LQ → adapter → CDB arbiter slot 3
   // ===========================================================================
   riscv_pkg::fu_complete_t lq_fu_complete;  // LQ → adapter
-  riscv_pkg::fu_complete_t mem_adapter_to_arbiter;  // adapter → arbiter slot 3
+  // mem_adapter_to_arbiter declared above (forward declaration)
   logic mem_adapter_result_pending;
 
   // ===========================================================================
@@ -634,7 +642,7 @@ module tomasulo_wrapper (
   // ===========================================================================
   riscv_pkg::rs_issue_t fp_rs_issue_w;  // FP_RS issue output (internal)
   riscv_pkg::fu_complete_t fp_add_shim_out;  // shim → adapter
-  riscv_pkg::fu_complete_t fp_add_adapter_to_arbiter;  // adapter → arbiter
+  // fp_add_adapter_to_arbiter declared above (forward declaration)
   logic fp_add_adapter_result_pending;
   logic fp_add_busy;
   logic fp_rs_fu_ready;
@@ -646,7 +654,7 @@ module tomasulo_wrapper (
   // ===========================================================================
   riscv_pkg::rs_issue_t    fmul_rs_issue_w;  // FMUL_RS issue output (internal)
   riscv_pkg::fu_complete_t fp_mul_shim_out;
-  riscv_pkg::fu_complete_t fp_mul_adapter_to_arbiter;
+  // fp_mul_adapter_to_arbiter declared above (forward declaration)
   logic                    fp_mul_adapter_result_pending;
   logic                    fp_mul_busy;
   logic                    fmul_rs_fu_ready;
@@ -658,7 +666,7 @@ module tomasulo_wrapper (
   // ===========================================================================
   riscv_pkg::rs_issue_t    fdiv_rs_issue_w;  // FDIV_RS issue output (internal)
   riscv_pkg::fu_complete_t fp_div_shim_out;
-  riscv_pkg::fu_complete_t fp_div_adapter_to_arbiter;
+  // fp_div_adapter_to_arbiter declared above (forward declaration)
   logic                    fp_div_adapter_result_pending;
   logic                    fp_div_busy;
   logic                    fdiv_rs_fu_ready;
diff --git a/verif/cocotb_tests/tomasulo/tomasulo_wrapper/test_tomasulo_wrapper.py b/verif/cocotb_tests/tomasulo/tomasulo_wrapper/test_tomasulo_wrapper.py
