Skip to content

Commit 9c9bb37

Browse files
committed
cache: Fix multi-way L0D corruption
L0D was broken with >1-way due to a flush bug: a flushed dirty line wrote back all zeros, because CacheFlushUnit clears coh in the same cycle it issues the writeback, and the writeback's data beats then mux through s2_tag_match_way, which ands in coh.isValid and so selects nothing. Select the way the writeback actually asked for instead. This only bit nWays > 1 because chisel's Mux1H discards its select when given a single input; for a still-valid line the two selects are identical, so this is a no-op wherever the cache already works. Found thru & verified on vecadd (64 sets, 4 ways, 16-nMSHRs L0D). Detailed agent log in docs/l0d-multiway-corruption.md.
1 parent e512e2e commit 9c9bb37

2 files changed

Lines changed: 245 additions & 1 deletion

File tree

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
# L0d data corruption at `nWays > 1`
2+
3+
**Status:** root-caused and **fixed** in `MuonDCache.scala`. `vecadd` now `*** PASSED ***` at
4+
64 sets x 4 ways (549,479 cycles); previously asserted at ~12.5 us.
5+
**Date:** 2026-08-12
6+
**Impact:** unblocks `nWays > 1` on the L0d. Note that associativity turned out **not** to fix
7+
the `vecadd` bandwidth problem it was pursued for — see §1.
8+
9+
---
10+
11+
## 1. Why this matters: it blocks the remaining `vecadd` gap
12+
13+
`vecadd` (N=65536, 2 cores, ILP2 w2) delivers **1.963 B/cycle = 24.5% of the 8 B/cycle AXI
14+
port** after the replay-depth fix (radiance `1be06d158`, which took it from 17.0% → 24.5%,
15+
1.45x). Everything below the L0d has been measured and exonerated:
16+
17+
| level | evidence it is *not* the limiter |
18+
|---|---|
19+
| L2 / bus | `l1_out` A-channel stall = **0.00%** over 400k cycles |
20+
| DRAM | `dram_full` = **0.0%**, avg 1.54 reads outstanding, ~21 cyc service |
21+
| AXI port | 2.297 B/cyc actual = **28.7%** utilized (32 B bursts) |
22+
| source shrinkers | inward/outward counters byte-identical; no ID exhaustion |
23+
24+
The binding constraint is the **L0d refusing its own input 74% of cycles**. Splitting
25+
`s2_nack` by source (qualified by `s2_valid`, cross-checked against the shim's independent
26+
count of 6,199):
27+
28+
```
29+
L0d m16_w1_s64 s2_valid=18677 nack=6199 (33%) miss=5161 (83%) victim=256 (4%) idxmatch=7398 (40%)
30+
```
31+
32+
83% of nacks are `s2_nack_miss = !s2_hit && !mshrs.io.req.ready`, and `idx_match` asserts on
33+
**40% of requests**. Rocket's `MSHRFile` serializes misses **per set index**, ways-blind:
34+
35+
```scala
36+
// rocket-chip NBDcache.scala:452
37+
io.req.ready := Mux(!cacheable, mmio_rdy,
38+
sdq_rdy && Mux(idx_match, tag_match && sec_rdy, pri_rdy))
39+
// :410
40+
alloc_arb.io.out.ready := io.req.valid && sdq_rdy && cacheable && !idx_match
41+
```
42+
43+
Same set + same tag merges as a secondary miss; **same set + different tag cannot allocate at
44+
all**, no matter how many MSHRs are free — and `idx_match` compares *set index only*, so this is
45+
equally true at 1 way and at 16.
46+
47+
**Associativity was expected to be the fix. It is not** — measured once the bug below was
48+
repaired (64x4, same kernel and geometry otherwise):
49+
50+
| metric | 1-way baseline | 4-way (fixed) |
51+
|---|---|---|
52+
| total cycles | 579,648 | 549,479 (-5.2%) |
53+
| AXI port utilization | 28.7% | 28.6% (unchanged) |
54+
| L0d nack rate | 33% | 36.8% |
55+
| `idx_match` | 40% of reqs | 44.8% |
56+
| nacks that are `victim` | 4% | 48% |
57+
58+
Adding ways cannot relax a ways-blind `idx_match`; it merely converted `s2_nack_miss` into
59+
`s2_nack_victim` (secondary-miss-on-hit) and slightly raised the total. The two real levers,
60+
in measured order of size, are:
61+
62+
1. **Shim head-of-line blocking.** `MuonHellaCacheIFReplayQueue` refuses *all* new requests
63+
while any nacked request awaits replay:
64+
`io.req.ready := !inflight.andR && !nackq.io.deq.valid && !io.nack.valid`.
65+
At a 37% nack rate the queue is almost never empty, so the L0d shim stalls
66+
**135,072 of 141,747 offered cycles (95%), of which 98% is `nackq`**. This is now the
67+
dominant term by a wide margin.
68+
2. **Way-aware MSHR allocation.** Let a primary miss allocate against a set that already has a
69+
fill in flight whenever a free way exists, instead of refusing on set index alone.
70+
71+
Ruled out empirically as alternative fixes: more MSHRs (byte-identical cycles at 4 vs 16),
72+
`maxInFlight` = `nMSHRs` instead of `nMSHRs+1` (5.8% worse), more sets (256x1 = 0.992x — the
73+
array stride `B-A = 2^18` aliases at *every* power-of-two geometry), and de-aliasing the arrays
74+
by one line (0.924x, nacks +36% — a 64 B pad merely re-phases the collision so `B[k]` lands on
75+
`A[k+1]`'s set).
76+
77+
---
78+
79+
## 2. Reproduction
80+
81+
```bash
82+
make -C sims/vcs debug CONFIG=RadianceL0dWaysConfig # 64 sets x 4 ways x 64 B, nMSHRs=16
83+
cd sims/vcs && ./simv-chipyard.harness-RadianceL0dWaysConfig-debug +permissive +verbose \
84+
+dramsim +dramsim_ini_dir=<...>/dramsim2_ini +max-cycles=40000 \
85+
+loadmem=<kernels/vecadd/sweep_ilp2_w2.soc.elf> +permissive-off <same elf>
86+
```
87+
88+
Configs are in `chipyard/RadianceConfigs.scala`. Any `nWays > 1` on the L0d failed; every
89+
`nWays = 1` geometry ran.
90+
91+
## 3. Symptom
92+
93+
Dies at ~12.5 us with `Assertion failed: MuonHellaCacheIF exception`:
94+
95+
```
96+
[XCPT] id2_m2_b32 paddr=0x00000000 ... ae.ld=1 ...
97+
```
98+
99+
`m2_b32` is the **L0i**, not the L0d — an instruction fetch from address 0. The crash is a
100+
*consequence*: the L0d returns zeros, the core follows a null function pointer, and the L0i
101+
faults fetching the resulting wild PC.
102+
103+
## 4. Root cause
104+
105+
**A dirty line's writeback reads out as zeros, because the flush invalidates the way's
106+
metadata in the same cycle it issues the writeback, and the writeback's read data is muxed by
107+
tag-match rather than by the way it asked for.**
108+
109+
`CacheFlushUnit` couples invalidate and writeback so they fire together — for a dirty line
110+
`meta_write.valid` requires `wbAndSrcReady` and `wb_req.valid` requires `meta_write.ready`,
111+
so both handshake in the same cycle (`MuonDCache.scala:123,150`). The `WritebackUnit` then
112+
issues `data_req` alongside `meta_read` and streams `refillCycles` beats *afterwards*
113+
(`rocket-chip/NBDcache.scala:491`), by which time `coh` is already `Nothing`.
114+
115+
Those beats come back through the shared datapath:
116+
117+
```scala
118+
val en1 = s1_clk_en && s1_tag_eq_way(w) // :413 tag EQUALITY, no valid bit
119+
when (en) { regs(i) := data.io.resp(w) ... } // -> way 0's data IS latched
120+
val s2_data_muxed = Mux1H(s2_tag_match_way, s2_data) // :422 tag MATCH, ands in coh.isValid()
121+
wb.io.data_resp := s2_data_corrected // :505
122+
```
123+
124+
The flush's `meta_write` preserves the tag (`data.tag := meta.tag`) and clears only `coh`, so
125+
`s1_tag_eq_way(0)` still fires and the correct data reaches `s2_data(0)` — but
126+
`s2_tag_match_way` is **all-zero**, and `Mux1H` with a zero select emits zeros.
127+
128+
### Why only at `nWays > 1`
129+
130+
`chisel3/SeqUtils.scala:89`:
131+
132+
```scala
133+
def do_oneHotMux[T <: Data](in: Iterable[(Bool, T)]) = {
134+
if (in.tail.isEmpty) { in.head._2 } // single way -> select is DISCARDED
135+
else { ...real AND-OR one-hot mux... }
136+
}
137+
```
138+
139+
At `nWays = 1` the select is optimized away entirely and `s2_data(0)` passes through
140+
regardless, so the zero select is harmless. At `nWays >= 2` it becomes a real mux and a
141+
zero select yields zero. The defect has always been present; associativity merely made it
142+
observable.
143+
144+
### Why the 4-way L1 is unaffected
145+
146+
`canFlush = params.flushAddr.isDefined` (`TLNBDCache.scala:102`), so only the L0ds get a
147+
`CacheFlushUnit` — confirmed empirically: exactly two `[FLDONE]` lines (the two cores' L0ds),
148+
none from the L1. The L1 already runs **4-way** (`m32_w4_s512`) today because all of its
149+
writebacks are MSHR- or prober-driven, and those keep the line valid until the writeback
150+
drains. Muon's flush is the only producer that violates that invariant.
151+
152+
## 5. Evidence
153+
154+
Flush handles the `schedule_context` line (`0x10041080` → tag `0x10041`, set 2) correctly:
155+
156+
```
157+
5668 [FLST] idx=2 way_en=0x1 valid=1 dirty=1 mw_v=1 mw_r=1 wb_v=1 wb_r=1
158+
5669 [FLINV] idx=2 way_en=0x1 tag=0x10041 dirty=1
159+
5670 [FLWB] idx=2 way_en=0x1 tag=0x10041
160+
```
161+
162+
Six lines later, both halves of that 64 B line arrive at the L1 as **zeros**:
163+
164+
```
165+
5676 [WAY] m32_w4_s512 addr=0x10041080 cmd=1 ... data=0x0000...0000
166+
5678 [WAY] m32_w4_s512 addr=0x100410a0 cmd=1 ... data=0x0000...0000
167+
```
168+
169+
The stores themselves were fine — `0x10041080` misses into way 0 (`replway=0x1`), then
170+
`...84` and `...88` hit way 0 (`tagmatchway=0x1`) carrying `0x100420b8` / `0x1000104c` /
171+
`0x2`. After the zero writeback, later loads miss and refill from L1 with zeros.
172+
173+
## 6. Hypotheses that were tested and refuted
174+
175+
Recorded because each looked strong and cost a build:
176+
177+
* **`CacheFlushUnit` skipping lines.** The unit does have a real structural race — a 2-deep
178+
read pipeline behind a 1-deep holding register (`readyForMeta = anyClear || !metaValid`
179+
is evaluated at T+1 while `metaValid` is still low, so a second read is issued before the
180+
first line is examined; the flush sits lowest on both `metaReadArb`/`metaWriteArb`).
181+
Instrumented as `metaValid && metaReadFired && !anyClear`: **`skip=0`**. It never fires,
182+
because 254 of 256 lines are invalid at flush time and `clearInvalid` supplies `anyClear`.
183+
Measured `read=256 inv=2 wb=2` — properly paired.
184+
* **A line duplicated across ways.** `PopCount(s1_tag_match_way) > 1` instrumented as
185+
`[DUPWAY]`: **0 occurrences**. No tag is ever valid in two ways.
186+
* **`replacer.way` s1/s2 skew**`s2_replaced_way_en` and the meta `RegEnable` use the same
187+
s1 way; self-consistent.
188+
* **Fork divergence from upstream.** Correct, and the reason the earlier diff found nothing:
189+
`NBDcache.scala:896,978` are *identical* to the fork. The bug is in how Muon's flush
190+
**uses** the datapath, not in the datapath's divergence from it.
191+
* **"count and pray" writeback source IDs** — bounded by `wbStall := inFlights >= (1 << srcWidth)`;
192+
no ID collision.
193+
* **`metaArb.io.in(5).bits.way_en := metaArb.io.in(4).bits.way_en`** — looks like a copy-paste
194+
way bug but is inside a `/* */` block (dead, like the `maskMshrs` region).
195+
196+
## 7. Fix
197+
198+
`MuonDCache.scala`, at the data mux — select the way the read asked for when the access is a
199+
writeback:
200+
201+
```scala
202+
val s1_wb_way_en = Reg(UInt(nWays.W))
203+
when (wb.io.data_req.valid) { s1_wb_way_en := wb.io.data_req.bits.way_en }
204+
val s2_wb_way_en = RegEnable(s1_wb_way_en, s1_clk_en)
205+
val s2_writeback = RegEnable(s1_writeback, s1_clk_en)
206+
val s2_data_muxed = Mux1H(Mux(s2_writeback, s2_wb_way_en, s2_tag_match_way), s2_data)
207+
```
208+
209+
For a still-valid line the two selects are identical, so this is a no-op everywhere the cache
210+
works today and repairs only the invalidated-then-written-back case.
211+
212+
Rejected alternative: gate the flush's `meta_write` on `wb_resp_fire` so the line stays valid
213+
until its release is acked. That restores upstream's invariant and is contained to Muon code,
214+
but it restructures the flush handshake (currently deliberately coupled) and serializes it to
215+
one outstanding writeback. Worth revisiting if the datapath fix proves insufficient.
216+
217+
**Residual race, not addressed:** between the flush's invalidate and the writeback's data
218+
beats, a new miss to that set could allocate the same way and overwrite its tag, which would
219+
stop `s1_tag_eq_way` from latching the line. The window is a few cycles; the `wb_resp_fire`
220+
alternative above would close it.
221+
222+
## 8. Instrumentation used (uncommitted at time of writing)
223+
224+
* `radiance/memory/TLCounterNode.scala` — pass-through TL counters (throughput / back-pressure /
225+
occupancy), inserted at L0d in/out, L1 in/out, and both sides of the non-coal shrinker.
226+
* `radiance/memory/MuonHellaCacheIF.scala``[NBDSTALL]` shim stall attribution, `[XCPT]` dump.
227+
* `radiance/memory/MuonDCache.scala``[S2NACK]` nack-source counters, `[WAY]`/`[WAYWR]` probes,
228+
`[FLRD]`/`[FLST]`/`[FLINV]`/`[FLWB]`/`[FLSKIP]`/`[FLDONE]` flush probes, `[DUPWAY]` check.
229+
* `testchipip/csrc/mm_dramsim2.cc``[DRAMSTAT]` DRAM transaction/back-pressure counters.
230+
231+
All printf-based counters require `+verbose` (`PRINTF_COND`); **rtlq does not pass it**, so
232+
these runs must invoke `simv` directly. Note `[WAYWR]` produced no output in the failing run
233+
and may be mis-qualified — it was not needed for the diagnosis.

src/main/scala/radiance/memory/MuonDCache.scala

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,18 @@ class MuonNonBlockingDCacheModule(outer: MuonNonBlockingDCache) extends HellaCac
365365
}
366366
s2_data(w) := regs.asUInt
367367
}
368-
val s2_data_muxed = Mux1H(s2_tag_match_way, s2_data)
368+
// A writeback reads one specific way, which CacheFlushUnit may already have
369+
// invalidated in the same cycle it issued the request. The flush preserves the
370+
// tag and clears only coh, so s1_tag_eq_way still latches the line into s2_data,
371+
// but s2_tag_match_way (which ands in coh.isValid) is all-zero and would mux out
372+
// zeros. Select by the way the read actually asked for instead. At nWays == 1
373+
// Mux1H discards its select entirely, which is why this only ever bit multi-way
374+
// configs; for a still-valid line the two selects are identical.
375+
val s1_wb_way_en = Reg(UInt(nWays.W))
376+
when (wb.io.data_req.valid) { s1_wb_way_en := wb.io.data_req.bits.way_en }
377+
val s2_wb_way_en = RegEnable(s1_wb_way_en, s1_clk_en)
378+
val s2_writeback = RegEnable(s1_writeback, s1_clk_en)
379+
val s2_data_muxed = Mux1H(Mux(s2_writeback, s2_wb_way_en, s2_tag_match_way), s2_data)
369380
val s2_data_decoded = (0 until rowWords).map(i => dECC.decode(s2_data_muxed(encDataBits*(i+1)-1,encDataBits*i)))
370381
val s2_data_corrected = s2_data_decoded.map(_.corrected).asUInt
371382
val s2_data_uncorrected = s2_data_decoded.map(_.uncorrected).asUInt

0 commit comments

Comments
 (0)