diff --git a/hardware/v2/logs/experiments.log b/hardware/v2/logs/experiments.log index 18ecbfd..9a2ef2e 100644 --- a/hardware/v2/logs/experiments.log +++ b/hardware/v2/logs/experiments.log @@ -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. diff --git a/hardware/v2/rtl/layer_prefetch_ctrl.v b/hardware/v2/rtl/layer_prefetch_ctrl.v new file mode 100644 index 0000000..23eb315 --- /dev/null +++ b/hardware/v2/rtl/layer_prefetch_ctrl.v @@ -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 diff --git a/hardware/v2/sim/tb_layer_prefetch_ctrl.v b/hardware/v2/sim/tb_layer_prefetch_ctrl.v new file mode 100644 index 0000000..215a301 --- /dev/null +++ b/hardware/v2/sim/tb_layer_prefetch_ctrl.v @@ -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