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

exp: layer_prefetch_ctrl.v, real synthesizable RTL for layer-weight-reuse prefetch, fixes a real address-truncation bug (EXP-0057b)

Browse Source 
Built the real FSM version of EXP-0057's own task-based prefetch
pattern (bulk-sequential layer fetch via sdram_controller_openrow.v
into layer_weight_buffer.v), so it's an actual instantiable module,
not just a simulation convenience.

Found and fixed a real bug in the process: cur_fill_addr's own address
arithmetic bit-sliced BYTES_PER_BURST down to too few bits
(BYTES_PER_BURST[BIDXW-1:0]), silently truncating 16 to 0 -- every
burst's bytes landed at fill offset 0-15 instead of their real
position, overwriting each other (only each layer's last burst
survived). Root cause: misapplied a widening idiom used safely
elsewhere in this codebase to a case where the target width was
actually too small. Found via a standalone control-flow debug test
first, then tracing data once control-flow was cleared.

Verified: 8192/8192 bit-exact, 0 errors (was 512/8192 before the fix)
through the real controller + SDRAM model, 16 layers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUG92aM9m68TRc4rG55BcC
This commit is contained in:
michele
2026-09-16 12:25:32 +02:00
co-authored by Claude Sonnet 5
parent 1ce78dff6e
commit 49b25f6eb6
3 changed files with 420 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hardware/v2/logs/experiments.log
+55
View File
@@ -3482,3 +3482,58 @@ production-ready. New files (additive only):
hardware/v2/rtl/layer_weight_buffer.v,
hardware/v2/sim/tb_layer_weight_buffer.v,
hardware/v2/sim/tb_layer_reuse_vs_zero_reuse.v.
EXP-0057b -- layer_prefetch_ctrl.v: real synthesizable RTL for the
layer-reuse prefetch pattern, real bug found and fixed (2026-09-16)
DATE: 2026-09-16
CONTEXT: EXP-0057's own 7.16x real measured speedup was driven by a
testbench TASK (prefetch_layer), not synthesizable RTL. Built
layer_prefetch_ctrl.v -- a real FSM that drives sdram_controller_
openrow.v's own req/wr/addr contract to bulk-fetch one layer into
layer_weight_buffer.v -- so the mechanism is actually instantiable in
a real design, not just a simulation convenience.
BUG FOUND (real, in the RTL, not the testbench): cur_fill_addr's own
address arithmetic used `BYTES_PER_BURST[BIDXW-1:0]` -- a bit-select
that TRUNCATED the 16-byte-per-burst constant down to BIDXW=3 bits,
silently evaluating to 0. Every burst's drained bytes landed in fill
addresses 0-15 instead of their real offset within the layer,
overwriting each other -- only the LAST burst of each layer survived.
Symptom: layer 0 always correct (its own fill happened to line up by
construction), every layer after showed only its last 16 bytes
correct and the rest reading back as 0 (never written). Two sibling
instances of the same pattern (`BYTES_PER_BURST[DIDXW-1:0]-1`,
`BURSTS_PER_LAYER[BIDXW-1:0]-1`) turned out to be harmless by
coincidence (power-of-2 modular-underflow identity happens to produce
the right N-1 value for THESE specific widths) but were cleaned up
anyway rather than left as a latent landmine for a future non-power-
of-2 parameter change. Root cause of reaching for the wrong pattern in
the first place: misapplied a WIDENING idiom seen elsewhere in this
codebase (e.g. `BURST_LEN[ADDR_WIDTH-1:0]`, safe because the target
width is LARGER than needed) to a case where the target width was
SMALLER than needed -- the same bit-select syntax means something
different depending on which direction the width mismatch goes.
Found via an isolated standalone-sequential debug testbench first
(confirmed the FSM's own busy/done control-flow was correct across
repeated invocations) followed by tracing the actual DATA once
control-flow was cleared as a suspect -- not by staring at the RTL in
isolation.
RESULT (tb_layer_prefetch_ctrl.v, real sdram_controller_openrow.v +
sdram_model.v, 16 layers x 4 reuses, sequential -- no double-buffer
overlap in THIS specific testbench, see its own header for why):
8192/8192 bit-exact, 0 errors, after the fix (was 512/8192 before,
i.e. only layer 0 correct). The double-buffered OVERLAPPED performance
number (7.16x) itself was already established via EXP-0057's own
task-based driver and is not re-derived here -- this experiment's own
job was confirming the real RTL controller composes correctly with
layer_weight_buffer.v end-to-end, which it now does.
next_action: layer_prefetch_ctrl.v + layer_weight_buffer.v are now
both real, verified, synthesizable building blocks for a weight-
stationary conv-style dataflow -- wiring them into the real per-slot
compute path (neural_processor.v) with a real conv-shaped benchmark
remains the next real integration step, not done here. New files
(additive only): hardware/v2/rtl/layer_prefetch_ctrl.v,
hardware/v2/sim/tb_layer_prefetch_ctrl.v.
hardware/v2/rtl/layer_prefetch_ctrl.v
+164
View File
@@ -0,0 +1,164 @@
`timescale 1ns/1ps
// ============================================================
// EXP-0057 -- layer prefetch controller: bulk-sequential fetch of one
// layer's weights from the real SDRAM controller (sdram_controller_
// openrow.v's own req/wr/addr/wdata/wmask -> rdata/ready/busy
// contract, BURST_LEN words per transaction) into a layer_weight_
// buffer.v's inactive side. Real RTL version of the exact access
// pattern tb_layer_reuse_vs_zero_reuse.v's own prefetch_layer task
// already measured (7.16x real memory-side speedup vs zero-reuse,
// same hardware, see that testbench's own header).
//
// One layer = LAYER_BYTES bytes, fetched as LAYER_BYTES/(2*BURST_LEN)
// back-to-back BURST_LEN-word transactions starting at layer_base
// (word address). Sequential -> lands in the SAME open row for any
// layer that fits within one row (1024 columns = 256 tile-blocks at
// BURST_LEN=8 -- true for any real layer size this project's own
// target models use), so this composes directly with EXP-0054's
// open-row policy without needing anything special here.
//
// Each captured burst (ctrl_rdata, 16*BURST_LEN bits) is LATCHED
// locally before draining -- does not assume the controller holds
// rdata stable beyond the cycle `ready` pulses (its own documented
// contract is "valid the same cycle ready pulses", nothing more).
// Drained one byte/cycle via a flat byte-index counter (drain_cnt)
// indexing directly into the latched burst -- no separate word/byte
// sub-counters to keep in sync, deliberately simpler than a first
// draft of this module that tracked them separately and was harder to
// convince correct by inspection.
// ============================================================
module layer_prefetch_ctrl #(
parameter DATA_WIDTH = 8,
parameter LAYER_BYTES = 128,
parameter BURST_LEN = 8,
parameter ADDR_WIDTH = 25, // matches sdram_controller_openrow.v's own word-address convention
parameter BUFADDRW = (LAYER_BYTES <= 1) ? 1 : $clog2(LAYER_BYTES)
)(
input wire clk,
input wire rst,
// ---- job control ----
input wire start, // pulse: begin fetching `layer_base` into the inactive buffer
input wire [ADDR_WIDTH-1:0] layer_base, // word address of this layer's weights in SDRAM
output reg busy,
output reg done, // pulse: matches layer_weight_buffer.v's own fill_done
// ---- layer_weight_buffer.v fill side ----
output reg fill_we,
output reg [BUFADDRW-1:0] fill_addr,
output reg [DATA_WIDTH-1:0] fill_data,
// ---- sdram_controller_openrow.v (or plain sdram_controller.v --
// identical port contract) ----
output reg ctrl_req,
output wire ctrl_wr, // always 0: read-only
output reg [ADDR_WIDTH-1:0] ctrl_addr,
output wire [16*BURST_LEN-1:0] ctrl_wdata, // unused (read-only), tied off
output wire [2*BURST_LEN-1:0] ctrl_wmask, // unused (read-only), tied off
input wire [16*BURST_LEN-1:0] ctrl_rdata,
input wire ctrl_ready,
input wire ctrl_busy
);
localparam BYTES_PER_BURST = 2*BURST_LEN;
localparam BURSTS_PER_LAYER = LAYER_BYTES/BYTES_PER_BURST;
localparam BIDXW = (BURSTS_PER_LAYER <= 1) ? 1 : $clog2(BURSTS_PER_LAYER);
localparam DIDXW = $clog2(BYTES_PER_BURST);
assign ctrl_wr = 1'b0;
assign ctrl_wdata = {(16*BURST_LEN){1'b0}};
assign ctrl_wmask = {(2*BURST_LEN){1'b0}};
localparam S_IDLE = 3'd0,
S_WAIT = 3'd1,
S_DRAIN = 3'd2,
S_TAIL = 3'd3;
reg [2:0] state;
reg [BIDXW-1:0] burst_idx;
reg [DIDXW-1:0] drain_cnt;
reg [ADDR_WIDTH-1:0] base_lat;
reg [16*BURST_LEN-1:0] burst_lat;
// combinational: which byte of the layer is currently being drained
wire [BUFADDRW-1:0] cur_fill_addr = burst_idx * BYTES_PER_BURST + drain_cnt;
always @(posedge clk) begin
if (rst) begin
state <= S_IDLE;
busy <= 1'b0;
done <= 1'b0;
fill_we <= 1'b0;
fill_addr <= {BUFADDRW{1'b0}};
fill_data <= {DATA_WIDTH{1'b0}};
ctrl_req <= 1'b0;
ctrl_addr <= {ADDR_WIDTH{1'b0}};
burst_idx <= {BIDXW{1'b0}};
drain_cnt <= {DIDXW{1'b0}};
base_lat <= {ADDR_WIDTH{1'b0}};
burst_lat <= {(16*BURST_LEN){1'b0}};
end else begin
ctrl_req <= 1'b0;
fill_we <= 1'b0;
done <= 1'b0;
case (state)
S_IDLE: begin
busy <= 1'b0;
if (start) begin
busy <= 1'b1;
base_lat <= layer_base;
burst_idx <= {BIDXW{1'b0}};
ctrl_req <= 1'b1;
ctrl_addr <= layer_base;
state <= S_WAIT;
end
end
S_WAIT: begin
if (ctrl_ready) begin
burst_lat <= ctrl_rdata;
drain_cnt <= {DIDXW{1'b0}};
state <= S_DRAIN;
end
end
S_DRAIN: begin
fill_we <= 1'b1;
fill_addr <= cur_fill_addr;
fill_data <= burst_lat[drain_cnt*8 +: 8];
if (drain_cnt == BYTES_PER_BURST - 1) begin
// this cycle drains the LAST byte of this burst
if (burst_idx == BURSTS_PER_LAYER - 1) begin
// last burst of the layer too -- one more
// cycle for this final fill_we to land, then done
state <= S_IDLE; // will be overridden below to a tail state
end else begin
burst_idx <= burst_idx + 1'b1;
ctrl_req <= 1'b1;
ctrl_addr <= base_lat + ((burst_idx + 1'b1) * BURST_LEN[ADDR_WIDTH-1:0]);
state <= S_WAIT;
end
end else begin
drain_cnt <= drain_cnt + 1'b1;
end
if (drain_cnt == BYTES_PER_BURST - 1 &&
burst_idx == BURSTS_PER_LAYER - 1) begin
state <= S_TAIL;
end
end
S_TAIL: begin
// the last fill_we (asserted combinationally in the
// S_DRAIN cycle above) is landing on THIS clock edge's
// rising edge as far as layer_weight_buffer.v is
// concerned (fill_we/_addr/_data were registered
// outputs of the previous cycle) -- signal done now.
busy <= 1'b0;
done <= 1'b1;
state <= S_IDLE;
end
default: state <= S_IDLE;
endcase
end
end
endmodule
hardware/v2/sim/tb_layer_prefetch_ctrl.v
+201
View File
@@ -0,0 +1,201 @@
`timescale 1ns/1ps
// ============================================================
// EXP-0057 -- real-RTL version of tb_layer_reuse_vs_zero_reuse.v's
// own prefetch_layer task: layer_prefetch_ctrl.v (real synthesizable
// FSM) driving layer_weight_buffer.v through the real sdram_
// controller_openrow.v + sdram_model.v. Same golden pattern, same
// L=16 layers, verifies bit-exact AND reports real cycles/layer for
// direct comparison against the task-based measurement (3777 cycles
// / 16 layers = ~236 cycles/layer average) already logged in
// experiments.log EXP-0057.
// ============================================================
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;
localparam CLK_FREQ_MHZ = 64;
localparam CLK_PERIOD_NS = 1000.0/CLK_FREQ_MHZ;
localparam LAYER_BYTES = 128;
localparam L = 16;
localparam WORDS_PER_LAYER = LAYER_BYTES/2;
reg clk = 0;
always #(CLK_PERIOD_NS/2.0) clk = ~clk;
reg rst;
integer cyc;
always @(posedge clk) if (!rst) cyc <= cyc + 1;
// ---- real SDRAM controller + model ----
wire ctrl_req, ctrl_wr;
wire [ADDR_WIDTH-1:0] ctrl_addr;
wire [16*BURST_LEN-1:0] ctrl_wdata;
wire [2*BURST_LEN-1:0] ctrl_wmask;
wire [16*BURST_LEN-1:0] ctrl_rdata;
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_openrow #(
.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)
);
// separate write-capable path to preload SDRAM (reuse the same
// controller -- write and prefetch never run concurrently here)
reg wpre_req, wpre_wr;
reg [ADDR_WIDTH-1:0] wpre_addr;
reg [16*BURST_LEN-1:0] wpre_wdata;
reg pre_active;
assign ctrl_req = pre_active ? wpre_req : pf_ctrl_req;
assign ctrl_wr = pre_active ? wpre_wr : pf_ctrl_wr;
assign ctrl_addr = pre_active ? wpre_addr : pf_ctrl_addr;
assign ctrl_wdata = pre_active ? wpre_wdata : pf_ctrl_wdata;
assign ctrl_wmask = pre_active ? {(2*BURST_LEN){1'b0}} : pf_ctrl_wmask;
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);
wpre_req = 1'b1; wpre_wr = 1'b1; wpre_addr = word_addr; wpre_wdata = data;
@(posedge clk); wpre_req = 1'b0;
while (!ctrl_ready) @(posedge clk);
end
endtask
task automatic preload_sdram_layers;
integer li, bi, wb;
reg [16*BURST_LEN-1:0] burst_data;
begin
for (li = 0; li < L; li = li + 1) begin
for (bi = 0; bi < (LAYER_BYTES/(2*BURST_LEN)); bi = bi + 1) begin
for (wb = 0; wb < BURST_LEN; wb = wb + 1)
burst_data[wb*16 +: 16] = {8'(8'h20+li), 8'(bi*BURST_LEN+wb)};
sdram_write_burst((li*WORDS_PER_LAYER + bi*BURST_LEN), burst_data);
end
end
end
endtask
// ---- layer_prefetch_ctrl.v (real RTL under test) ----
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;
reg pf_start;
reg [ADDR_WIDTH-1:0] pf_layer_base;
wire pf_busy, pf_done;
wire pf_fill_we;
wire [$clog2(LAYER_BYTES)-1:0] pf_fill_addr;
wire [7:0] pf_fill_data;
layer_prefetch_ctrl #(
.DATA_WIDTH(8), .LAYER_BYTES(LAYER_BYTES), .BURST_LEN(BURST_LEN), .ADDR_WIDTH(ADDR_WIDTH)
) u_pf (
.clk(clk), .rst(rst),
.start(pf_start), .layer_base(pf_layer_base), .busy(pf_busy), .done(pf_done),
.fill_we(pf_fill_we), .fill_addr(pf_fill_addr), .fill_data(pf_fill_data),
.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)
);
// ---- layer_weight_buffer.v ----
reg [$clog2(LAYER_BYTES)-1:0] rd_addr;
wire [7:0] rd_data;
reg consume_done;
wire active_sel, swapped;
layer_weight_buffer #(.DATA_WIDTH(8), .LAYER_DEPTH(LAYER_BYTES)) u_lwb (
.clk(clk), .rst(rst),
.fill_we(pf_fill_we), .fill_addr(pf_fill_addr), .fill_data(pf_fill_data), .fill_done(pf_done),
.rd_addr(rd_addr), .rd_data(rd_data), .consume_done(consume_done),
.active_sel(active_sel), .swapped(swapped)
);
integer errors, tests, li_i, t0, total_cycles;
reg [7:0] expected;
integer r, k;
initial begin
errors = 0; tests = 0; cyc = 0;
rst = 1; pre_active = 1'b1;
wpre_req = 0; wpre_wr = 0; wpre_addr = 0; wpre_wdata = 0;
pf_start = 0; pf_layer_base = 0; rd_addr = 0; consume_done = 0;
repeat(5) @(posedge clk);
rst = 0;
@(posedge clk); while (ctrl_busy) @(posedge clk);
$display("=== preload SDRAM with %0d distinct layer patterns ===", L);
preload_sdram_layers;
pre_active = 1'b0; // hand control to layer_prefetch_ctrl.v
// NOTE: sequential (no prefetch/consume overlap) -- this test
// exists to confirm layer_prefetch_ctrl.v (real RTL) correctly
// composes with layer_weight_buffer.v end-to-end, data-wise.
// The real DOUBLE-BUFFERED (overlapped) performance benefit
// (7.16x) was already measured and verified separately via
// tb_layer_reuse_vs_zero_reuse.v's own task-based driver,
// which does not have this testbench's own fork/join
// complexity -- not re-derived here to avoid re-debugging
// testbench-only concurrency timing a second time for no new
// information.
$display("=== real-RTL prefetch + reuse, %0d layers, sequential (correctness only) ===", L);
t0 = cyc;
pf_layer_base = 0; pf_start = 1'b1; @(posedge clk); pf_start = 1'b0;
while (!pf_done) @(posedge clk);
consume_done = 1'b1; @(posedge clk); consume_done = 1'b0; // initial swap
@(posedge clk); #1;
for (li_i = 0; li_i < L; li_i = li_i + 1) begin
for (r = 0; r < 4; r = r + 1) begin
for (k = 0; k < LAYER_BYTES; k = k + 1) begin
rd_addr = k[$clog2(LAYER_BYTES)-1:0];
#1;
tests = tests + 1;
if (k[0] == 1'b0) expected = {1'b0, k[7:1]};
else expected = 8'(8'h20+li_i);
if (rd_data !== expected) begin
$display("FAIL layer=%0d reuse=%0d k=%0d: expected %h got %h", li_i, r, k, expected, rd_data);
errors = errors + 1;
end
@(posedge clk);
end
end
consume_done = 1'b1; @(posedge clk); consume_done = 1'b0;
if (li_i+1 < L) begin
pf_layer_base = (li_i+1)*WORDS_PER_LAYER;
pf_start = 1'b1; @(posedge clk); pf_start = 1'b0;
while (!pf_done) @(posedge clk);
end
@(posedge clk); #1; // let the swap settle before the next iteration reads
end
total_cycles = cyc - t0;
$display("=== RESULT: %0d/%0d bit-exact, %0d errors, %0d total cycles for %0d layers (real RTL prefetch controller) ===",
tests-errors, tests, errors, total_cycles, L);
if (errors == 0) $display("ALL TESTS PASSED (tb_layer_prefetch_ctrl)");
$finish;
end
endmodule