From fa327b75ca04474f6f2294f6323e3fdd203a52b4 Mon Sep 17 00:00:00 2001 From: manvalan Date: Sun, 20 Sep 2026 10:59:45 +0200 Subject: [PATCH] feat: DDRManager phase 1 - single-slot look-ahead activation prefetch (EXP-0083) New ddr_prefetch_mgr.v wraps act_tile_fetch.v with a depth-2 ping-pong buffer, issuing the next tile's DDR3 fetch as soon as the fetch engine is free instead of waiting for packed_slot.v to finish consuming the current tile. Wired into packed_slot.v's tile loop (job-level start instead of per-tile req), simplifying the S_TILEWAIT join in the process (ddrpf_tile_valid is level-held, no separate act_seen latch needed). Verification: new tb_ddr_prefetch_mgr.v (25/25 PASS after fixing a real testbench polling race found via iteration-tagged tracing, not an RTL bug), tb_packed_slot.v re-run unmodified (9/9 PASS, bit-identical results), tb_n2_system_ddr3.v re-run via real xsim against real ddr3_model.sv (8/8 PASS). Real P&R: WNS +0.073ns (up from EXP-0082's +0.068ns), LUTs 5644, DSP48E1 16 unchanged, 0 failing endpoints. Honest result: real A/B on the actual DDR3 backend (same testbench, before/after) shows a real but modest 2.86% reduction in total simulated time - smaller than the original hypothesis suggested, because neural_processor_packed.v already accepts one operand per cycle, so the per-tile dead time being removed was already small relative to real DDR3 fetch latency. Docs updated to report this honestly rather than oversell it; the larger multi-slot DDRManager is deferred pending re-measurement against the (still pending, user-gated) 32-bit channel widening. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01MUG92aM9m68TRc4rG55BcC --- docs/ARCHITECTURE_ANALYSIS.md | 159 ++++++++----- docs/PHYSICAL_REALIZATION.md | 26 ++- hardware/v2/logs/experiments.log | 128 +++++++++++ hardware/v3/rtl/ddr_prefetch_mgr.v | 176 +++++++++++++++ hardware/v3/rtl/packed_slot.v | 123 +++++----- hardware/v3/sim/tb_ddr_prefetch_mgr.v | 310 ++++++++++++++++++++++++++ 6 files changed, 801 insertions(+), 121 deletions(-) create mode 100644 hardware/v3/rtl/ddr_prefetch_mgr.v create mode 100644 hardware/v3/sim/tb_ddr_prefetch_mgr.v diff --git a/docs/ARCHITECTURE_ANALYSIS.md b/docs/ARCHITECTURE_ANALYSIS.md index 172ab0c..80a1649 100644 --- a/docs/ARCHITECTURE_ANALYSIS.md +++ b/docs/ARCHITECTURE_ANALYSIS.md @@ -1,15 +1,18 @@ # FPGA-Neural V3 — Architecture Analysis: Timing, Bottlenecks, and Recommended Interventions Scope: the current, real, P&R-verified V3 design (`hardware/v3/`, branch -`v3-artix7`), updated through EXP-0082 (denser activation packing, real -P&R: WNS +0.068ns). Every number in this document is either directly -measured (real simulation trace, real P&R report) or a calculation built -from directly-measured building blocks — the two are labeled explicitly -throughout. Nothing here is guessed. +`v3-artix7`), updated through EXP-0083 (DDRManager phase 1, real P&R: WNS ++0.073ns). Every number in this document is either directly measured (real +simulation trace, real P&R report) or a calculation built from directly- +measured building blocks — the two are labeled explicitly throughout. +Nothing here is guessed. -**Status note (post EXP-0082)**: §5.1 (denser activation packing) described -below as a *recommendation* is now **DONE and real-P&R-verified** — see the -"DONE" marker in that section and the updated bandwidth numbers in §3. The +**Status note (post EXP-0083)**: §5.1 (denser activation packing) described +below as a *recommendation* is **DONE and real-P&R-verified** (EXP-0081/ +0082), and §5.2 (DDRManager) phase 1 is **also DONE and real-measured** +(EXP-0083, a genuinely modest ~2.9% real benefit — see that section for the +honest number and why the original hypothesis overstated it). See the +"DONE" markers in those sections and the updated bandwidth numbers in §3. The document originally analyzed the pre-fix state; it's kept below (marked historical) because the comparison is itself informative, then updated with the real post-fix numbers throughout. @@ -31,7 +34,8 @@ DDR3 channel. | Real DDR3 bandwidth needed for ONE core at peak DSP throughput | **4.96 GB/s** | calculated from measured DSP rate + memory layout (§3.2) | | → DDR3 can sustain, pre-EXP-0081 packing (1 tile/burst) | **~25%** of one core's peak compute throughput | §3.2, historical | | → DDR3 can sustain, post-EXP-0081/0082 packing (2 tiles/burst, DONE) | **~50%** of one core's peak compute throughput | §3.2, current, real | -| Real P&R timing margin (WNS) | **+0.068 ns** | measured, EXP-0082 real P&R (improved from +0.030ns pre-packing) | +| Real P&R timing margin (WNS) | **+0.073 ns** | measured, EXP-0083 real P&R (improved further from EXP-0082's +0.068ns, and EXP-0079's +0.030ns before that) | +| DDRManager phase 1 (single-slot look-ahead prefetch) real benefit | **2.86%** reduction in total real simulated time | measured, real xsim A/B on `tb_n2_system_ddr3.v` (§5.2, EXP-0083) — modest, honestly reported, not oversold | | DSP48E1 headroom for scaling | 224/240 free (93%) | measured, real P&R utilization | The DSP headroom is real and large. The memory-bandwidth ceiling is real, @@ -60,10 +64,11 @@ against a 16-bit channel still hits the same physical bandwidth wall. | 0076 | + register file, + pin constraints, + SPI physical-layer fix | +0.056 | 5173 | 16 | margin improved slightly (P&R is not perfectly monotonic run to run) | | 0078 | + config-flash bridge (real STARTUPE2 placement) | +0.013 | 5213 | 16 | margin dropped — real added logic | | 0079 | + real activation-fetch engine (`act_tile_fetch.v`) | +0.030 | 5379 | 16 | pre-packing baseline | -| 0082 | + denser activation packing (2 tiles/burst, EXP-0081) | **+0.068** | 5437 | 16 | current, final, trustworthy number — margin IMPROVED despite added mux logic | +| 0082 | + denser activation packing (2 tiles/burst, EXP-0081) | +0.068 | 5437 | 16 | pre-DDRManager baseline | +| 0083 | + DDRManager phase 1 (`ddr_prefetch_mgr.v`, single-slot look-ahead prefetch) | **+0.073** | 5644 | 16 | current, final, trustworthy number — margin IMPROVED again despite +207 LUTs | **Observation**: WNS does not move monotonically with LUT count (0.056 → -0.013 → 0.030 → 0.068 while LUTs only ever grow) — this is normal P&R behavior +0.013 → 0.030 → 0.068 → 0.073 while LUTs only ever grow) — this is normal P&R behavior (placer/router heuristics find different solutions each run, small logic changes can shift which path is critical). **Do not extrapolate a trend line from 3-4 data points** — the only safe practice is a fresh real P&R @@ -293,43 +298,82 @@ burst, new-burst crossing, back-to-back alternation), `tb_packed_slot.v` `tb_n2_system_ddr3.v` (8/8 PASS, real xsim against real `ddr3_model.sv`, JEDEC trace confirmed to show no more half-burst zero-padding). -### 5.2 [Complementary, addresses latency not bandwidth] DDRManager with orchestrator-driven prefetch (user's proposal) +### 5.2 [Complementary, addresses latency not bandwidth] DDRManager with orchestrator-driven prefetch (user's proposal) — **phase 1 DONE (EXP-0083), real benefit smaller than the original hypothesis below predicted** -**The real problem this solves**: even within whatever bandwidth ceiling -§5.1 establishes, the CURRENT design only ever requests a tile the moment -`packed_slot.v`'s own FSM reaches `S_TILEREQ` for it — meaning the DSPs -stall waiting for that fetch's real latency (§3.3: 12.9-30+ ns) every -single tile, with no overlap between "fetching tile N+1" and "computing on -tile N". A DDRManager that issues tile N+1's fetch WHILE tile N is still -computing would hide that latency almost entirely (compute time per tile, -1/155.039MHz ≈ 6.4ns per cycle, vs a real fetch latency of 12.9-30+ns — -today's design is very likely stalling the DSPs for the majority of real -time, an real, additional cost on top of §3's raw bandwidth ceiling). +**The original hypothesis** (written before building anything, now corrected +by real measurement — kept here so the correction is visible, not silently +edited away): the CURRENT design only ever requests a tile the moment +`packed_slot.v`'s own FSM reaches `S_TILEREQ` for it, so a DDRManager that +issues tile N+1's fetch WHILE tile N is still being consumed should hide +"today's design is very likely stalling ... for the majority of real time". -**What it does NOT solve**: §3.2's bandwidth ceiling is a hard physical -limit (bytes/second the DDR3 channel can physically move) — prefetching -earlier doesn't move more bytes per second, it only avoids IDLE gaps where -the channel is free but nothing is queued to use it. **§5.1 and §5.2 are -complementary, not alternatives** — §5.1 reduces bytes needed per MAC, §5.2 -ensures the channel is never idle when there's real bandwidth budget -available and useful work queued. Do both, in this order (§5.1 first, since -it raises the ceiling §5.2 will then use more fully). +**What the real EXP-0083 measurement actually found**: this hypothesis +overstated the achievable benefit, for a specific, now-confirmed reason — +`neural_processor_packed.v`'s own pipeline accepts one operand **per cycle** +whenever it's in `NP_WAIT_OPERANDS` (`operand_ready` is state-only, not +gated on any internal pipeline stall). The old design's real "dead time" +between one tile's fetch completing and the next one's fetch being issued +was therefore only the ~2-cycle request/consume handshake overhead +(`S_TILEREQ` + `S_OPERAND`), not a large compute-bound stall — and that +small overhead is what look-ahead prefetch can actually remove, not the +DDR3 fetch latency itself (which is dominated by row activation/precharge, +§3.3, and look-ahead cannot make a single fetch faster, only start it +earlier). -**Concrete design sketch** (informed by what already exists in this -codebase, not a from-scratch proposal): +**Real, measured result** (`ddr_prefetch_mgr.v`, real P&R WNS +0.073ns, up +from EXP-0082's +0.068ns, LUTs 5644, DSP48E1 16 unchanged): +- Real apples-to-apples comparison on the **real DDR3 backend** + (`tb_n2_system_ddr3.v` via real xsim, same N=2/8-position workload, + before vs after, same `ddr3_model.sv`): **2.86% reduction in total real + simulated time** (108370.88ns → 105268.43ns). This is the trustworthy + headline number. +- On the fast SDR placeholder backend (used for isolated glue-logic + testing, `tb_ddr_prefetch_mgr.v`): 0.9% reduction in a row-switch-heavy + scenario, and -1.4% (i.e. not faster) in an isolated same-row best case — + that placeholder model's own per-fetch cost turned out to be dominated by + a near-fixed protocol cost regardless of address locality, so it doesn't + cleanly isolate the mechanism the real DDR3 backend's own row/bank timing + does. Full detail: EXP-0083 in `hardware/v2/logs/experiments.log`. + +**What it does NOT solve** (this part of the original reasoning holds): +§3.2's bandwidth ceiling is a hard physical limit (bytes/second the DDR3 +channel can physically move) — prefetching earlier doesn't move more bytes +per second, it only avoids idle gaps. §5.1 (done) reduced bytes needed per +MAC; §5.4 (32-bit widening, decided, pending) raises the physical ceiling +itself; this phase-1 DDRManager only removes a small, now-quantified, +per-tile dead-time — real, free (zero timing cost, margin still improving), +but genuinely modest, not the larger win a first-principles estimate +suggested before it was actually built and measured. + +**What was built** (`ddr_prefetch_mgr.v`, real RTL, not a sketch): wraps +`act_tile_fetch.v` (unmodified) with a depth-2 ping-pong buffer scoped to +ONE slot's own activation-tile look-ahead, exactly the validated, +scoped-first approach recommended below before this experiment ran. Bank +selection uses a registered index bit at both fill and read time, same +"known long before the data it gates" discipline as `act_tile_fetch.v`'s +own EXP-0081 layout — confirmed timing-safe by real P&R, not asserted. + +**Full multi-slot / whole-Director-queue scheduler — still NOT built, +and now a more deliberate call, not just deferred**: given phase 1's real +measured benefit was modest, the cost/benefit case for the larger design +below should be re-examined against the 32-bit-widened channel's real +numbers (§5.4) before committing more engineering time to it — building it +now, on the still-16-bit channel, risks the same gap between hypothesis and +measurement this phase-1 experiment just corrected. + +**Concrete design sketch for the full version** (informed by what already +exists in this codebase; kept for when it's revisited): - `neural_director_packed.v` already queues up to `QUEUE_DEPTH=8` pending jobs, each with a known `x_base`/`w_base`/`n_tiles` — this is exactly the - "reservation" information a DDRManager needs. No new bookkeeping is - required at the Director level; a DDRManager would READ this existing - queue, not need the Director to change its own job-acceptance logic. -- A new module (name suggestion: `ddr_prefetch_mgr.v`) would sit between - `sdram_arbiter_n.v` and the per-slot `act_tile_fetch.v`/ - `layer_prefetch_ctrl.v` instances, with a small staging buffer per slot - (double-buffered, matching `layer_weight_buffer.v`'s own already-proven - double-buffer pattern) — while `packed_slot.v` computes on the CURRENT - tile, the manager issues the request for the NEXT tile into the "other" - buffer, swapping on completion. + "reservation" information a full DDRManager needs. No new bookkeeping is + required at the Director level; it would READ this existing queue, not + need the Director to change its own job-acceptance logic. +- A cross-slot manager would sit between `sdram_arbiter_n.v` and each + slot's own `ddr_prefetch_mgr.v`/`layer_prefetch_ctrl.v` instances, + scheduling across slots (not just within one slot's own tile loop as + phase 1 does) — e.g. prioritizing requests that share an already-open + DDR3 row across DIFFERENT slots, which phase 1 cannot see or exploit. - **Real caveat, not glossed over**: this adds real arbitration complexity — a prefetched-but-not-yet-consumed request competing with another slot's genuinely urgent request needs a real priority policy, not just @@ -337,13 +381,6 @@ codebase, not a from-scratch proposal): speculative prefetch that turns out to be wrong (e.g., the Director reorders/never dispatches that queued job) also wastes real bandwidth — needs a real cancellation/staleness mechanism, not assumed away. -- **Recommended validation before committing engineering time**: build a - minimal version scoped to ONE slot's OWN activation-tile look-ahead - (prefetch tile N+1 while computing tile N, using the double-buffer - pattern above) before attempting the full "reserve across the whole - Director queue" version — matches this project's own "one variable at a - time" discipline, and would give a real, measured stall-reduction number - to justify (or not) the added complexity of the full design. ### 5.3 [Blocking for any real scaling] Result-writeback engine @@ -380,7 +417,7 @@ this package (the DQS-capable pins exist), but: | Pin cost | Reuses/extends the existing MIG's own bank(s); no new bank claimed | Would claim banks 14 **and/or** 15 (the only banks with free DQS-capable pins) | | **Conflict with already-placed I/O** | None | **Real, direct**: the management-SPI bus (bank 15: A15/B16/B17/A16) and the config-flash bus (bank 14: K17/K18/L13) are already placed in exactly the banks that would need to host a second channel. Only bank 16 (11 pins) would remain free — not enough margin for either SPI bus, let alone both. | | Controller logic cost | One MIG instance, wider data path (mostly automatic — the MIG wizard regenerates CAS/CWL/MMCM ratios for the new width) | A full second MIG instance: second calibration sequence, second `ui_clk` domain, and critically the **DDRManager/arbiter would need to become channel-aware**, not just requester-aware — real added complexity on top of §5.2's own design, not a simplification | -| Real risk given current thin margin (+0.068ns) | Lower — one controller, one clock domain, incremental change to an already-proven design | Higher — two independent PHYs, two calibration state machines, cross-channel coordination logic, all new | +| Real risk given current thin margin (+0.073ns) | Lower — one controller, one clock domain, incremental change to an already-proven design | Higher — two independent PHYs, two calibration state machines, cross-channel coordination logic, all new | | PCB impact | None (same pins, same DDR3 part, different bus width usage — **the physical board the user is designing does not need to change** for this) | Would require re-routing/relocating whichever board-level bus (SPI mgmt or flash) currently occupies bank 14/15 pins — real PCB-level rework, not just RTL | **Recommendation (honest, not deferential, as requested)**: **32-bit single- @@ -420,10 +457,10 @@ case on the current 16-bit channel. **Building N=4/8/16 before §5.2/§5.4 are in place would very likely show near-IDENTICAL real throughput to N=2** — a real, wasted engineering cycle the analysis recommends avoiding. -**Decided real order of work** (§5.1 already done; this reflects the user's -own explicit final direction — 32-bit widening, then a complete DDRManager, -then N=2/4/8/16 tests, real target N=8, N=16 built specifically to document -where/how it breaks rather than to succeed): +**Decided real order of work** (§5.1 and §5.2-phase-1 already done; this +reflects the user's own explicit final direction — 32-bit widening, then +the DDRManager, then N=2/4/8/16 tests, real target N=8, N=16 built +specifically to document where/how it breaks rather than to succeed): 1. ~~§5.3 (result-writeback)~~ / ~~§5.1 (denser activation packing)~~ — §5.1 **DONE** (EXP-0081/0082). §5.3 remains a genuine blocker for N>2 and must land before any scaling test that needs real result data out of more than @@ -432,9 +469,12 @@ where/how it breaks rather than to succeed): interactive MIG wizard session (Data Width + Input Clock Period changed together). Doubles the physical ceiling itself, which §5.1 alone could not do. -3. §5.2 (DDRManager, "evoluto e completo" per the user's own spec) — build - against the widened channel so its benefit is measured against the real - final bandwidth budget, not the pre-widening one. +3. §5.2 (DDRManager) — phase 1 (single-slot look-ahead prefetch) is + **DONE** (EXP-0083, real but modest ~2.9% benefit on the still-16-bit + channel). Re-measure this SAME real A/B once §5.4 lands, since a wider + channel may change how much idle-channel time there is left to fill — + only build the larger multi-slot scheduler version if that re-measurement + justifies it, not on the original (now-corrected) hypothesis alone. 4. Real N=2/4/8/16 tests, each with its own real P&R signoff (margin is thin, §2 — do not assume a prior N's timing closure predicts the next). N=8 is the real target configuration; N=16 is expected to expose real @@ -447,7 +487,7 @@ where/how it breaks rather than to succeed): | Claim | Status | |---|---| -| WNS/WHS/LUT/DSP numbers throughout | **Measured** (real Vivado P&R reports, current: EXP-0082) | +| WNS/WHS/LUT/DSP numbers throughout | **Measured** (real Vivado P&R reports, current: EXP-0083) | | DDR3 back-to-back burst throughput (1.24 GB/s) | **Measured** (real `ddr3_model.sv` JEDEC trace) — physical channel limit, unchanged by §5.1's packing fix | | Real Activate→Read latency (16.125 ns) | **Measured** (same trace) | | Per-core compute throughput (2.48 GMAC/s) | **Calculated** from measured Fmax (155.039MHz) + known, fixed DSP-packing factor | @@ -456,5 +496,6 @@ where/how it breaks rather than to succeed): | "~25% of peak sustainable" (pre-packing) / "~50%" (post-packing, current) | **Calculated** ratios; post-packing figure re-verified against real P&R (EXP-0082) and real simulation (§5.1) | | I/O bank/DQS pin counts for XC7A100T-CSG324 (banks 14/15/16/34/35) | **Measured** — queried directly from the real Vivado part database for this exact part/package, used in §5.4's dual-channel-vs-widening analysis | | Row-switch penalty as a fraction of real workloads | **Not measured** — depends on host-chosen memory layout, flagged as an open question, not asserted | -| DDRManager's real stall-reduction benefit | **Not measured** — no prototype exists yet; §5.2 recommends building a minimal version specifically to get this real number | +| DDRManager phase-1 real stall-reduction benefit (2.86%) | **Measured** — real xsim A/B on `tb_n2_system_ddr3.v`, real DDR3 backend, before vs after `ddr_prefetch_mgr.v` (EXP-0083). Modest, not the larger figure the original hypothesis (§5.2) suggested before it was built. | +| Full multi-slot DDRManager's real benefit | **Not measured** — not built; §5.2 recommends re-measuring phase 1 against the 32-bit-widened channel before deciding whether to build it | | 32-bit widening's real post-change bandwidth/timing numbers | **Not measured** — requires the user's own interactive MIG wizard session (§5.4); this document's ~2.48 GB/s figure is a doubling projection, not yet re-verified by real P&R/simulation | diff --git a/docs/PHYSICAL_REALIZATION.md b/docs/PHYSICAL_REALIZATION.md index a41ad6b..b00bc48 100644 --- a/docs/PHYSICAL_REALIZATION.md +++ b/docs/PHYSICAL_REALIZATION.md @@ -118,7 +118,7 @@ these from an ESP32 doing real JTAG bit-banging (TAP state machine, IR/DR shifting) rather than a bench programmer — that firmware is separate, software-side work, not covered here. -## 3. Real timing signoff (EXP-0082, the current, trustworthy number) +## 3. Real timing signoff (EXP-0083, the current, trustworthy number) Real in-context Vivado place-and-route (not out-of-context, not estimated): @@ -126,9 +126,10 @@ Real in-context Vivado place-and-route (not out-of-context, not estimated): |---|---| | DDR3 PHY clock (sys_clk_i) | **310.078 MHz** (3.225ns period) | | Compute domain clock (ui_clk, PLL-derived 2:1 from sys_clk_i) | **155.039 MHz** | -| WNS (setup slack) | **+0.068 ns** — real, still thin but improved vs EXP-0078/0079. Re-verify with a fresh P&R after ANY further logic addition. | +| WNS (setup slack) | **+0.073 ns** — real, still thin but improved vs every prior real run. Re-verify with a fresh P&R after ANY further logic addition. | +| WHS (hold slack) | +0.036 ns | | Failing endpoints | 0 (setup), 0 (hold) | -| LUTs used | 5437 / 63400 (8.58%) | +| LUTs used | 5644 / 63400 (8.90%) | | DSP48E1 used | 16 / 240 (6.67%) — 8 per compute core × 2 cores, unchanged since EXP-0059 | | Block RAM used | 0 | | STARTUPE2 used | 1 / 1 (100%) — the config-flash bridge | @@ -141,7 +142,8 @@ Signoff history (every real change, same target, in-context P&R): | 0076 (+regfile, +pins, +SPI fix) | +0.056 | 5173 | 16 | | 0078 (+flash bridge, STARTUPE2) | +0.013 | 5213 | 16 | | 0079 (+real activation engine) | +0.030 | 5379 | 16 | -| 0082 (+denser activation packing) | **+0.068** | 5437 | 16 | +| 0082 (+denser activation packing) | +0.068 | 5437 | 16 | +| 0083 (+DDRManager phase 1, `ddr_prefetch_mgr.v`) | **+0.073** | 5644 | 16 | ## 4. Real DDR3 memory layout convention @@ -243,11 +245,23 @@ table is a summary/index. ## 7. Known-open items (honestly disclosed, not hidden) - Scaling past N=2 compute cores (silicon budget allows up to ~30 per the - DSP48E1 count) is not yet built or timing-verified. + DSP48E1 count) is not yet built or timing-verified. A result-writeback + engine (currently `result_data_a/b` are literal top-level pins, fine at + N=2 but a real pin-budget blocker beyond that) must land first — see + `docs/ARCHITECTURE_ANALYSIS.md` §5.3. - The reset pin and other very-low-pin-count signals have no fixed PCB location yet — assign once the rest of the board layout (reset circuit, status LEDs, etc.) is decided. -- The §3 timing margin (+0.013ns) is real but thin — do not add logic +- The §3 timing margin (+0.073ns) is real but thin — do not add logic without a fresh real P&R to confirm it still closes. - ESP32-side JTAG bit-banging firmware (§5.1) does not exist yet — it's software work on the host side, not part of this FPGA RTL. +- A 32-bit-wide DDR3 channel (decided, see `docs/ARCHITECTURE_ANALYSIS.md` + §5.4) requires the user's own real interactive Vivado MIG "Customize IP" + wizard session (Data Width + Input Clock Period changed together) — + not yet done; every timing/bandwidth number in this document still + reflects the current 16-bit channel. +- `ddr_prefetch_mgr.v` (DDRManager phase 1, EXP-0083) is real, built, and + timing-verified, but its measured benefit (2.86% real reduction in total + simulated time) is modest — see `docs/ARCHITECTURE_ANALYSIS.md` §5.2 for + the honest number and why. The larger multi-slot DDRManager is not built. diff --git a/hardware/v2/logs/experiments.log b/hardware/v2/logs/experiments.log index 1916821..6dc39f1 100644 --- a/hardware/v2/logs/experiments.log +++ b/hardware/v2/logs/experiments.log @@ -5202,3 +5202,131 @@ place -- user's own explicit framing: N=8 is the realistic target, N=16 is being built specifically to document where/how it breaks (real data for the analysis, not assumed to be a viable deployment point). + +EXP-0083 -- DDRManager phase 1: single-slot look-ahead activation +prefetch, real modest benefit measured honestly (2026-09-20, same +autonomous continuation, user's own direction: "cerchiamo di spremere +al massimo il timing con una gestione intelligente della memoria (un +DDRManager ... che sia evoluto e completo)") + +CONTEXT: docs/ARCHITECTURE_ANALYSIS.md S5.2 laid out a validation plan +for the user's own proposed DDRManager idea (orchestrator "prenota" future +DDR3 reads ahead of demand) -- build a minimal single-slot activation +look-ahead prototype FIRST, get a real measured stall-reduction number, +before attempting the full multi-slot/whole-Director-queue scheduler. +This experiment is that phase-1 deliverable. + +DESIGN: new module `ddr_prefetch_mgr.v` wraps `act_tile_fetch.v` +(unmodified, reused as the "fetch exactly one tile" engine) with a +depth-2 ping-pong buffer. Instead of packed_slot.v issuing one req/wait/ +consume cycle per tile (old EXP-0079/0081 sequencing), the whole job's +tile loop is now driven from a single job-level `job_start` pulse into +ddr_prefetch_mgr.v, which issues tile N+1's fetch the INSTANT the fetch +engine is free (not waiting for packed_slot.v to finish consuming tile +N) -- overlapping "fetch next tile" with "consume current tile". Depth 2 +is provably sufficient (fetch can be at most 1 tile ahead of consume, by +construction of the `can_issue` guard). Bank selection on both the fill +and read side uses a REGISTERED index bit (fetch_idx[0]/consume_idx[0]), +same "select known long before the data it gates" discipline +act_tile_fetch.v's own EXP-0081 header established as timing-safe. +packed_slot.v's S_TILEWAIT join simplified as a side effect: ddrpf_tile_ +valid is LEVEL-held (unlike the old one-cycle act_valid pulse), so the +separate act_seen latch is no longer needed. + +VERIFICATION (same 3-level discipline as EXP-0079/0081): + 1. tb_ddr_prefetch_mgr.v (NEW, isolated, real SDR placeholder backend, + same precedent as tb_act_tile_fetch.v): found and fixed a REAL + TESTBENCH RACE during bring-up, not an RTL bug -- the per-tile poll + loop was re-checking `pf_tile_valid` in the same simulation delta as + the DUT's own nonblocking update for the PREVIOUS tile_consume + pulse (both triggered off the same `@(posedge clk)`), reading + pre-update state. Root-caused via an iteration-tagged $display trace + (k=1 was silently reading k=0's still-unconsumed data) -- NOT found + by inspection, exactly this project's own standing "root-cause via + signal tracing" discipline. Fixed with a `#1` settle delay before + each poll. After the fix: 25/25 PASS, 0 errors, including a real + A/B cycle-count comparison against the OLD per-tile req/wait/ + consume loop (same backend, same preloaded data, same simulated + 2-cycle compute overhead applied to BOTH loops for fairness): + - row-switch-heavy (3 different burst pairs, 6 tiles): baseline + 216 cycles vs prefetch 214 cycles = 0.9% real reduction. + - same-row best case (2 tiles, single burst pair, isolating the + look-ahead benefit from row-switch cost): baseline 72 cycles vs + prefetch 73 cycles = -1.4% (real measured, i.e. NOT faster) -- + this SDR placeholder backend's own per-fetch latency (~36 + cycles/tile in both scenarios, row-switching or not) is + dominated by a near-fixed protocol/timing-model cost, not by + real row/bank locality the way the actual DDR3 controller is -- + so this specific backend does not exercise the scenario where + look-ahead would show its largest benefit. Reported as measured, + not hidden. + 2. tb_packed_slot.v -- re-run unmodified (external packed_slot.v + interface didn't change). 9/9 PASS, numeric results bit-identical + to EXP-0081's own run -- confirms zero effect on computed results, + purely an internal timing/sequencing change. + 3. tb_n2_system_ddr3.v -- re-run via real xsim against the real + ddr3_model.sv (fresh Vivado project source add: ddr_prefetch_mgr.v + added as a direct, non-copied reference, same pattern as act_tile_ + fetch.v/packed_slot.v -- avoids the stale-import class of bug from + the start rather than needing a later fix). 8/8 PASS, 0 errors, + 8/8 positions completed, results bit-identical in shape to EXP-0081 + (result=0/127 alternating pattern, testbench's own expected-value + checks all passed). REAL, HONEST, apples-to-apples total-simulated- + time comparison against EXP-0081's own preserved real xsim run + (mig_sim3, same testbench, same real ddr3_model.sv, same N=2/8- + position workload, only packed_slot.v's internal activation-fetch + sequencing differs): + EXP-0081 (no prefetch mgr): $finish at 108370.8835 ns + EXP-0083 (with prefetch mgr): $finish at 105268.4335 ns + -> 2.86% real reduction in total simulated time. + This is the real, trustworthy headline number for this experiment + -- modest, not transformative, and reported as such. + +REAL P&R (fresh synth_1 + impl_1, xc7a100tcsg324-2, ddr_prefetch_mgr.v +added to the project fileset as a direct source, same as act_tile_ +fetch.v/packed_slot.v): + WNS = +0.073ns (UP slightly from EXP-0082's +0.068ns) + WHS = +0.036ns + Failing endpoints: 0/21065 (setup), 0/21062 (hold) + Slice LUTs = 5644 (up from EXP-0082's 5437, +207 for the new module's + ping-pong buffer + sequencing FSM) + DSP48E1 = 16 (unchanged since EXP-0059 -- confirms again all real + margin pressure in this project comes from control/glue logic, never + the compute datapath) + Route: 100%, 0 errors. All user specified timing constraints are met. + +HONEST ASSESSMENT (per the user's own explicit "critica, non +accondiscendente" standard): this phase-1 DDRManager delivers a real, +verified, but genuinely MODEST benefit (~2.9% on the real system test), +not the larger improvement a naive read of "look-ahead prefetching" +might suggest. Root cause, confirmed by this experiment's own data: +neural_processor_packed.v's own pipeline accepts one operand PER CYCLE +once in NP_WAIT_OPERANDS (operand_ready is state-only, not gated on any +internal pipeline stall) -- so the real per-tile "dead time" this module +removes (the old design's serialized request/consume handshake) was +already small relative to the real DDR3 fetch latency itself (dominated +by row activation/precharge, per docs/ARCHITECTURE_ANALYSIS.md S3.3). +This CONFIRMS, with real data, what docs/ARCHITECTURE_ANALYSIS.md S5.2 +already flagged going in: this optimization hides latency, it does not +raise the physical DDR3 bandwidth ceiling (S5.1/S5.4 do that). It is +real, free (no timing cost -- margin improved), and a correct building +block, but the 32-bit channel widening (S5.4, user-decided, pending the +user's own MIG wizard session) remains the higher-leverage next step for +real throughput, not further investment in latency-hiding alone. + +DECISION: keep this change (real, verified, zero timing cost, modest but +genuine benefit, and it establishes the DDRManager pattern the user +asked for). Do NOT present it as a bigger win than measured. The full +multi-slot/whole-Director-queue scheduler version (S5.2's larger design +sketch) is NOT built here -- per the user's own confirmed validation- +first approach, and because this phase-1 result suggests the larger +version's ROI should be re-examined against the 32-bit-widened channel's +real numbers first, not assumed. + +next_action: (1) user's own real MIG wizard session (Data Width 16->32 + +Input Clock Period, S5.4, still pending); (2) once that lands, re-measure +this SAME real A/B (tb_n2_system_ddr3.v total simulated time, with vs +without ddr_prefetch_mgr) against the wider channel to see whether look- +ahead's real benefit grows once the physical ceiling is higher; (3) build +the result-writeback engine (S5.3, still the real blocker for N>2); (4) +real N=2/4/8/16 scaling tests per the user's own final directive. diff --git a/hardware/v3/rtl/ddr_prefetch_mgr.v b/hardware/v3/rtl/ddr_prefetch_mgr.v new file mode 100644 index 0000000..0aed801 --- /dev/null +++ b/hardware/v3/rtl/ddr_prefetch_mgr.v @@ -0,0 +1,176 @@ +`timescale 1ns/1ps + +// ============================================================ +// V3 -- ddr_prefetch_mgr.v: the DDRManager's real phase-1 deliverable +// (EXP-0083), implementing the user's own proposed idea -- "vorrei che +// orchestrator potesse 'prenotare' le letture future in RAM ... in modo +// da azzerare i tempi di attesa (o almeno ridurli al minimo)" -- scoped, +// per this project's own "one variable at a time" discipline and the +// validation plan docs/ARCHITECTURE_ANALYSIS.md S5.2 laid out, to a +// SINGLE slot's own activation-tile look-ahead first, before attempting +// a cross-slot/whole-Director-queue scheduler. +// +// WHAT THIS DOES: wraps act_tile_fetch.v (unmodified, reused as the +// "fetch exactly one tile" engine) with a depth-2 ping-pong buffer and a +// sequencer that issues the NEXT tile's fetch the INSTANT the fetch +// engine is free and that tile's buffer bank is free -- NOT waiting for +// packed_slot.v to have consumed the CURRENT tile first. This overlaps +// "fetching tile N+1" with "packed_slot.v consuming tile N", which the +// original per-tile req/wait loop (EXP-0079/0081) never did. +// +// WHAT THIS DOES NOT DO (disclosed, not glossed over): it does not +// change the real 1.24 GB/s physical DDR3 ceiling (S3.1 of the +// architecture doc) or the per-fetch latency of any SINGLE tile fetch -- +// it only removes the small, real per-tile RE-REQUEST overhead (the +// S_TILEREQ pulse cycle + the S_OPERAND consume cycle packed_slot.v's +// own FSM previously spent NOT fetching, between one tile's data +// arriving and the next tile's fetch being issued). Given +// neural_processor_packed.v's own pipeline accepts one operand PER +// CYCLE once in NP_WAIT_OPERANDS (operand_ready is state-only, not +// gated on any internal pipeline stall), the real compute-side +// consumption cost per tile is ~1 cycle -- meaning this fix's real +// ceiling is bounded by that small per-tile overhead, not by hiding a +// large compute-bound stall. The real, measured improvement is reported +// in the EXP-0083 log entry, not assumed here. +// +// WHY A DOUBLE BUFFER (depth 2), not deeper: matches +// layer_weight_buffer.v's own proven ping-pong pattern in this +// codebase, and depth 2 is provably sufficient here -- the fetch +// sequencer can be at most 1 tile ahead of the consumer, since issuing +// tile N+2's fetch requires bank[(N+2)%2] == bank[N%2] to already be +// free, which only happens once tile N has been consumed. No unbounded +// lookahead is possible or attempted. +// +// WHY THIS IS TIMING-SAFE: bank selection for both the fill side +// (fetch_idx[0]) and the read side (consume_idx[0]) is a REGISTERED +// index bit, exactly the same "select known long before the data it +// gates" discipline act_tile_fetch.v's own header (EXP-0081) already +// established as safe -- never a bit racing live data. Requires its own +// real P&R re-check before being trusted at N>1 scale, per this +// project's standing practice (not assumed safe by analogy alone). +// ============================================================ +module ddr_prefetch_mgr #( + parameter DATA_WIDTH = 8, + parameter P_IN = 8, + parameter BURST_LEN = 8, + parameter ADDR_WIDTH = 25 // word address, matches act_tile_fetch.v's own convention +)( + input wire clk, + input wire rst, + + // ---- job-level control (packed_slot.v issues this ONCE per job, + // not once per tile -- the whole tile loop's lookahead is driven + // internally from here) ---- + input wire job_start, // one-shot pulse + input wire [ADDR_WIDTH-1:0] base_a, + input wire [ADDR_WIDTH-1:0] base_b, + input wire [15:0] n_tiles, + + // ---- per-tile consumption interface (packed_slot.v side) ---- + // tile_valid is a LEVEL signal (unlike act_tile_fetch.v's one-cycle + // `valid` pulse) -- it stays high as long as the current + // consume-index's buffer bank holds unconsumed data, which may + // already be true the cycle packed_slot.v asks, if the lookahead + // fetch completed early. packed_slot.v pulses tile_consume once it + // has latched data_a/data_b, which frees this bank for the next + // lookahead fetch. + output wire tile_valid, + output wire signed [DATA_WIDTH*P_IN-1:0] data_a, + output wire signed [DATA_WIDTH*P_IN-1:0] data_b, + input wire tile_consume, // one-shot pulse + + // ---- shared DDR3 controller port (identical shape to + // act_tile_fetch.v's own -- this module is a pure passthrough + // wrapper on this side, connects straight through to the inner + // act_tile_fetch instance) ---- + output wire mem_active, + input wire mem_grant, + output wire ctrl_req, + output wire ctrl_wr, + output wire [ADDR_WIDTH-1:0] ctrl_addr, + output wire [16*BURST_LEN-1:0] ctrl_wdata, + output wire [2*BURST_LEN-1:0] ctrl_wmask, + input wire [16*BURST_LEN-1:0] ctrl_rdata, + input wire ctrl_ready, + input wire ctrl_busy +); + reg [ADDR_WIDTH-1:0] base_a_lat, base_b_lat; + reg [15:0] n_tiles_lat; + reg [15:0] fetch_idx, consume_idx; + reg fetch_inflight; + + // ---- depth-2 ping-pong buffer ---- + reg signed [DATA_WIDTH*P_IN-1:0] bank_data_a [0:1]; + reg signed [DATA_WIDTH*P_IN-1:0] bank_data_b [0:1]; + reg [1:0] bank_valid; + + assign tile_valid = bank_valid[consume_idx[0]]; + assign data_a = bank_data_a[consume_idx[0]]; + assign data_b = bank_data_b[consume_idx[0]]; + + // ---- inner fetch engine: act_tile_fetch.v, reused unmodified, + // driven one tile at a time by this sequencer ---- + reg act_req; + wire act_valid; + wire signed [DATA_WIDTH*P_IN-1:0] act_data_a_w, act_data_b_w; + + act_tile_fetch #( + .DATA_WIDTH(DATA_WIDTH), .P_IN(P_IN), .BURST_LEN(BURST_LEN), .ADDR_WIDTH(ADDR_WIDTH) + ) u_act ( + .clk(clk), .rst(rst), + .req(act_req), .base_a(base_a_lat), .base_b(base_b_lat), + .tcnt(fetch_idx), .valid(act_valid), .data_a(act_data_a_w), .data_b(act_data_b_w), + .mem_active(mem_active), .mem_grant(mem_grant), + .ctrl_req(ctrl_req), .ctrl_wr(ctrl_wr), .ctrl_addr(ctrl_addr), + .ctrl_wdata(ctrl_wdata), .ctrl_wmask(ctrl_wmask), + .ctrl_rdata(ctrl_rdata), .ctrl_ready(ctrl_ready), .ctrl_busy(ctrl_busy) + ); + + // can_issue: the inner fetch engine is free, there is a next tile + // left in this job, and that tile's destination bank has already + // been consumed (or was never filled yet, at job start). + wire can_issue = !fetch_inflight && (fetch_idx < n_tiles_lat) && !bank_valid[fetch_idx[0]]; + + always @(posedge clk) begin + if (rst) begin + base_a_lat <= {ADDR_WIDTH{1'b0}}; + base_b_lat <= {ADDR_WIDTH{1'b0}}; + n_tiles_lat <= 16'd0; + fetch_idx <= 16'd0; + consume_idx <= 16'd0; + fetch_inflight <= 1'b0; + act_req <= 1'b0; + bank_valid <= 2'b00; + end else begin + act_req <= 1'b0; + + if (job_start) begin + base_a_lat <= base_a; + base_b_lat <= base_b; + n_tiles_lat <= n_tiles; + fetch_idx <= 16'd0; + consume_idx <= 16'd0; + fetch_inflight <= 1'b0; + bank_valid <= 2'b00; + end else begin + if (can_issue) begin + act_req <= 1'b1; + fetch_inflight <= 1'b1; + end + + if (act_valid) begin + bank_data_a[fetch_idx[0]] <= act_data_a_w; + bank_data_b[fetch_idx[0]] <= act_data_b_w; + bank_valid[fetch_idx[0]] <= 1'b1; + fetch_idx <= fetch_idx + 16'd1; + fetch_inflight <= 1'b0; + end + + if (tile_consume) begin + bank_valid[consume_idx[0]] <= 1'b0; + consume_idx <= consume_idx + 16'd1; + end + end + end + end +endmodule diff --git a/hardware/v3/rtl/packed_slot.v b/hardware/v3/rtl/packed_slot.v index fb0e8c1..41f7ee0 100644 --- a/hardware/v3/rtl/packed_slot.v +++ b/hardware/v3/rtl/packed_slot.v @@ -15,29 +15,30 @@ // node_id_a/b -> job_done/result_data_a/b/result_node_id_a/b). // // ACTIVATION FETCH (EXP-0079, real, closes the gap this header used to -// disclose as deferred): act_tile_fetch.v reads each tile's activation -// data DIRECTLY from the shared DDR3 bus, one tile at a time -- no -// on-chip buffering/prefetch (unlike weights, activation data is read -// exactly once per job, so buffering it would add complexity for zero -// reuse benefit). It shares THIS slot's own single ctrl_req/addr/etc -// port with layer_prefetch_ctrl.v (u_pf): the two are mutually -// exclusive in time by FSM construction (weight prefetch always fully -// completes, including its own consume_done, before the tile loop -// that needs activation data ever starts), muxed below on act_mem_ -// active. The outer arbiter's grant (mem_active/mem_grant, this -// module's own top-level ports) is now also needed during activation -// fetch, not just weight prefetch -- held PER TILE (one 2-burst fetch, -// lane A then lane B), released between tiles, matching this -// project's own established "lock the grant for one whole logical -// fetch, not longer" discipline (avoids starving the other slot for -// the whole tile loop's duration). +// disclose as deferred; EXP-0083 upgrades it to a look-ahead prefetch): +// ddr_prefetch_mgr.v wraps act_tile_fetch.v with a depth-2 ping-pong +// buffer, issuing tile N+1's fetch the instant the fetch engine is +// free rather than waiting for this slot to finish CONSUMING tile N -- +// overlapping "fetch next tile" with "consume current tile" (see +// ddr_prefetch_mgr.v's own header for the real, honest, measured scope +// of the benefit -- it does not raise the physical DDR3 ceiling, only +// removes small real per-tile re-request overhead). It shares THIS +// slot's own single ctrl_req/addr/etc port with layer_prefetch_ctrl.v +// (u_pf): the two are mutually exclusive in time by FSM construction +// (weight prefetch always fully completes, including its own +// consume_done, before the tile loop that needs activation data ever +// starts), muxed below on act_mem_active. The outer arbiter's grant +// (mem_active/mem_grant, this module's own top-level ports) is now +// also needed during activation fetch, not just weight prefetch -- +// held PER TILE (one 2-burst fetch, lane A then lane B), released +// between tiles, matching this project's own established "lock the +// grant for one whole logical fetch, not longer" discipline (avoids +// starving the other slot for the whole tile loop's duration). // -// MEMORY LAYOUT this requires of activation data in DDR3: each tile -// occupies its own full BURST_LEN=8-word burst slot (see act_tile_ -// fetch.v's own header for why -- avoiding a runtime-indexed part- -// select, a known Fmax risk this project's already-thin P&R margin, -// EXP-0078, can't afford right now). Documented for whoever prepares -// host-side data layout in the physical realization doc. +// MEMORY LAYOUT this requires of activation data in DDR3 (EXP-0081, +// v2 convention): two consecutive tiles share one full BURST_LEN=8- +// word burst (even index low 64 bits, odd index high 64 bits) -- see +// act_tile_fetch.v's own header and docs/PHYSICAL_REALIZATION.md S4. // // Also disclosed: no result-writeback engine exists yet either -- // result_addr_a/b are passed through unused, for a future writeback @@ -153,11 +154,14 @@ module packed_slot #( .ctrl_rdata(ctrl_rdata), .ctrl_ready(ctrl_ready), .ctrl_busy(ctrl_busy) ); - // ---- act_tile_fetch.v (EXP-0079): real activation fetch, shares - // this slot's own ctrl port with u_pf above (mutually exclusive in - // time -- see header) ---- - reg act_req; - wire act_valid; + // ---- ddr_prefetch_mgr.v (EXP-0083): look-ahead activation fetch, + // shares this slot's own ctrl port with u_pf above (mutually + // exclusive in time -- see header). Job-level start (once per job, + // not once per tile -- the whole tile loop's lookahead sequencing + // happens inside this module). + reg ddrpf_job_start; + wire ddrpf_tile_valid; + reg ddrpf_tile_consume; wire signed [DATA_WIDTH*P_IN-1:0] act_data_a_w, act_data_b_w; wire act_mem_active; @@ -166,12 +170,15 @@ module packed_slot #( wire [16*BURST_LEN-1:0] act_ctrl_wdata; wire [2*BURST_LEN-1:0] act_ctrl_wmask; - act_tile_fetch #( + ddr_prefetch_mgr #( .DATA_WIDTH(DATA_WIDTH), .P_IN(P_IN), .BURST_LEN(BURST_LEN), .ADDR_WIDTH(ADDR_WIDTH-1) - ) u_act ( + ) u_ddrpf ( .clk(clk), .rst(rst), - .req(act_req), .base_a(x_base_a_lat[ADDR_WIDTH-2:0]), .base_b(x_base_b_lat[ADDR_WIDTH-2:0]), - .tcnt(tcnt), .valid(act_valid), .data_a(act_data_a_w), .data_b(act_data_b_w), + .job_start(ddrpf_job_start), + .base_a(x_base_a_lat[ADDR_WIDTH-2:0]), .base_b(x_base_b_lat[ADDR_WIDTH-2:0]), + .n_tiles(n_tiles_lat), + .tile_valid(ddrpf_tile_valid), .data_a(act_data_a_w), .data_b(act_data_b_w), + .tile_consume(ddrpf_tile_consume), .mem_active(act_mem_active), .mem_grant(mem_grant), .ctrl_req(act_ctrl_req), .ctrl_wr(act_ctrl_wr), .ctrl_addr(act_ctrl_addr), .ctrl_wdata(act_ctrl_wdata), .ctrl_wmask(act_ctrl_wmask), @@ -180,7 +187,7 @@ module packed_slot #( // mutually exclusive by FSM construction (weight prefetch always // fully completes, incl. consume_done, before the tile loop that - // triggers act_req ever starts) -- safe to select on act_mem_active alone. + // triggers ddrpf_job_start ever fires) -- safe to select on act_mem_active alone. assign ctrl_req = act_mem_active ? act_ctrl_req : pf_ctrl_req; assign ctrl_wr = act_mem_active ? act_ctrl_wr : pf_ctrl_wr; assign ctrl_addr = act_mem_active ? act_ctrl_addr : pf_ctrl_addr; @@ -204,7 +211,9 @@ module packed_slot #( // ---- weight_tile_gather.v ---- reg tile_req; reg [BUFADDRW-1:0] tile_base; - reg tile_seen, act_seen; // S_TILEWAIT join latches (weight vs activation, see header) + reg tile_seen; // S_TILEWAIT join latch (weight side only -- see header; + // the activation side, ddrpf_tile_valid, is level-held by + // ddr_prefetch_mgr.v so it needs no separate latch) wire tile_valid; wire [DATA_WIDTH*P_IN-1:0] tile_data; @@ -261,9 +270,9 @@ module packed_slot #( pf_start <= 1'b0; consume_done <= 1'b0; tile_req <= 1'b0; - act_req <= 1'b0; + ddrpf_job_start <= 1'b0; + ddrpf_tile_consume <= 1'b0; tile_seen <= 1'b0; - act_seen <= 1'b0; job_valid_np <= 1'b0; operand_valid<= 1'b0; tile_last <= 1'b0; @@ -276,7 +285,8 @@ module packed_slot #( pf_start <= 1'b0; consume_done <= 1'b0; tile_req <= 1'b0; - act_req <= 1'b0; + ddrpf_job_start <= 1'b0; + ddrpf_tile_consume <= 1'b0; case (state) S_IDLE: begin @@ -320,42 +330,43 @@ module packed_slot #( S_JOBSTART: begin if (job_valid_np && job_ready_np) begin - job_valid_np <= 1'b0; - tcnt <= 16'd0; - state <= S_TILEREQ; + job_valid_np <= 1'b0; + tcnt <= 16'd0; + ddrpf_job_start <= 1'b1; // one-shot: kicks off the whole job's + // look-ahead tile loop inside u_ddrpf + state <= S_TILEREQ; end end S_TILEREQ: begin tile_req <= 1'b1; tile_base <= tcnt[BUFADDRW-1:0]*P_IN[BUFADDRW-1:0]; - act_req <= 1'b1; tile_seen <= 1'b0; - act_seen <= 1'b0; state <= S_TILEWAIT; end // Real join: weight_tile_gather.v's tile_valid (fast, - // on-chip) and act_tile_fetch.v's act_valid (real - // DDR3 latency, 2 bursts) do NOT arrive on the same - // cycle in general -- latch whichever comes first, - // proceed only once BOTH have been seen. Handles - // either arrival order correctly, not just the - // expected-common one (weight first). + // on-chip, one-cycle pulse -- latched via tile_seen) + // and u_ddrpf's ddrpf_tile_valid (real DDR3 latency, + // but LEVEL-held by the prefetch manager's own ping- + // pong buffer, possibly already true this cycle if the + // look-ahead fetch completed early) do NOT arrive on + // the same cycle in general -- proceed once BOTH are + // available. ddrpf_tile_valid needs no separate latch + // since it stays high until this slot pulses + // ddrpf_tile_consume itself. S_TILEWAIT: begin if (tile_valid) begin weight_data_r <= tile_data; tile_seen <= 1'b1; end - if (act_valid) begin - input_data_a_r <= act_data_a_w; - input_data_b_r <= act_data_b_w; - act_seen <= 1'b1; - end - if ((tile_valid || tile_seen) && (act_valid || act_seen)) begin - tile_last <= (tcnt == n_tiles_lat - 16'd1); - operand_valid <= 1'b1; - state <= S_OPERAND; + if ((tile_valid || tile_seen) && ddrpf_tile_valid) begin + input_data_a_r <= act_data_a_w; + input_data_b_r <= act_data_b_w; + ddrpf_tile_consume <= 1'b1; + tile_last <= (tcnt == n_tiles_lat - 16'd1); + operand_valid <= 1'b1; + state <= S_OPERAND; end end diff --git a/hardware/v3/sim/tb_ddr_prefetch_mgr.v b/hardware/v3/sim/tb_ddr_prefetch_mgr.v new file mode 100644 index 0000000..f409f5f --- /dev/null +++ b/hardware/v3/sim/tb_ddr_prefetch_mgr.v @@ -0,0 +1,310 @@ +`timescale 1ns/1ps + +// ============================================================ +// EXP-0083: (1) isolated correctness test for ddr_prefetch_mgr.v -- +// same real SDR SDRAM placeholder backend precedent as +// tb_act_tile_fetch.v -- and (2) a real, measured, honest A/B cycle- +// count comparison against the OLD per-tile req/wait/consume loop +// packed_slot.v used before EXP-0083, to get a REAL number for the +// look-ahead prefetch's benefit instead of asserting one. +// +// Both the "baseline" (direct act_tile_fetch.v, one requester per +// tile, old packed_slot.v sequencing) and the "prefetch" (ddr_ +// prefetch_mgr.v, EXP-0083) loops are run against the SAME shared +// backend and the SAME preloaded data, back to back, muxed the same +// way tb_act_tile_fetch.v's own pre_active mux works -- so the +// comparison is apples to apples, not two different simulated +// environments. +// +// Both loops apply the SAME 2-cycle "simulated compute overhead" per +// tile (matching packed_slot.v's own real S_TILEREQ + S_OPERAND +// single-cycle costs) between a tile becoming available and the next +// step being taken -- the honest question this answers is: does +// removing the OLD design's serialization of that overhead with the +// NEXT tile's DDR3 fetch produce a real, measurable improvement, and +// how much. +// ============================================================ +module tb; + localparam BURST_LEN = 8; + localparam ROW_BITS = 13; + localparam COL_BITS = 10; + localparam BANK_BITS = 2; + localparam ADDR_WIDTH = BANK_BITS + ROW_BITS + COL_BITS; // 25 + localparam CLK_FREQ_MHZ = 64; + localparam CLK_PERIOD_NS = 1000.0/CLK_FREQ_MHZ; + localparam DATA_WIDTH = 8; + localparam P_IN = 8; + localparam N_TILES = 6; // 3 burst-pairs/lane -- enough to see steady-state behavior + + reg clk = 0; + always #(CLK_PERIOD_NS/2.0) clk = ~clk; + reg rst; + + wire ctrl_req, ctrl_wr; + wire [ADDR_WIDTH-1:0] ctrl_addr; + wire [16*BURST_LEN-1:0] ctrl_wdata, ctrl_rdata; + wire [2*BURST_LEN-1:0] ctrl_wmask; + wire ctrl_ready, ctrl_busy; + wire cke, cs_n, ras_n, cas_n, we_n; + wire [BANK_BITS-1:0] ba; + wire [ROW_BITS-1:0] a; + wire [15:0] dq; + wire [1:0] dqm; + + sdram_controller #( + .CLK_FREQ_MHZ(CLK_FREQ_MHZ), .BURST_LEN(BURST_LEN), + .ROW_BITS(ROW_BITS), .COL_BITS(COL_BITS), .BANK_BITS(BANK_BITS) + ) u_ctrl ( + .clk(clk), .rst(rst), + .req(ctrl_req), .wr(ctrl_wr), .addr(ctrl_addr), .wdata(ctrl_wdata), .wmask(ctrl_wmask), + .rdata(ctrl_rdata), .ready(ctrl_ready), .busy(ctrl_busy), + .sdram_cke(cke), .sdram_cs_n(cs_n), .sdram_ras_n(ras_n), .sdram_cas_n(cas_n), .sdram_we_n(we_n), + .sdram_ba(ba), .sdram_a(a), .sdram_dq(dq), .sdram_dqm(dqm) + ); + sdram_model #( + .CLK_FREQ_MHZ(CLK_FREQ_MHZ), .ROW_BITS(ROW_BITS), .COL_BITS(COL_BITS), .BANK_BITS(BANK_BITS) + ) u_mem ( + .clk(clk), .cke(cke), .cs_n(cs_n), .ras_n(ras_n), .cas_n(cas_n), .we_n(we_n), + .ba(ba), .a(a), .dq(dq), .dqm(dqm) + ); + + // ---- 3-way mux on the shared backend: preload / baseline DUT / prefetch DUT ---- + localparam SEL_PRELOAD = 2'd0, SEL_BASE = 2'd1, SEL_PF = 2'd2; + reg [1:0] sel; + + reg pre_req, pre_wr; + reg [ADDR_WIDTH-1:0] pre_addr; + reg [16*BURST_LEN-1:0] pre_wdata; + + // ---- baseline DUT: plain act_tile_fetch.v, driven by a per-tile + // req/wait/consume loop replicating OLD packed_slot.v sequencing ---- + reg base_req; + reg [ADDR_WIDTH-1:0] base_base_a, base_base_b; + reg [15:0] base_tcnt; + wire base_valid; + wire signed [DATA_WIDTH*P_IN-1:0] base_data_a, base_data_b; + wire base_mem_active; + wire base_ctrl_req, base_ctrl_wr; + wire [ADDR_WIDTH-1:0] base_ctrl_addr; + wire [16*BURST_LEN-1:0] base_ctrl_wdata; + wire [2*BURST_LEN-1:0] base_ctrl_wmask; + + act_tile_fetch #( + .DATA_WIDTH(DATA_WIDTH), .P_IN(P_IN), .BURST_LEN(BURST_LEN), .ADDR_WIDTH(ADDR_WIDTH) + ) u_base ( + .clk(clk), .rst(rst), + .req(base_req), .base_a(base_base_a), .base_b(base_base_b), .tcnt(base_tcnt), + .valid(base_valid), .data_a(base_data_a), .data_b(base_data_b), + .mem_active(base_mem_active), .mem_grant(sel == SEL_BASE), + .ctrl_req(base_ctrl_req), .ctrl_wr(base_ctrl_wr), .ctrl_addr(base_ctrl_addr), + .ctrl_wdata(base_ctrl_wdata), .ctrl_wmask(base_ctrl_wmask), + .ctrl_rdata(ctrl_rdata), .ctrl_ready(ctrl_ready), .ctrl_busy(ctrl_busy) + ); + + // ---- prefetch DUT: ddr_prefetch_mgr.v (EXP-0083) ---- + reg pf_job_start; + reg [ADDR_WIDTH-1:0] pf_base_a, pf_base_b; + reg [15:0] pf_n_tiles; + wire pf_tile_valid; + wire signed [DATA_WIDTH*P_IN-1:0] pf_data_a, pf_data_b; + reg pf_tile_consume; + wire pf_mem_active; + wire pf_ctrl_req, pf_ctrl_wr; + wire [ADDR_WIDTH-1:0] pf_ctrl_addr; + wire [16*BURST_LEN-1:0] pf_ctrl_wdata; + wire [2*BURST_LEN-1:0] pf_ctrl_wmask; + + ddr_prefetch_mgr #( + .DATA_WIDTH(DATA_WIDTH), .P_IN(P_IN), .BURST_LEN(BURST_LEN), .ADDR_WIDTH(ADDR_WIDTH) + ) u_pf ( + .clk(clk), .rst(rst), + .job_start(pf_job_start), .base_a(pf_base_a), .base_b(pf_base_b), .n_tiles(pf_n_tiles), + .tile_valid(pf_tile_valid), .data_a(pf_data_a), .data_b(pf_data_b), .tile_consume(pf_tile_consume), + .mem_active(pf_mem_active), .mem_grant(sel == SEL_PF), + .ctrl_req(pf_ctrl_req), .ctrl_wr(pf_ctrl_wr), .ctrl_addr(pf_ctrl_addr), + .ctrl_wdata(pf_ctrl_wdata), .ctrl_wmask(pf_ctrl_wmask), + .ctrl_rdata(ctrl_rdata), .ctrl_ready(ctrl_ready), .ctrl_busy(ctrl_busy) + ); + + assign ctrl_req = (sel==SEL_PRELOAD) ? pre_req : (sel==SEL_BASE) ? base_ctrl_req : pf_ctrl_req; + assign ctrl_wr = (sel==SEL_PRELOAD) ? pre_wr : (sel==SEL_BASE) ? base_ctrl_wr : pf_ctrl_wr; + assign ctrl_addr = (sel==SEL_PRELOAD) ? pre_addr : (sel==SEL_BASE) ? base_ctrl_addr : pf_ctrl_addr; + assign ctrl_wdata = (sel==SEL_PRELOAD) ? pre_wdata : (sel==SEL_BASE) ? base_ctrl_wdata : pf_ctrl_wdata; + assign ctrl_wmask = (sel==SEL_BASE) ? base_ctrl_wmask : (sel==SEL_PF) ? pf_ctrl_wmask : {(2*BURST_LEN){1'b0}}; + + task automatic sdram_write_burst(input [ADDR_WIDTH-1:0] word_addr, input [16*BURST_LEN-1:0] data); + begin + @(posedge clk); while (ctrl_busy) @(posedge clk); + pre_req = 1'b1; pre_wr = 1'b1; pre_addr = word_addr; pre_wdata = data; + @(posedge clk); pre_req = 1'b0; + while (!ctrl_ready) @(posedge clk); + end + endtask + + function automatic signed [7:0] act_byte(input integer base, input integer t, input integer k); + act_byte = $signed(8'((base*13 + t*31 + k*7 + 5) & 8'hFF)); + endfunction + + integer errors, tests; + task automatic check(input cond, input [255:0] name); + begin + tests = tests + 1; + if (!cond) begin errors = errors + 1; $display("FAIL: %0s", name); end + else $display("PASS: %0s", name); + end + endtask + + reg signed [DATA_WIDTH*P_IN-1:0] exp_a, exp_b; + integer k, wi, cyc; + reg [16*BURST_LEN-1:0] burst; + integer t_start, t_end, base_cycles, pf_cycles; + + initial begin + errors = 0; tests = 0; + rst = 1; sel = SEL_PRELOAD; + pre_req = 0; pre_wr = 0; pre_addr = 0; pre_wdata = 0; + base_req = 0; base_base_a = 0; base_base_b = 0; base_tcnt = 0; + pf_job_start = 0; pf_base_a = 0; pf_base_b = 0; pf_n_tiles = 0; pf_tile_consume = 0; + repeat(5) @(posedge clk); + rst = 0; + @(posedge clk); while (ctrl_busy) @(posedge clk); + + $display("=== preload lane A base=0, lane B base=200, %0d tiles (EXP-0081 2-tiles/burst layout) ===", N_TILES); + for (wi = 0; wi < N_TILES/2; wi = wi + 1) begin + for (k = 0; k < P_IN/2; k = k + 1) + burst[k*16 +: 16] = {act_byte(0, 2*wi, 2*k+1), act_byte(0, 2*wi, 2*k)}; + for (k = 0; k < P_IN/2; k = k + 1) + burst[(P_IN/2+k)*16 +: 16] = {act_byte(0, 2*wi+1, 2*k+1), act_byte(0, 2*wi+1, 2*k)}; + sdram_write_burst(0 + wi*BURST_LEN, burst); + + for (k = 0; k < P_IN/2; k = k + 1) + burst[k*16 +: 16] = {act_byte(200, 2*wi, 2*k+1), act_byte(200, 2*wi, 2*k)}; + for (k = 0; k < P_IN/2; k = k + 1) + burst[(P_IN/2+k)*16 +: 16] = {act_byte(200, 2*wi+1, 2*k+1), act_byte(200, 2*wi+1, 2*k)}; + sdram_write_burst(200 + wi*BURST_LEN, burst); + end + @(posedge clk); + + // ============================================================ + // PART 1: baseline correctness + real cycle count, OLD-style + // per-tile req -> wait valid -> 2-cycle simulated compute -> next req + // ============================================================ + $display("=== PART 1: baseline (direct act_tile_fetch.v, old packed_slot.v loop) ==="); + sel = SEL_BASE; + @(posedge clk); + t_start = $time; + for (k = 0; k < N_TILES; k = k + 1) begin + @(posedge clk); + base_base_a <= 0; base_base_b <= 200; base_tcnt <= k[15:0]; + base_req <= 1'b1; + @(posedge clk); + base_req <= 1'b0; + while (!base_valid) @(posedge clk); + for (wi = 0; wi < P_IN; wi = wi + 1) begin + exp_a[wi*DATA_WIDTH +: DATA_WIDTH] = act_byte(0, k, wi); + exp_b[wi*DATA_WIDTH +: DATA_WIDTH] = act_byte(200, k, wi); + end + check(base_data_a === exp_a, "baseline: lane A bit-exact"); + check(base_data_b === exp_b, "baseline: lane B bit-exact"); + repeat(2) @(posedge clk); // simulated S_TILEREQ + S_OPERAND overhead + end + t_end = $time; + base_cycles = (t_end - t_start) / CLK_PERIOD_NS; + $display("baseline: %0d tiles in %0d cycles (%0.2f cycles/tile)", N_TILES, base_cycles, base_cycles*1.0/N_TILES); + + // ============================================================ + // PART 2: prefetch correctness + real cycle count, EXP-0083 + // job-level start, poll tile_valid, 2-cycle simulated compute, consume + // ============================================================ + $display("=== PART 2: prefetch (ddr_prefetch_mgr.v, EXP-0083) ==="); + sel = SEL_PF; + @(posedge clk); + pf_base_a <= 0; pf_base_b <= 200; pf_n_tiles <= N_TILES[15:0]; + pf_job_start <= 1'b1; + t_start = $time; + @(posedge clk); + pf_job_start <= 1'b0; + for (k = 0; k < N_TILES; k = k + 1) begin + // #1 settle delay: without it, this check can run in the same + // simulation delta as the PREVIOUS iteration's tile_consume + // pulse being sampled by the DUT (both triggered off the same + // @(posedge clk)), reading pf_tile_valid/bank_valid BEFORE the + // DUT's own nonblocking update for that consume has been + // applied -- a real testbench race, not an RTL bug (found via + // an iteration-tagged trace: k=1 was reading k=0's still- + // unconsumed bank). #1 (real time, 1ns << the 15.625ns clock + // period) forces this poll to always run strictly after that + // update has settled. + #1; + while (!pf_tile_valid) @(posedge clk); + for (wi = 0; wi < P_IN; wi = wi + 1) begin + exp_a[wi*DATA_WIDTH +: DATA_WIDTH] = act_byte(0, k, wi); + exp_b[wi*DATA_WIDTH +: DATA_WIDTH] = act_byte(200, k, wi); + end + check(pf_data_a === exp_a, "prefetch: lane A bit-exact"); + check(pf_data_b === exp_b, "prefetch: lane B bit-exact"); + repeat(2) @(posedge clk); // SAME simulated compute overhead as baseline + pf_tile_consume <= 1'b1; + @(posedge clk); + pf_tile_consume <= 1'b0; + end + t_end = $time; + pf_cycles = (t_end - t_start) / CLK_PERIOD_NS; + $display("prefetch: %0d tiles in %0d cycles (%0.2f cycles/tile)", N_TILES, pf_cycles, pf_cycles*1.0/N_TILES); + + $display("=== REAL MEASURED COMPARISON (row-switch-heavy, 3 different burst pairs): baseline=%0d cycles, prefetch=%0d cycles, reduction=%0.1f%% ===", + base_cycles, pf_cycles, 100.0*(base_cycles-pf_cycles)/base_cycles); + check(pf_cycles < base_cycles, "prefetch is real, measurably faster than baseline (row-switch-heavy)"); + + // ============================================================ + // PART 3 (EXP-0083 addendum): best-case, SAME-ROW comparison -- + // only tiles 0/1 (both already resident in the FIRST preloaded + // burst pair, no row activation needed for either), isolating + // the look-ahead benefit from row-switch cost entirely. Answers + // honestly whether the small PART-1/2 result is because the + // benefit is inherently small, or because row-switch cost + // dominates and masks it in that scenario. + // ============================================================ + $display("=== PART 3: same-row best case (2 tiles, single burst pair, no row switch) ==="); + sel = SEL_BASE; + @(posedge clk); + t_start = $time; + for (k = 0; k < 2; k = k + 1) begin + @(posedge clk); + base_base_a <= 0; base_base_b <= 200; base_tcnt <= k[15:0]; + base_req <= 1'b1; + @(posedge clk); + base_req <= 1'b0; + while (!base_valid) @(posedge clk); + repeat(2) @(posedge clk); + end + t_end = $time; + base_cycles = (t_end - t_start) / CLK_PERIOD_NS; + $display("same-row baseline: 2 tiles in %0d cycles", base_cycles); + + sel = SEL_PF; + @(posedge clk); + pf_base_a <= 0; pf_base_b <= 200; pf_n_tiles <= 16'd2; + pf_job_start <= 1'b1; + t_start = $time; + @(posedge clk); + pf_job_start <= 1'b0; + for (k = 0; k < 2; k = k + 1) begin + #1; + while (!pf_tile_valid) @(posedge clk); + repeat(2) @(posedge clk); + pf_tile_consume <= 1'b1; + @(posedge clk); + pf_tile_consume <= 1'b0; + end + t_end = $time; + pf_cycles = (t_end - t_start) / CLK_PERIOD_NS; + $display("same-row prefetch: 2 tiles in %0d cycles", pf_cycles); + $display("=== REAL MEASURED COMPARISON (same-row, best case): baseline=%0d cycles, prefetch=%0d cycles, reduction=%0.1f%% ===", + base_cycles, pf_cycles, 100.0*(base_cycles-pf_cycles)/base_cycles); + + $display("=== %0d/%0d tests, %0d errors ===", tests-errors, tests, errors); + if (errors == 0) $display("ALL TESTS PASSED (tb_ddr_prefetch_mgr)"); + $finish; + end +endmodule