Cache RTL hardening: fwd-buffer + folded data + sync-flush + MSHR - #1
Open
Aquaticfuller wants to merge 62 commits into
Open
Cache RTL hardening: fwd-buffer + folded data + sync-flush + MSHR#1Aquaticfuller wants to merge 62 commits into
Aquaticfuller wants to merge 62 commits into
Conversation
- add DataPartSplit and folded bank utility; propagate through core/wrappers/top - implement skewed folded data read/write - add full-line read handling for refill/evict/flush paths and way masks - preserve word offsets for part selection in cachepool/flamingo controllers - update tests and Bender.yml for new sources
Derive coalescer width from PartSplit, carry part_idx through the bypass/coal xbar meta, pack coalesced wdata/wmask into the addressed part of the full line, and slice responses back to part width before returning to the coalescer.
…retrieval path This change fixes response-path deadlock/X-propagation seen under VLSU traffic and flush/invalidate pressure by making response metadata stable under backpressure and guarding invalid retrieval entries.
…path - Stop using returned resp_info.bypass_coalescer for response sink select. - Add route FIFO to track request source (bypass vs coalescer) by handshake order. - Add bypass meta FIFO to track scalar core_meta + addr_offset from request side. - Use tracked metadata for scalar response meta/data extraction. - Add non-synthesis FIFO overflow checks.
Fix pseudo_dual_port_tcdm_wrapper when a full-line read collides with a masked write to the same bank address.
Insert a one-cycle preread bubble after creating a new pending line so back-to-back same-line requests see the updated READ_PEND/WRITE_PEND state instead of allocating a second way before the bank write becomes visible.
… kernels, but have performance issue Serialize read refill bursts in cachepool_cache_ctrl and use latched request metadata for assembled refill responses. Add preread request buffering and same-line hazard handling in insitu_cache_core, plus deferred folded-bank writeback. Fix folded tcdm wrapper arbitration for selected-way accesses and full-line refill/flush reads.
Disable multi-read-pend, plumb hash-based way selection through the cache controller/core/decoder, and move dirty/LRU handling off the meta SRAM with folded-bank forwarding and meta-skip plumbing.
…lers Add a reusable 1-entry write-back forwarding buffer (sram_forwarding_buffer) that sits between each SRAM access controller and its downstream SRAM banks. The buffer caches one SRAM row in registers: matching reads return buffer data, writes merge into the buffer, and the buffer is invalidated on address change.
writeback, and core hazard relaxation port
Upgrade the 1-entry SRAM forwarding buffer from transparent (step 1) to
active write-back mode (step 2):
- sram_forwarding_buffer: part-aware hit checking (PartSplit parameter),
dirty tracking with write-back mask, stale-data invalidation on
concurrent SRAM writes, and combinational hit outputs for gating
downstream SRAM operations.
- insitu_cache_bank_access_controller: FSM gates SRAM reads on buffer
hit, absorbs writes into buffer on hit, explicit writeback state
(ACCESS_WRITEBACK) for dirty eviction, and speculative writeback
(UseSpecWbIdle / UseSpecWbAddrTrans) that issues writebacks alongside
reads when the pseudo_dual_port can resolve the R/W conflict.
- insitu_cache_core: bank_write_data_buf_hit_i input relaxes the
write-read hazard when the data buffer absorbs the write (currently
no-op since data buffer is disabled).
Meta buffer enabled with speculative writeback -- 86% read hit rate,
71% write absorption on cache-vector-rw. Data buffer disabled pending
resolution of bank_write_cache_req all-ways broadcast issue.
All tests pass: cache-vector-rw, cache-line-rw-smoke, cache-rlc-mimic,
cache-mix-pressure, RLC M1_N1350_K100.
Add rd_total, wr_total, sram_rd, and wb counters to sram_forwarding_buffer. A final-block $display prints per-instance hit/miss/merge/writeback statistics at end of simulation, reporting read hit rate, write absorption rate, SRAM read count, and writeback count for each buffer instance.
- Data path: AllowReadDuringWrite + buffer + spec-WB all on.
Vector load: ~1 elem/cycle on warm hits; vector store: partial-
coverage absorbed into buffer.
- sram_forwarding_buffer: re-enable wr_concurrent_hit.
- 4 cache tests pass.
- Known: RLC hits meta-side MSHR-subarray race ~44.85us; pending fix.
- Replace single-index part tracking ({buf_part_idx_q, buf_all_parts_q}
and sram_rd_part_idx_q/all_parts_q) with a bitmap
buf_parts_valid_q / sram_rd_parts_q. Subset checks for hit/coverage;
wb_mask_o loops over the bitmap.
- Same-line populate ACCUMULATES (gated to clean buffer, no concurrent
write): OR new parts into bitmap, hold bytes for already-cached parts.
Avoids spurious writebacks when successive parts of the same clean
line are probed.
- Allow wr_buf_hit during sram_rd_pend_q when the write parts are
already cached AND disjoint from the SRAM-read parts. Populate and
absorb run in parallel on disjoint byte lanes.
Adds upstream_write_ready_o between access ctrl and cache_core, plumbed through tcdm_wrapper as bank_write_ready_i. Phase 1: always-1 (no behavioral change). Phase 2: deassert during wb_active_q and ACCESS_STALL to make the cache controller honor transient buffer states explicitly. HK-1 SVA enforces the contract. Also drops the conservative "different-addr write -> invalidate clean buffer" path so the buffer can act as a sticky victim entry. All 4 cache tests pass; vector-rw slightly faster (1:26 vs 1:32 baseline).
Adds the (D) ACCUMULATE-CONCURRENT-MERGE branch to the buffer's populate path -- preserves old parts on same-line populate with concurrent covered write instead of REPLACE-merge dropping them. Gated by a new advisory bit `wr_target_valid_i`, driven by cache_core's `bank_write_target_valid_o = (status[depth][way] == VALID)`. When the target line is in PEND state (mid miss/refill protocol), (D) falls back to REPLACE-merge so the cache controller's status array stays synchronized -- prevents the WR_CONFLICT_STALL liveness bug that ungated (D) triggered around 331us into cache-vector-rw. All 4 cache tests pass; vector-rw 1:30 vs 1:32 baseline.
- bank_read_all_parts_o now fires on every normal preread, not just
evict/refill: one preread populates all PartSplit parts of the bank
line, raising buffer hit-rate for stride-1 vector load/store patterns.
- New EnableRawForwarding parameter on the forwarding buffer (default
off): when on, a same-cycle read+write that both hit the buffer for
the same line returns the post-write merged data via a byte-mask mux
into buf_rd_data_q, instead of the pre-write (read-before-write)
value. Adds a wr_data->buf_rd_data_q combinational path before the
response register; mildly increases the cycle's critical path.
- Surfaced at tcdm_wrapper as DataFwdBufEnableRawForwarding /
MetaFwdBufEnableRawForwarding (both on in this build) and plumbed
through insitu_cache_bank_access_controller into both single-entry
and multi-entry buffer instances.
Drop the !(write & wr_hit) gate on spec_wb_fire and let the FSM drive a concurrent writeback during an absorb when there is also a read miss to a different line. Buffer's wb_data_o/wb_mask_o now combinationally merge the absorb's bytes/parts so SRAM stays consistent after wb_done clears dirty. Address invariant wr_addr_i == buf_addr_q under buf_dirty_q & wr_hit_comb_o is guaranteed by C3 + wr_full_hit's clean-buffer gate.
New parameter on sram_forwarding_buffer and insitu_cache_bank_access_controller. When set, rd_inflight_hit fires even with a concurrent same-addr write; the buffer captures sram_rdata_i with wr_data overlaid on wr_mask bytes, matching the post-write value buf_data_q lands on at posedge T+1 via the (D) / REPLACE-with-merge populate path. Eliminates the redundant SRAM read that the previous gate forced for this scenario. Enabled on the data access-ctrl instance only; meta path unchanged. Measured: cache-vector-rw 7475 -> 7026 cycles (-6%), stable across 3 runs. cache-line-rw-smoke, cache-rlc-mimic, and cache-mix-pressure pass with matching baseline signatures.
wr_parts_bm, has_wr_data, and wr_full_line previously asserted whenever wr_mask_i was non-zero, regardless of wr_req_i. When upstream holds wr_mask_i/wr_addr_i stable between requests, the buffer's combinational hit signals could fire spuriously on cycles with wr_req_i=0, and the (B) PEND_DISJOINT NBA path -- which gates only on wr_buf_hit_pend_disjoint -- could absorb stale wr_data into buf_data_q and dirty the buffer on a phantom write.
… SYNC_CTRL_FLUSH The sync-ctrl FSM was silently dropping multi-way dirty data during flush/init operations. Four coordinated fixes: - Source has_dirty from dirty_rf[sync_ctrl_ptr_q][i] directly instead of (flush_read_cache_status[i] == VALID && flush_read_cache_dirty[i]). The status path is gated by way_mask: on per-way writebacks the one-hot mask returns 0 for non-selected ways' status, so has_dirty oscillated back to 0 mid-eviction and the FSM advanced sync_ctrl_ptr leaving remaining dirty ways behind. dirty_rf is per-set, always fresh, and not gated by way_mask. - Sticky-dirty gate: hold the dirty-eviction sub-branch until flush_full_wait_q / flush_full_data_valid_q resolve (i.e. wb_done), so a momentary has_dirty deassertion cannot drop the FSM out mid-eviction. - Read-completion guard: only latch flush_full_tag_d / flush_full_mask_d for the writeback once bank_read_cache_addr_q == sync_ctrl_ptr_q (= the meta SRAM read for this set has aligned). Previously the writeback could pick up stale tag/mask from a prior unrelated read. - Give flush bank-read priority while sync-ctrl is active (any state other than IDLE / FINISH), not only when proc is idle. Previously a draining proc-side read (e.g. a refill straggler) kept proc_read_cache_valid high and starved the flush_read handshake; bank_read_cache_addr_q never updated with the flush ptr; dirty_rf[addr_q] was then read for an unrelated set and the FSM silently cleared each set without writeback. Software gates new proc activity through cache_sync_ready_o so blocking residual proc reads during a sync is safe. - Hoisted dirty_rf and bank_read_cache_addr_q declarations to the top of the wrapper's signal block so the gen_sync_ctrl_fsm always_comb (which appears earlier in the file) can reference them.
In UseHashWaySelect=1 mode, dec_way_o was correctly assigned to _hash_way at the top of proc_bank_decode, but the "find miss way" loop further down ran unconditionally on every miss and overrode it with an LRU-picked way. Since LRU bits are unused in hash mode (stubbed by lru_meta_unused), the loop typically picked way 0 and clobbered the correct hash way. The cache_core's miss FSM then read dec_cache_status / dec_cache_dirty / dec_cache_tag from the wrong way and missed the displaced VALID+dirty victim at the actual hash way. The new tag's bank_write still went to the correct hash way via hash_way_fsm, so the dirty data was silently overwritten with no eviction queued -- subsequent readbacks returned stale DRAM contents. Fix: gate the LRU loop on !UseHashWaySelect. Repro & verification: software/tests/cache-coverage-min (8 stride-64KiB writes hashing to the same depth, then readback) -- 8/8 errors before, 0/8 after, with 8 evictions correctly shipped to DRAM.
…vides CacheLineWidth When InfoStoreWidth perfectly divides CacheLineWidth, MSHRPadWidth collapsed to 0 and `logic [MSHRPadWidth-1:0]` became the illegal range `logic [-1:0]` (vopt-13201). Reserve one slot in that case so the pad field stays at least InfoStoreWidth bits wide.
Without this gate, a request accepted during sync can issue its PEND meta-write during the FLUSH phase. That write races the sync invalidate; the line ends up INVALID and the subsequent refill response trips proc_assert_read_refill_reread in insitu_cache_core, losing the proc response and hanging the sim before EOC. Fixes cache-test-scalar regression.
Two fixes for a meta-corruption race in load-store_M16 mode-3 (3priv-1shr) that tripped proc_assert_read_refill_reread and the zero stored MSHR subarray assertion in insitu_cache_core: 1) CHECK_PEND drain extension. The existing wrapper-side drain signals (outstanding_refill_cnt, core_miss_valid, core_evic_valid, write_through_valid) miss in-flight installs latched inside cache_core. Add ~preread_task_q.valid and retr_fifo_empty to the drain check, then require those conditions to hold for CheckPendDrainCycles (=20) consecutive cycles before advancing to FLUSH. 2) Install-defer in CHECK_PEND/INIT/FLUSH/INVALID. Refill responses are held at the wrapper boundary during the sync FSM's bank-writing phases so installs cannot fire concurrent with the sync FSM's own bank writes. Covers the init-all path (READ_BANK -> INIT) which skips the CHECK_PEND drain, and acts as a backup for any residual pipeline state the drain misses. A sim-only META-TRACE probe (ifdef'd out under TARGET_SYNTHESIS) is included to ease future debug of meta-bank lifetime issues.
Both debug $displays are now off unless the sim is launched with +insitu_trace, matching the pattern wb_trace already uses. Default runs no longer spam the log; assertions and scoreboard errors are unaffected.
proc_read_cache_ready was driven unconditionally from bank_read_cache_ready, so the proc-side ready pulsed even when the arbiter was serving a FLUSH bank-read. cache_core saw a spurious handshake, advanced its install pipeline with whatever was on the read-data bus, and the install bank-write later committed garbage (status=VALID, tag=0), tripping proc_assert_read_refill_reread. - Restrict bank_read_sel_flush=1 to INIT/FLUSH/INVAL (the meta-writing phases). CHECK_PEND keeps proc priority so its drain can complete. - Gate proc_read_cache_ready with ~bank_read_sel_flush.
Extend the per-controller scoreboard so it stops flagging legitimate
cache behavior as RESP NO_LINE. The cache delivers correct data via
fwd-buffer hits, MSHR-merge, and refill-direct-forward responses, none
of which fire the proc_commit signal the original SB tracked.
New state in the scoreboard (verif-only, gated by `ifndef TARGET_SYNTHESIS`):
- sb_shadow: byte-granular line-data map keyed on {tag, depth} -- the
cache's own line identifier. Updated from THREE paths:
1. upstream WRITE requests (captures fwd-buffer state)
2. proc_commit refill installs / write-hit updates
3. downstream refill response (info-id keyed against the matching
outstanding refill request), populated the moment the refill
lands at the wrapper boundary -- BEFORE the install commits,
so direct-forward responses to merged readers are covered.
- same-cycle proc_commit bypass: when the install and upresp fire in
the same posedge, the NBA-updated sb_shadow isn't visible yet;
directly compare upresp_data against proc_commit_data.
When sb_find_hit misses, the SB now checks the shadow. Match -> count
as SHADOW_HIT (no error). Disagree with full byte coverage ->
SHADOW_MISMATCH (real bug). No shadow record at all -> genuine
NO_LINE.
The wrapper change is verif-only: 5 new info inputs added to the SB
instantiation, all driven from existing wrapper signals inside the
synthesis-guard block. No RTL behavior change.
Impact on the 1t/4c sweep:
- cache-coverage: 11 -> 0
- cache-test-vector: 4 -> 0 (timeout remains, unrelated)
- gemv_M512: 22 -> 0
- gemv_M1024: 54 -> 0
- gemv-opt_M512: 13 -> 0
- gemv-opt_M1024: 27 -> 0
When sb_find_hit succeeds, the existing data check compares the SB's tracked sb_data[depth][way] against the cache's response. If a write reached the cache via a path that bypasses proc_commit (e.g. a store- hit absorbed by the fwd-buffer), sb_data goes stale and a legitimate read of the new value fires a spurious RESP DATA MISMATCH. Mirror the existing shadow fallback on the !sb_hit branch: on sb_data disagreement, consult sb_shadow. If the shadow agrees with the cache's response (with full byte coverage), bump shadow_hit instead of erroring. Real bugs where the shadow ALSO disagrees still fire DATA MISMATCH as before.
When a write commits to the same line between a read's decoder hit
and its upresp delivery, sb_data is updated but the cache's response
carries the data AT READ TIME. The upresp data check (which compares
upresp_data against current sb_data) then fires a spurious RESP DATA
MISMATCH even though the cache is correct.
Concretely (linked-list K10 case at t=32434..32438):
t=32434 UPREQ READ addr=0xe0000ec4 info=0x6083
t=32436 dec_valid cache hit, line byte 63 = 0x3c (matches sb_data)
t=32437 COMMIT sb_data[d][w] byte 63 <- 0x3d (intervening WRITE)
t=32438 upresp cache returns 0x3c (correct, read-time value)
SB compares against sb_data (0x3d) -> false MISMATCH
Fix: capture dec_cache_data at the moment of the decoder hit and pin
it to the request's info-keyed entry. The upresp check now compares
against that snapshot when present.
Wiring: one new SB input port (dec_info), connected to
preread_task_q.task_pay.request.info inside the synthesis-guarded SB
instantiation in tcdm_wrapper.sv. Verif-only -- no RTL behavior change.
Impact on 1t/4c sweep:
- linked-list K10/K100/K300: 1 -> 0 each
…is on With UseHashWaySelect=0 the cache_core's bank_read_way_mask falls back to all-ones, every way drives the SRAM port simultaneously, and the tile-level skewed-fold arbiter collapses the requests onto a single partition slot per column. The forwarding buffer masks this until eviction, then dirty data is lost. Catch the misconfiguration at elaboration.
SubarrayCntWidth was clog2(NumSubarray+1), one bit short for the +1 arithmetic in proc_refill's MSHR_FULL_STALL escape path: 3'b111 + 1 wrapped to 3'b000, gating retr_fifo_push off and silently dropping ALL pending sub-entry responses (originating cores hang on the load). Widen to clog2(NumSubarray+2). The < NumSubarray merge guard still bounds the stored count to 7, so the meta SRAM payload is unchanged. Also: - [VERIF] scoreboard: cross-check dec_cache_mask against an SB- mirrored MSHR sub-entry count on every dec_is_hit_pend pulse, to catch this class of meta-side hazard at the cycle it happens. - [RTL] tcdm_wrapper: align meta access-ctrl perf knobs with the data side (AllowReadDuringWrite=1, EnableInflightWriteMerge=1).
…o break grant-feedback comb loop
Replace assignment-pattern '{} with concatenation {} on the i_bypass_xbar
port connections. '{} on a packed-array lvalue (slv_rsp_o, slv_*_valid/ready)
is non-standard and leaves SpyGlass unable to trace the driver -> W123
'read but never set' false flag. Concatenation is the LRM-clean idiom;
behavior-identical (MSB-first matches the by-position ordering).
…ix PartSplit=1 - Promote UseForwardingBuffer from a hardcoded localparam to a top-level parameter on insitu_cache_tcdm_wrapper; thread it through cachepool_cache_ctrl. - Make the UseHashWaySelect guard fold-aware: skewed-fold (PartSplit>1) requires hash-way-select regardless of the buffer; the buffer requires it regardless of fold. So hash=0 is legal only for the unfolded cache with the buffer off. - Fix reversed part-selects that broke elaboration when PartSplit==1: use ascending +: form in cachepool_cache_ctrl (coalescing_req_part_idx) and insitu_cache_core (preread_part_idx); bit-identical when folded. Verified: folded (4/1/1) bit-identical pass; unfolded conventional (0/0/0) elaborates and passes load-store.
… drop barrel-shift mask off bank-write critical path
…e to parallel SOP (cut decoder critical-path depth)
…block_install | ~proc_read_valid (CSE + drop dead READ_BANK arm)
…edundant refill_full_read_req arm in hash mode)
…ll_ones[dec_way] (move 64-AND off the meta-write endpoint)
… + XOR), all_pend = s1; guard enum encoding
…_hit (wr_full_line implies it)
…ord_write_en (bank_req_write implies it)
…lName); import insitu_cache_pkg in insitu_cache_top for cache_addr_hashing
….3/T2.4) T2.3: drop redundant `& read_valid_i` from word_read_en (bank_req_read implies it). T2.4: precompute word_has_wmask from write_mask_i instead of reducing demuxed bank_wmask. Both logic-preserving / bit-identical.
Drive bank_req_read from a flat one-hot after the status case instead of the
status-enum encode -> case-decode round-trip, so the late read_valid_i is the
final AND on the worst (meta->meta read) path to the SRAM clock-gate enable.
read issued === status in {R_ONLY, WR_DIFF_BANK, WR_SAME_ADDR}
=== read_valid_i & ~(write_has_data & same-bank & diff-addr)
(absorption ~wd | wd&~C === ~(wd&C)). The wide read/write bank-addr compare
resolves from early write operands; read_valid_i ANDs in last. The status case
is retained for bank_addr / read_data_from_*_d / read_ready_o; behaviour is
bit-identical across all states, partitions, flush, and reset.
Collapse the nested `proc_write_select ? <per-way proc req> : <flush>` write request (data + meta access controllers) into a flat one-level (proc_write_cache_req & way==i) | (~proc_write_select & flush) and drop the data_proc_write_req / meta_proc_write_req intermediates. Equivalent: proc_write_select = proc_write_cache_req | proc_write_LRU_req, so proc_write_cache_req is 0 whenever proc_write_select=0 -> the proc term vanishes and the (~proc_write_select & flush) term broadcasts flush to every way, exactly as the old `: flush` arm did. meta_skip / mc_suppress kept verbatim; per-way `== i` comparator retained (no power-of-2 / one-hot indexing assumption). Shortens the late proc_write_cache_req path to the SRAM clock-gate enable.
…ic. (will cause vsim optimization failed)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cachepool_cache_ctrl.UseHashWaySelect=1.has_dirtyoscillation and bank-read starvation; drop unreliablependline_cntgate.SubarrayCntWidth(+1 escape bit) to fix drain-count overflow; reserve one MSHR slot whenInfoStoreWidthdividesCacheLineWidth; clear pend counter on invalidate/flush.WRITE-WATCH/WR-BALprobes.