michele/FPGA-Neural
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: propagate runtime-indexed-crossbar fix to slot_mem_arbiter(_wide) + dataflow_core mux

Browse Source 
DEC-0043: slot_mem_arbiter.v/slot_mem_arbiter_wide.v both still had the
exact runtime-variable-indexed part-select anti-pattern
(pending_addr[grant_idx*ADDR_WIDTH +: ADDR_WIDTH], grant_idx a runtime
register) that neural_director.v had already found and fixed once
before -- ADDR_WIDTH=26 not being a power of 2 means this synthesizes
as a real multiplier + wide crossbar, sitting right on the
arbiter<->backend boundary this project's own N=8 congestion diagnosis
names, growing with N_PORTS=N_SLOTS(+1). Also fixed the cheaper but
same-class dir_job_out_slot*16 mux in nms_dataflow_core_sdram.v,
feeding directly into dependency_manager -- this exact signal was
DEC-0042's own diagnosed N=4 critical path.

Fix: N_PORTS/N_SLOTS parallel constant-indexed comparisons (unrolled
for-loop) instead of a runtime-indexed read -- same technique already
proven in neural_director.v. Purely an internal-implementation change.

Verified bit-exact via Verilator: tb_fpga_neural_v2_top_smoke.v 11/11
PASS; tb_nms_dstress_sdram_unified.v (256-neuron stress) at both
N_SLOTS_CFG=4 and =8, 256/256 bit-exact vs golden, total_cycles
IDENTICAL to pre-fix historical values (49927/49909, exact match to
DEC-0042's own recorded numbers).

Bonus finding from the same D-Stress run (not this commit's main
point, logged for Phase 3/4): sdram_busy_cycles ~81.6% and
useful-MAC-cycle fraction HALVING from N=4 to N=8 (2.04%->1.02%) --
real existing evidence the system is memory-bound on a single SDRAM
bank well before N=8, independent of Fmax.

Re-synthesis (8-seed sweep, N=4/N=8) in progress to measure the actual
Fmax delta from this fix -- committed separately once complete.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
michele
2026-09-15 22:39:00 +00:00
co-authored by Claude Sonnet 5
parent 19ef54aea8
commit 6a46dc857f
5 changed files with 204 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hardware/v2/logs/decisions.log
+75
View File
@@ -2462,3 +2462,78 @@ 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.
DEC-0043 -- Propagated the neural_director.v runtime-indexed-crossbar
fix to slot_mem_arbiter.v/slot_mem_arbiter_wide.v and to
nms_dataflow_core_sdram.v's own director->dependency_manager mux
DATE: 2026-09-15
CONTEXT: new brief (N=8 timing closure, LFE5U-85F retarget, 4/8/16 x
1/2-bank SDRAM sweep). Phase 0 established N_SLOTS=4/8 both already
PASS at 64MHz on this session's toolchain (EXP-0049/EXP-0050) with
real margin, but RTL analysis of the arbiter<->backend boundary (the
congestion hub the brief itself names) found a real, concrete,
previously-missed instance of an anti-pattern this project had ALREADY
found and fixed once before, just not everywhere it occurs.
ROOT CAUSE (real, found by direct RTL inspection, not guessed):
neural_director.v's own header comment documents a real historical
bug -- a runtime-computed part-select (`slot_x_base[free_slot_idx*
ADDR_WIDTH +: ADDR_WIDTH] <= ...`) synthesized as an actual
MULT18X18D hard multiplier feeding a wide demux/crossbar (ADDR_WIDTH=26
is not a power of 2), collapsing worst-seed Fmax from 68.51MHz to
40-47MHz -- fixed there via N_SLOTS parallel CONSTANT-indexed
comparisons instead. The SAME exact pattern was found, unfixed, in:
- rtl/slot_mem_arbiter.v line ~162-163 and rtl/slot_mem_arbiter_
wide.v line ~163-164: `m_addr <= pending_addr[grant_idx*
ADDR_WIDTH +: ADDR_WIDTH]` where `grant_idx` is a REGISTER (the
output of the arbiter's own priority-encoder), not a constant --
sitting exactly on the arbiter<->backend boundary, whose N_PORTS
(=N_SLOTS+1 / N_SLOTS) scales with N_SLOTS.
- nms/rtl/nms_dataflow_core_sdram.v line 189: `dir_slot_node_id[
dir_job_out_slot*16 +: 16]`, feeding directly into
dependency_manager -- the multiplier here is by a power of 2 (16),
so cheaper (no real hardware multiplier), but the resulting
N_SLOTS-way runtime mux still grows with N_SLOTS, and this EXACT
signal was DEC-0042's own diagnosed N=4 critical path (neural_
director.job_out_slot -> dependency_manager.node_resolved/
node_state) -- confirmed again in this session's own N=4 seed3
critical path report (EXP-0049).
FIX: replaced all three runtime-indexed reads with N_PORTS/N_SLOTS
parallel CONSTANT-indexed comparisons (`for` loop, unrolled at
elaboration, each iteration's index is a compile-time constant) --
same technique neural_director.v itself already established.
Functionally IDENTICAL: exactly one iteration's condition is ever true
(grant_idx/dir_job_out_slot is always a single valid index when the
mux fires), so this is a pure internal-implementation change, zero
external behavior change. Files changed: rtl/slot_mem_arbiter.v,
rtl/slot_mem_arbiter_wide.v, nms/rtl/nms_dataflow_core_sdram.v.
VERIFICATION (bit-exact, real): tb_fpga_neural_v2_top_smoke.v (the
real board-level top's own smoke test, run via Verilator 5.053 with
-DSIM for the PLL behavioral bypass): 11/11 PASS, unchanged. tb_nms_
dstress_sdram_unified.v (256-neuron stress regression, the project's
main bit-exact oracle) at both N_SLOTS_CFG=4 and N_SLOTS_CFG=8: PASS,
all 256 neurons bit-exact vs golden, total_cycles IDENTICAL to the
pre-fix historical values (N=4: 49927 cycles; N=8: 49909 cycles,
matching DEC-0042's own recorded numbers exactly) -- confirms zero
behavioral/cycle-count change, as expected for an internal-
implementation-only fix.
BONUS FINDING (not this decision's main point, but real and worth
flagging for Phase 3/4): tb_nms_dstress_sdram_unified.v's own
benchmark report already shows sdram_busy_cycles ~81.6% and weight_
stall_cycles 89-94% of total_cycles*N_SLOTS at BOTH N=4 and N=8, with
the useful-MAC-cycle fraction actually HALVING from N=4 to N=8
(2.04%->1.02% of slot-cycles) -- strong existing evidence, from a
testbench this project already had, that the system is memory-bound
(single shared SDRAM channel) well before N=8, independent of Fmax.
Relevant to the brief's own Phase 4 question; see experiments.log.
STATUS: fix applied and bit-exact verified. Re-synthesis (fresh 8-seed
nextpnr-ecp5 sweep, N=4/N=8, real fpga_neural_v2_top +
v2_board_top.lpf) in progress to measure the actual Fmax delta -- see
timing.log for the result once available.
TOOLCHAIN NOTE: this session's environment had no C/C++ toolchain
(make/gcc/g++) or working Perl FindBin module, both required to build/
run Verilator-based simulations -- installed via `dnf install -y make
gcc-c++ perl-FindBin libatomic` (system packages, not project files)
to make bit-exact verification possible at all. Recorded here since it
was a real, if minor, environment gap.
hardware/v2/logs/simulation.log
+24
View File
@@ -180,3 +180,27 @@ errors: 2 real bugs found and fixed during implementation (errors.log
a new instance in the activation-cache side of memory_manager.v),
and a repeat of ERR-0009's N_SLOTS=1 zero-width replication bug
(this time in activation_cache.v itself).
[2026-09-15] DEC-0043 verification -- Verilator 5.053 (-DSIM for PLL
behavioral bypass on tb_fpga_neural_v2_top_smoke.v; plain build for
tb_nms_dstress_sdram_unified.v, -GN_SLOTS_CFG override)
test=tb_fpga_neural_v2_top_smoke, config=fpga_neural_v2_top N_SLOTS=4
(default): 11/11 PASS, unchanged.
test=tb_nms_dstress_sdram_unified (D-Stress, 256 neurons, 128 inputs
each), config=N_SLOTS_CFG=4: PASS, 256/256 bit-exact vs golden,
total_cycles=49927 (IDENTICAL to pre-fix), sdram_busy=81.56%,
weight_stall=89.37% of total_cycles*N_SLOTS, useful_mac_cycles=2.04%
of slot-cycles.
test=tb_nms_dstress_sdram_unified (D-Stress), config=N_SLOTS_CFG=8:
PASS, 256/256 bit-exact vs golden, total_cycles=49909 (IDENTICAL to
pre-fix, matches DEC-0042's own recorded N=8 cycle count exactly),
sdram_busy=81.62%, weight_stall=94.54% of total_cycles*N_SLOTS,
useful_mac_cycles=1.02% of slot-cycles (HALF of N=4's fraction --
more slots sharing one SDRAM channel does not proportionally
increase useful work).
Result: zero behavioral/cycle-count regression from the slot_mem_
arbiter.v/slot_mem_arbiter_wide.v/nms_dataflow_core_sdram.v runtime-
index fix (DEC-0043). Real, disclosed evidence the system is
memory-bound at both N=4 and N=8 on a single SDRAM bank, independent
of the arbiter fix or Fmax -- relevant to Phase 3/4 of the current
brief.
hardware/v2/nms/rtl/nms_dataflow_core_sdram.v
+23 -2
View File
@@ -186,9 +186,30 @@ module nms_dataflow_core_sdram #(
end
assign data_ready = data_ready_reg;
wire [15:0] completed_node_id_16 = dir_slot_node_id[dir_job_out_slot*16 +: 16];
// completed_node_id_16: which slot's node_id feeds dependency_manager
// this cycle, selected via N_SLOTS parallel CONSTANT-indexed reads
// (`cni` is the for-loop's own unrolled constant) instead of a
// runtime-indexed part-select (`dir_job_out_slot*16`) -- same fix
// class as neural_director.v's own write-side fix (see that file's
// header) and slot_mem_arbiter.v/slot_mem_arbiter_wide.v's grant_idx
// fix: this exact signal was DEC-0042's own diagnosed N=4 critical
// path (neural_director.job_out_slot -> dependency_manager.
// node_resolved/node_state). The multiplier here is by a power of 2
// (16), so no real hardware multiplier was involved, but the
// resulting N_SLOTS-way runtime mux still grows with N_SLOTS.
// Functionally IDENTICAL (exactly one cni matches dir_job_out_slot).
reg [15:0] completed_node_id_16_c;
integer cni;
always @(*) begin
completed_node_id_16_c = 16'h0000;
for (cni = 0; cni < N_SLOTS; cni = cni + 1) begin
if (dir_job_out_slot == cni[$clog2(N_SLOTS)-1:0]) begin
completed_node_id_16_c = dir_slot_node_id[cni*16 +: 16];
end
end
end
assign dm_producer_done_valid = dir_job_out_done;
assign dm_producer_done_node_id = completed_node_id_16[NODE_IDW-1:0];
assign dm_producer_done_node_id = completed_node_id_16_c[NODE_IDW-1:0];
// ---- NMS memory: shared Activation SRAM (replicated) + private
// Weight SRAM (packed), per DEC-0019/DEC-0020 ----
hardware/v2/rtl/slot_mem_arbiter.v
+41 -6
View File
@@ -110,6 +110,39 @@ module slot_mem_arbiter #(
end
end
// grant_idx-selected pending fields, read out via N_PORTS parallel
// CONSTANT-indexed comparisons (`gi` is the for-loop's own unrolled
// constant, not a runtime value) instead of a runtime-indexed part-
// select of a wide packed array -- same fix class already applied
// in neural_director.v (see that file's own header comment): a
// variable-indexed read/write of a wide packed array synthesizes as
// a real multiplier (index * ADDR_WIDTH, ADDR_WIDTH=26 not a power
// of 2) feeding a wide demux/crossbar, measurably worse as
// ADDR_WIDTH/N_PORTS grow -- exactly the arbiter<->backend boundary
// this project's own N=8 congestion diagnosis names. Functionally
// IDENTICAL to the old `pending_*[grant_idx]` reads (exactly one gi
// matches grant_idx whenever any_pending is set).
reg grant_wr_c, grant_lb_n_c, grant_ub_n_c;
reg [ADDR_WIDTH-1:0] grant_addr_c;
reg [15:0] grant_wdata_c;
integer gi;
always @(*) begin
grant_wr_c = 1'b0;
grant_lb_n_c = 1'b1;
grant_ub_n_c = 1'b1;
grant_addr_c = {ADDR_WIDTH{1'b0}};
grant_wdata_c = 16'h0000;
for (gi = 0; gi < N_PORTS; gi = gi + 1) begin
if (grant_idx == gi[PIDXW-1:0]) begin
grant_wr_c = pending_wr[gi];
grant_lb_n_c = pending_lb_n[gi];
grant_ub_n_c = pending_ub_n[gi];
grant_addr_c = pending_addr[gi*ADDR_WIDTH +: ADDR_WIDTH];
grant_wdata_c = pending_wdata[gi*16 +: 16];
end
end
end
integer pi;
always @(posedge clk) begin
@@ -156,12 +189,14 @@ module slot_mem_arbiter #(
if (any_pending) begin
owner <= grant_idx + 1'b1;
m_req <= 1'b1;
m_wr <= pending_wr[grant_idx];
m_lb_n <= pending_lb_n[grant_idx];
m_ub_n <= pending_ub_n[grant_idx];
m_addr <= pending_addr[grant_idx*ADDR_WIDTH +: ADDR_WIDTH];
m_wdata <= pending_wdata[grant_idx*16 +: 16];
pending[grant_idx] <= 1'b0;
m_wr <= grant_wr_c;
m_lb_n <= grant_lb_n_c;
m_ub_n <= grant_ub_n_c;
m_addr <= grant_addr_c;
m_wdata <= grant_wdata_c;
for (pi = 0; pi < N_PORTS; pi = pi + 1) begin
if (grant_idx == pi[PIDXW-1:0]) pending[pi] <= 1'b0;
end
end
end else begin
if (m_ready) begin
hardware/v2/rtl/slot_mem_arbiter_wide.v
+41 -6
View File
@@ -111,6 +111,39 @@ module slot_mem_arbiter_wide #(
end
end
// grant_idx-selected pending fields, read out via N_PORTS parallel
// CONSTANT-indexed comparisons (`gi` is the for-loop's own unrolled
// constant, not a runtime value) instead of a runtime-indexed part-
// select of a wide packed array -- same fix class already applied
// in neural_director.v (see that file's own header comment): a
// variable-indexed read/write of a wide packed array synthesizes as
// a real multiplier (index * ADDR_WIDTH, ADDR_WIDTH=26 not a power
// of 2) feeding a wide demux/crossbar, measurably worse as
// ADDR_WIDTH/N_PORTS grow -- exactly the arbiter<->backend boundary
// this project's own N=8 congestion diagnosis names. Functionally
// IDENTICAL to the old `pending_*[grant_idx]` reads (exactly one gi
// matches grant_idx whenever any_pending is set).
reg grant_wr_c, grant_lb_n_c, grant_ub_n_c;
reg [ADDR_WIDTH-1:0] grant_addr_c;
reg [DATA_WIDTH-1:0] grant_wdata_c;
integer gi;
always @(*) begin
grant_wr_c = 1'b0;
grant_lb_n_c = 1'b1;
grant_ub_n_c = 1'b1;
grant_addr_c = {ADDR_WIDTH{1'b0}};
grant_wdata_c = {DATA_WIDTH{1'b0}};
for (gi = 0; gi < N_PORTS; gi = gi + 1) begin
if (grant_idx == gi[PIDXW-1:0]) begin
grant_wr_c = pending_wr[gi];
grant_lb_n_c = pending_lb_n[gi];
grant_ub_n_c = pending_ub_n[gi];
grant_addr_c = pending_addr[gi*ADDR_WIDTH +: ADDR_WIDTH];
grant_wdata_c = pending_wdata[gi*DATA_WIDTH +: DATA_WIDTH];
end
end
end
integer pi;
always @(posedge clk) begin
@@ -157,12 +190,14 @@ module slot_mem_arbiter_wide #(
if (any_pending) begin
owner <= grant_idx + 1'b1;
m_req <= 1'b1;
m_wr <= pending_wr[grant_idx];
m_lb_n <= pending_lb_n[grant_idx];
m_ub_n <= pending_ub_n[grant_idx];
m_addr <= pending_addr[grant_idx*ADDR_WIDTH +: ADDR_WIDTH];
m_wdata <= pending_wdata[grant_idx*DATA_WIDTH +: DATA_WIDTH];
pending[grant_idx] <= 1'b0;
m_wr <= grant_wr_c;
m_lb_n <= grant_lb_n_c;
m_ub_n <= grant_ub_n_c;
m_addr <= grant_addr_c;
m_wdata <= grant_wdata_c;
for (pi = 0; pi < N_PORTS; pi = pi + 1) begin
if (grant_idx == pi[PIDXW-1:0]) pending[pi] <= 1'b0;
end
end
end else begin
if (m_ready) begin