feat: real hierarchical 2-level arbiter, real N=16 timing WNS -0.913ns -> -0.646ns (EXP-0094)

sdram_arbiter_hier.v: fixes EXP-0093's own real, traced P&R timing
failure (flat 21-way req_wdata mux, route-delay-dominated). Reuses
sdram_arbiter_n.v unmodified, twice: 4 leaf instances (NUM_REQ=5, one
per group) + 1 top instance (NUM_REQ=5: 4 groups + host, host
bypassed/unpipelined), one real pipeline register stage between
levels. Isolated verification (tb_sdram_arbiter_hier.v): 23/23 PASS.

Two real bugs found and fixed via signal tracing: a testbench helper
not waiting for grant before firing req, and a genuine RTL lost-pulse
bug at the leaf-to-top boundary (a transient one-shot request could be
dropped if the top level was busy with a different group) -- fixed
with a sticky per-group pending_req_r latch.

Wired into n16_system_ddr3_top.v (drop-in). Real, full P&R re-run:
WNS improved -0.913ns -> -0.646ns, TNS -690ns -> -97.5ns, failing
endpoints 3021 -> 771 -- substantial, measured improvement, confirming
the arbiter was correctly root-caused (bottleneck moved elsewhere:
neural_processor_packed.v's own already-thin-margin MAC datapath,
eroded by N=16's higher overall congestion). Functional xsim still
32/32 PASS. Timing not yet fully closed -- real next steps documented,
not yet attempted without further direction.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUG92aM9m68TRc4rG55BcC
This commit is contained in:
2026-09-21 02:25:44 +02:00
co-authored by Claude Sonnet 5
parent 50c940a41d
commit 9e1f16db47
7 changed files with 840 additions and 19 deletions
+38
View File
@@ -115,6 +115,44 @@ unmodified by v3, e.g. `layer_prefetch_ctrl.v`/`layer_weight_buffer.v`).
registered/one-cycle-late grant silently loses the request forever
(EXP-0066's own real bug, now a standing design rule for every arbiter/
requester pair in this project).
- **When PIPELINING/hierarchically staging an arbiter's own `req`
signal (not just its `active`/`grant`), the `req` pulse must be
latched STICKY across the pipeline boundary, not just registered
every cycle** (EXP-0094, `sdram_arbiter_hier.v`). A real one-shot
`ctrl_req` pulse (e.g. `act_tile_fetch.v`'s own S_MEMWAIT: `ctrl_req
<= 1'b1` for exactly one cycle) is captured fine by a SINGLE-level
arbiter (the winning requester's own grant and the physical
controller's readiness to capture it are the SAME decision, always
same-cycle). Once a SECOND arbitration level is added downstream
(e.g. a top-level arbiter deciding which of several leaf groups gets
the real shared port), a leaf's own LOCAL grant no longer guarantees
the top level is free to act on it that same cycle — if the pipeline
register between levels just does `top_req_r <= leaf_req` every
cycle, the transient one-shot pulse reverts to 0 before the top level
gets around to it (e.g. busy with a different leaf), silently losing
the request — same EXP-0066 lost-pulse class, newly exposed at the
hierarchy boundary. Fix: `pending_r <= (pending_r | leaf_req) &
~dispatched;` (set on first pulse, clear only once the top level
confirms real dispatch) feeds the pipeline register instead of the
bare transient signal. `active`/addr/data fields don't need this (a
real requester holds `active` level, and stable addr/data, for its
whole transaction) — only the transient `req` pulse does.
- **A testbench helper that fires a one-shot `req` the same cycle as
`active`, unconditionally (without checking `grant` first), is only
safe for a flat, single-level, uncontended arbiter test** — copying
it verbatim into a test for a hierarchical/pipelined arbiter (EXP-0094)
can itself cause a spurious hang, unrelated to any real RTL bug. Real
requesters in this project (`act_tile_fetch.v` etc) already wait for
`grant` before firing `req` (S_MEMWAIT) — match that in any new
testbench helper, don't assume the naive same-cycle-fire pattern
generalizes.
- **Give every new testbench a real cycle-counted watchdog from the
start** (EXP-0094) — one testbench, copied from a simpler precedent
that didn't need one, had none; a real protocol bug spun Icarus
forever at ~99% CPU with zero output instead of failing cleanly.
Every other testbench in this project already has a `wd`-counted
watchdog inside its own completion-wait loop — don't skip it "just
this once."
- **`xvlog`/`iverilog` need `-sv`/`-g2012`** respectively to accept
SystemVerilog-only syntax (e.g. `'0`) even in a plain `.v` file — prefer
just not using SV-only syntax in synthesizable RTL (Vivado's `synth_design`