From 481b5d223fbdcb060c2a62fbcbd6bc67ceb5dba6 Mon Sep 17 00:00:00 2001 From: Michele Bigi Date: Mon, 7 Sep 2026 19:16:01 +0200 Subject: [PATCH] perf: fix N=4@64MHz 7/8->8/8 by removing any_pending fan-out (user's idea) Real critical-path trace after the flash #1 revert showed a NEW bottleneck (neural_director.job_out_slot -> dependency_manager. node_resolved/node_state, 76-84% routing) distinct from every prior fix this session -- already a flat, parallel 64-way compare, not a serial chain, so the established restructuring fix class doesn't apply. Root cause: any_pending (added for FPGA_DATA_READY) reads node_state[0:N_NODES-1] combinationally every cycle, adding real fan-out onto the same congested signal. User's own suggestion: replace the combinational scan with a synchronous up/down counter. pending_count +1 on registration acceptance, -1 on dispatch acceptance; any_pending = (pending_count != 0) -- mathematically identical (DEC-0008: nodes never reclaimed mid-run) but reads one small register instead of scanning a 16-wide array every cycle. Verified: D-Stress N=4 bit-exact (49927 cycles, data_ready PASS). Fresh 8-seed P&R: N_SLOTS=4 @ 64MHz now 8/8 PASS (was 7/8 after the flash revert), worst seed1 64.55MHz, best seed0 72.37MHz. See decisions.log DEC-0042. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_013xXuuRUWZScuo1DeYJxs3v --- hardware/v2/logs/decisions.log | 60 ++++++++++++++++++++++++++++ hardware/v2/rtl/dependency_manager.v | 42 ++++++++++++++----- 2 files changed, 91 insertions(+), 11 deletions(-) diff --git a/hardware/v2/logs/decisions.log b/hardware/v2/logs/decisions.log index c659726..d37e3fa 100644 --- a/hardware/v2/logs/decisions.log +++ b/hardware/v2/logs/decisions.log @@ -2402,3 +2402,63 @@ STATUS: two-flash architecture and FPGA_DATA_READY CLOSED and verified (bit-exact + real placement). Flash #1's RTL port (`flash_copy_engine.v` integration into V2's top-level) remains a real, separate, OPEN task -- ball positions reserved, not wired. + +DEC-0042 -- Flash #1 removed (user priority: MHz over on-board +persistence); dependency_manager's any_pending switched from a +combinational scan to a counter, recovering N_SLOTS=4 @ 64MHz to 8/8 + +DATE: 2026-09-07 +CONTEXT: DEC-0042's own predecessor (see the reverted commit) closed +flash #1 functionally but caused a real, disclosed timing regression +(N_SLOTS=4 8/8 -> 3/8). User's explicit decision: prioritize clock +frequency over on-board flash persistence -- "preferisco avere un +Megahertz più alti che avere la flash" -- ESP32 can push data fresh +each session instead. Flash #1 fully reverted (`git revert`, clean, +history-preserving) rather than hand-edited back out. + +POST-REVERT REAL BASELINE (fresh 8-seed matrix, first rigorous +measurement of this exact RTL state -- includes the earlier sdram_clk +fix, never previously swept at full seed count): N_SLOTS=4 @ 64MHz +7/8 PASS (worst seed1 62.47MHz FAIL; best seed4 77.21MHz). N_SLOTS=8 +@ 64MHz 3/8 PASS. User's explicit direction: focus exclusively on +N_SLOTS=4 for now, treat N_SLOTS=8 as a later/separate effort. + +ROOT CAUSE of the remaining N=4 seed1 failure (real critical-path +trace, not guessed): a NEW bottleneck class, distinct from every prior +fix this session (ERR-0027/0028/0029, all serial-to-parallel logic +restructurings) -- `neural_director.job_out_slot` -> +`dependency_manager.node_resolved`/`node_state`, 76-84% routing. +Inspected the actual logic: already a flat, parallel 64-way compare +(N_NODES x MAX_DEPS independent comparators, no serial chain), so the +established "flatten a chain" fix class does not apply. Root cause is +signal FAN-OUT, not logic depth: this session's own earlier +`any_pending` addition (DEC-0041/ERR's own FPGA_DATA_READY work) reads +`node_state[0:N_NODES-1]` combinationally every cycle (a 16-wide +OR-reduce), adding real fan-out load onto the SAME `node_state` array +already sitting on this critical path. + +FIX (user's own idea, credited): replace the combinational OR-reduce +with a synchronous up/down counter -- `pending_count` incremented on +a node's own registration acceptance (`reg_valid&®_ready`, the +exact edge it enters WAITING/READY) and decremented on its own +dispatch acceptance (`ready_valid&&ready_ready`, the exact edge it +leaves WAITING/READY for DISPATCHED); `any_pending = (pending_count != +0)`. Mathematically identical to the original OR-reduce (DEC-0008: +nodes are never reclaimed mid-run, so registered-count minus +dispatched-count exactly equals "currently WAITING or READY"), but +reads one small registered counter instead of scanning a 16-wide array +every cycle -- zero added fan-out on the congested node_state signal. +File changed: `hardware/v2/rtl/dependency_manager.v`. + +VERIFICATION: D-Stress N_SLOTS=4 re-run, bit-exact (49927 cycles, +identical to before, 256/256 vs golden), `data_ready` PASS (confirms +the counter-based reimplementation is semantically identical in +practice, not just in theory). Fresh 8-seed nextpnr-ecp5 P&R matrix: +**N_SLOTS=4 @ 64MHz now 8/8 PASS** -- worst seed1 64.55MHz (real but +thin margin, +0.55MHz), best seed0 72.37MHz. + +STATUS: N_SLOTS=4 @ 64MHz CLOSED, 8/8, real production baseline. +N_SLOTS=8 explicitly deferred by user request, not attempted further +this pass. Flash #1 removed; its RTL (flash_mem_adapter.v, +tb_flash_integration_smoke.v, the OP_FLASH_CMD opcode) remains in git +history (revertible commit `59901a4`) if ever needed again. diff --git a/hardware/v2/rtl/dependency_manager.v b/hardware/v2/rtl/dependency_manager.v index 947c769..a9e58c4 100644 --- a/hardware/v2/rtl/dependency_manager.v +++ b/hardware/v2/rtl/dependency_manager.v @@ -118,16 +118,26 @@ module dependency_manager #( end end - // ---- FPGA_DATA_READY support (see any_pending port comment above) ---- - reg any_pending_r; - integer pi; - always @(*) begin - any_pending_r = 1'b0; - for (pi = 0; pi < N_NODES; pi = pi + 1) - if (node_state[pi] == ST_WAITING || node_state[pi] == ST_READY) - any_pending_r = 1'b1; - end - assign any_pending = any_pending_r; + // ---- FPGA_DATA_READY support (see any_pending port comment above). + // Originally a combinational OR-reduce over node_state[0:N_NODES-1] + // (16-wide), which added real fan-out load onto node_state -- a + // signal this session's own real P&R critical-path traces later + // showed sitting on the SAME already-congested job_out_slot -> + // node_resolved/node_state broadcast path (routing-dominated, + // 76-84%). Replaced with a synchronous up/down counter: +1 on a + // node's own registration acceptance (reg_valid&®_ready -- + // exactly when it enters WAITING/READY), -1 on its own dispatch + // acceptance (ready_valid&&ready_ready -- exactly when it leaves + // WAITING/READY for DISPATCHED). registered-minus-dispatched is + // mathematically identical to the original OR-reduce's own + // "any node currently WAITING or READY" condition (DEC-0008: nodes + // are never reclaimed mid-run, so every node visits EMPTY -> + // {WAITING or READY} -> DISPATCHED exactly once), but reads a + // single small registered counter instead of scanning a wide array + // every cycle -- zero added fan-out on the congested signals. ---- + localparam PENDW = $clog2(N_NODES+1); + reg [PENDW-1:0] pending_count; + assign any_pending = (pending_count != {PENDW{1'b0}}); integer ni, di; @@ -138,9 +148,19 @@ module dependency_manager #( node_required[ni] <= {REQW{1'b0}}; node_resolved[ni] <= {REQW{1'b0}}; end - ready_valid <= 1'b0; + ready_valid <= 1'b0; + pending_count <= {PENDW{1'b0}}; end else begin + // pending_count: +1 on registration acceptance, -1 on + // dispatch acceptance; a same-cycle occurrence of both is a + // net zero change (no assignment needed, old value holds). + case ({(reg_valid && reg_ready), (ready_valid && ready_ready)}) + 2'b10: pending_count <= pending_count + 1'b1; + 2'b01: pending_count <= pending_count - 1'b1; + default: ; // 00 or 11: no net change + endcase + // ---- registration: create a new WAITING (or immediately // READY, if required==0) node entry. ---- if (reg_valid && reg_ready) begin