From ca940833666d377a8fca46e90ee3d0ff1f05be6c Mon Sep 17 00:00:00 2001 From: manvalan Date: Wed, 16 Sep 2026 12:29:52 +0200 Subject: [PATCH] prod: promote EXP-0056 fixes into production RTL (dependency_manager.v, nms_activation_fill_ctrl_v3.v) Per explicit instruction: when a bug/fix found in an experimental fork also applies to the production file it was forked from, apply it there too, not just in the fork. dependency_manager.v: swaps in priority_encoder_lsb.v for the first_ready_idx scan (was: serial O(N_NODES) for-loop). Bit-exact equivalent, correctness-neutral by construction. nms_activation_fill_ctrl_v3.v: adds the missing N_SLOTS==16 balanced max-tree case (was: silently falling back to the slow flat scan for any N_SLOTS not in {1,2,4,8}) -- this was the real cause of N_SLOTS=16 failing timing closure (23-24MHz vs 64MHz target), fixed to 71.01MHz PASS in the experimental fork. Verified on the REAL, unmodified production top (fpga_neural_v2_top.v, N_SLOTS=4 default): tb_dependency_manager.v 4/4 PASS, board smoke test 11/11 PASS, D-Stress N=4 total_cycles=49927 (bit-exact, IDENTICAL to the pre-fix baseline -- zero functional regression, as expected from a pure combinational-depth change). Real nextpnr-ecp5 P&R (LFE5U-45F, seed 1): 97.36MHz, PASS at 64MHz -- BETTER margin than the pre-fix baseline's own 76.80-88.25MHz seed range, not just neutral. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01MUG92aM9m68TRc4rG55BcC --- .../v2/nms/rtl/nms_activation_fill_ctrl_v3.v | 30 +++++++++++++ hardware/v2/rtl/dependency_manager.v | 43 +++++++++++++------ 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/hardware/v2/nms/rtl/nms_activation_fill_ctrl_v3.v b/hardware/v2/nms/rtl/nms_activation_fill_ctrl_v3.v index 44e10ca..9e6df5c 100644 --- a/hardware/v2/nms/rtl/nms_activation_fill_ctrl_v3.v +++ b/hardware/v2/nms/rtl/nms_activation_fill_ctrl_v3.v @@ -178,6 +178,36 @@ module nms_activation_fill_ctrl_v3 #( if (rst) max_n_tiles_reg <= 16'h0; else max_n_tiles_reg <= max_final; end + end else if (N_SLOTS == 16) begin : GEN_MAXTREE_N16 + // EXP-0056: same balanced-tree pattern as N_SLOTS==8 above, + // one more level. Promoted from nms_activation_fill_ctrl_v3_n16.v + // (experimental fork) after that fork's own fix was + // verified (isolated: 10017/10017; real nextpnr-ecp5 P&R + // at N_SLOTS=16, LFE5U-85F: 71.01MHz, PASS at 64MHz, vs + // 23.52-24.64MHz before -- see experiments.log EXP-0056) + // -- this exact case used to fall through to + // GEN_MAXTREE_FALLBACK's own flat sequential scan, the + // real measured critical path blocking N_SLOTS=16 timing + // closure. + wire [15:0] m0 = (n_tiles_masked[0] > n_tiles_masked[1]) ? n_tiles_masked[0] : n_tiles_masked[1]; + wire [15:0] m1 = (n_tiles_masked[2] > n_tiles_masked[3]) ? n_tiles_masked[2] : n_tiles_masked[3]; + wire [15:0] m2 = (n_tiles_masked[4] > n_tiles_masked[5]) ? n_tiles_masked[4] : n_tiles_masked[5]; + wire [15:0] m3 = (n_tiles_masked[6] > n_tiles_masked[7]) ? n_tiles_masked[6] : n_tiles_masked[7]; + wire [15:0] m4 = (n_tiles_masked[8] > n_tiles_masked[9]) ? n_tiles_masked[8] : n_tiles_masked[9]; + wire [15:0] m5 = (n_tiles_masked[10] > n_tiles_masked[11]) ? n_tiles_masked[10] : n_tiles_masked[11]; + wire [15:0] m6 = (n_tiles_masked[12] > n_tiles_masked[13]) ? n_tiles_masked[12] : n_tiles_masked[13]; + wire [15:0] m7 = (n_tiles_masked[14] > n_tiles_masked[15]) ? n_tiles_masked[14] : n_tiles_masked[15]; + wire [15:0] m01 = (m0 > m1) ? m0 : m1; + wire [15:0] m23 = (m2 > m3) ? m2 : m3; + wire [15:0] m45 = (m4 > m5) ? m4 : m5; + wire [15:0] m67 = (m6 > m7) ? m6 : m7; + wire [15:0] m0123 = (m01 > m23) ? m01 : m23; + wire [15:0] m4567 = (m45 > m67) ? m45 : m67; + wire [15:0] max_final = (m0123 > m4567) ? m0123 : m4567; + always @(posedge clk) begin + if (rst) max_n_tiles_reg <= 16'h0; + else max_n_tiles_reg <= max_final; + end end else begin : GEN_MAXTREE_FALLBACK reg [15:0] max_n_tiles_comb_fallback; integer j; diff --git a/hardware/v2/rtl/dependency_manager.v b/hardware/v2/rtl/dependency_manager.v index a9e58c4..9ef4651 100644 --- a/hardware/v2/rtl/dependency_manager.v +++ b/hardware/v2/rtl/dependency_manager.v @@ -102,21 +102,36 @@ module dependency_manager #( // therefore cover the full id range a caller intends to use. assign reg_ready = (node_state[reg_node_id] == ST_EMPTY); - // ---- priority-encoded first READY node (first-found scan, same - // idiom as neural_director's own free-slot scan) ---- - reg [NODE_IDW-1:0] first_ready_idx; - reg any_ready; - integer ri; - always @(*) begin - first_ready_idx = {NODE_IDW{1'b0}}; - any_ready = 1'b0; - for (ri = N_NODES-1; ri >= 0; ri = ri - 1) begin - if (node_state[ri] == ST_READY) begin - first_ready_idx = ri[NODE_IDW-1:0]; - any_ready = 1'b1; - end + // ---- EXP-0056: priority-encoded first READY node, via a + // recursive binary-tree lowest-set-bit encoder (O(log2(N_NODES)) + // depth) instead of the original serial for-loop scan (O(N_NODES) + // depth, the same architectural anti-pattern already fixed twice + // elsewhere in this project -- ERR-0027/ERR-0028/ERR-0029). + // Promoted from dependency_manager_fast.v (experimental fork) + // after that fork's own fix was verified bit-exact equivalent to + // this original scan (isolated: 65536/65536 exhaustive at + // WIDTH=16, 76562/76562 at WIDTH=1024; 20000/20000 cycles matched + // under random stimulus against this exact module, plus the + // original hand-crafted DAG testbench, both 100% -- see + // priority_encoder_lsb.v's own header and experiments.log + // EXP-0056). Correctness-neutral by construction (shorter + // combinational depth is never worse); NOTE this was NOT the real + // N_SLOTS=16 timing bottleneck (that was nms_activation_fill_ + // ctrl_v3.v's own missing N==16 case, see that file) -- kept here + // as a real, disclosed, strictly-better improvement regardless. ---- + wire [N_NODES-1:0] ready_oh; + generate + genvar gi; + for (gi = 0; gi < N_NODES; gi = gi + 1) begin : GEN_READY_OH + assign ready_oh[gi] = (node_state[gi] == ST_READY); end - end + endgenerate + + wire [NODE_IDW-1:0] first_ready_idx; + wire any_ready; + priority_encoder_lsb #(.WIDTH(N_NODES)) u_ready_penc ( + .in(ready_oh), .idx(first_ready_idx), .valid(any_ready) + ); // ---- FPGA_DATA_READY support (see any_pending port comment above). // Originally a combinational OR-reduce over node_state[0:N_NODES-1]