feat: real result-writeback engine, removes the last hard N-scaling pin blocker (EXP-0088)

Adds result_writeback.v, one instance per packed_slot.v, writing each
completed job's result directly into DDR3 at the job's own
result_addr_a/b instead of driving literal top-level pins -- the same
architectural shape as the weight-fetch path, in reverse. job_done now
means "durably in DDR3", not "captured in a register only a pin could
see". n2_system_ddr3_top.v's own s0_result_data_a/b, s1_result_data_a/b
top-level package pins are removed (and the now-dangling XDC constraint
for them), closing the real, hard scaling blocker docs/ARCHITECTURE_
ANALYSIS.md flagged since EXP-0074/0079 (8 bits x 2 lanes x N cores ->
256 pins at N=16).

Addressing reuses the exact same JOB_ADDR_WIDTH->ctrl-bus-word
truncation x_base_a/w_base already use (verified against act_tile_
fetch.v's/layer_prefetch_ctrl.v's own real code, not guessed). The host
reads results back via the already-existing READ_MEM (0x02) SPI opcode
-- no new protocol. A real EXP-0066-class bug (issuing ctrl_req before
mem_grant) was caught and fixed before ever compiling, by re-deriving
the design against act_tile_fetch.v's own proven S_MEMWAIT/S_GAP
sequencing.

Verified two ways: tb_packed_slot.v extended with a real DDR3
read-after-write check (9/9 PASS, confirms the write actually landed,
not just that job_done pulsed); tb_n2_system_ddr3.v re-run via real
xsim to confirm correct behavior under real 2-slot shared-bus
arbitration (8/8 PASS, 0 errors, consistent timing with EXP-0087's own
baseline for this workload).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUG92aM9m68TRc4rG55BcC
This commit is contained in:
2026-09-20 19:18:57 +02:00
co-authored by Claude Sonnet 5
parent 344e798ad5
commit ccaf3ee059
8 changed files with 534 additions and 51 deletions
+17 -7
View File
@@ -98,13 +98,17 @@ module n2_system_ddr3_top #(
output wire flash_mosi,
input wire flash_miso,
// ---- results (small enough to keep as real top-level pins for
// observation; NOT part of the activation-interface pin-count
// problem described below) ----
output wire signed [DATA_WIDTH-1:0] s0_result_data_a,
output wire signed [DATA_WIDTH-1:0] s0_result_data_b,
output wire signed [DATA_WIDTH-1:0] s1_result_data_a,
output wire signed [DATA_WIDTH-1:0] s1_result_data_b,
// ---- results: EXP-0088 REMOVES the literal per-slot top-level
// result pins that used to live here (s0_result_data_a/b,
// s1_result_data_a/b) -- they were flagged (docs/ARCHITECTURE_
// ANALYSIS.md S4.6/S5.3) as the exact same class of scaling mistake
// already caught once for activation data (EXP-0074): fine at N=2
// (4 pins), a hard blocker at N=16 (8 bits x 2 lanes x 16 cores =
// 256 pins on this port alone). Each packed_slot.v instance now
// writes its own result directly into DDR3 via its own internal
// result_writeback.v (see packed_slot.v's own header) -- the host
// reads results back via the already-existing READ_MEM (0x02) SPI
// opcode, no new top-level port needed at any N.
// ---- status ----
output wire ui_clk_o,
@@ -283,6 +287,12 @@ module n2_system_ddr3_top #(
wire [15:0] s0_nid_a, s0_nid_b, s1_nid_a, s1_nid_b;
wire [JOB_ADDR_WIDTH-1:0] s0_raddr_a, s0_raddr_b, s1_raddr_a, s1_raddr_b;
// EXP-0088: plain internal debug wires now (no longer top-level
// pins) -- the real result is written to DDR3 by each slot's own
// internal result_writeback.v; these remain wired from packed_
// slot.v's own output ports purely for internal observability.
wire signed [DATA_WIDTH-1:0] s0_result_data_a, s0_result_data_b;
wire signed [DATA_WIDTH-1:0] s1_result_data_a, s1_result_data_b;
// ---- activation fetch: REAL now (EXP-0079) -- each packed_slot
// instance owns its own act_tile_fetch.v internally, sharing that
+75 -13
View File
@@ -40,9 +40,16 @@
// word burst (even index low 64 bits, odd index high 64 bits) -- see
// act_tile_fetch.v's own header and docs/PHYSICAL_REALIZATION.md S4.
//
// Also disclosed: no result-writeback engine exists yet either --
// result_addr_a/b are passed through unused, for a future writeback
// stage to consume.
// RESULT WRITEBACK (EXP-0088, real, closes the gap this header used to
// disclose as deferred): result_writeback.v (u_wb) writes each
// completed job's result INTO DDR3 at result_addr_a/b, sharing this
// slot's own ctrl port the same way u_pf/u_ddrpf already do (mutually
// exclusive in time -- writeback only ever starts in S_RESULT, after
// the whole tile loop has finished). This was the last real blocker
// for scaling past a handful of slots: result_data_a/b/etc. remain
// real output ports here for debug/testbench visibility, but the
// TOP-LEVEL n2_system_ddr3_top.v no longer exposes them as literal,
// N-scaled package pins -- see result_writeback.v's own header.
//
// EVERY job re-fetches its layer from SDRAM (no resident-weight-skip
// optimization) -- correctness first; EXP-0057's own measured
@@ -122,7 +129,8 @@ module packed_slot #(
S_TILEWAIT = 4'd6,
S_OPERAND = 4'd7,
S_RESULT = 4'd8,
S_DONE = 4'd9;
S_DONE = 4'd9,
S_WRITEBACK = 4'd10;
reg [3:0] state;
reg [ADDR_WIDTH-1:0] w_base_lat, x_base_a_lat, x_base_b_lat;
@@ -185,16 +193,50 @@ module packed_slot #(
.ctrl_rdata(ctrl_rdata), .ctrl_ready(ctrl_ready), .ctrl_busy(ctrl_busy)
);
// ---- result_writeback.v (EXP-0088): real result-writeback engine,
// shares this slot's own ctrl port with u_pf/u_ddrpf above
// (mutually exclusive in time by FSM construction -- writeback only
// ever starts in S_RESULT, strictly after the whole tile loop that
// drives u_ddrpf has already finished). Writes both lanes' results
// into DDR3 at their own job-supplied result_addr_a/b -- see
// result_writeback.v's own header for the real addressing/format.
reg wb_start;
wire wb_busy, wb_done;
wire wb_mem_active;
wire wb_ctrl_req, wb_ctrl_wr;
wire [ADDR_WIDTH-2:0] wb_ctrl_addr;
wire [32*BURST_LEN-1:0] wb_ctrl_wdata;
wire [4*BURST_LEN-1:0] wb_ctrl_wmask;
result_writeback #(
.BURST_LEN(BURST_LEN), .DATA_WIDTH(DATA_WIDTH),
.JOB_ADDR_WIDTH(ADDR_WIDTH), .ADDR_WIDTH(ADDR_WIDTH-1)
) u_wb (
.clk(clk), .rst(rst),
.start(wb_start),
.result_addr_a(result_addr_a_lat), .result_addr_b(result_addr_b_lat),
.result_data_a(result_data_a), .result_data_b(result_data_b),
.result_node_id_a(result_node_id_a), .result_node_id_b(result_node_id_b),
.busy(wb_busy), .done(wb_done),
.mem_active(wb_mem_active), .mem_grant(mem_grant),
.ctrl_req(wb_ctrl_req), .ctrl_wr(wb_ctrl_wr), .ctrl_addr(wb_ctrl_addr),
.ctrl_wdata(wb_ctrl_wdata), .ctrl_wmask(wb_ctrl_wmask),
.ctrl_rdata(ctrl_rdata), .ctrl_ready(ctrl_ready), .ctrl_busy(ctrl_busy)
);
// mutually exclusive by FSM construction (weight prefetch always
// fully completes, incl. consume_done, before the tile loop that
// triggers ddrpf_job_start ever fires) -- safe to select on act_mem_active alone.
assign ctrl_req = act_mem_active ? act_ctrl_req : pf_ctrl_req;
assign ctrl_wr = act_mem_active ? act_ctrl_wr : pf_ctrl_wr;
assign ctrl_addr = act_mem_active ? act_ctrl_addr : pf_ctrl_addr;
assign ctrl_wdata = act_mem_active ? act_ctrl_wdata : pf_ctrl_wdata;
assign ctrl_wmask = act_mem_active ? act_ctrl_wmask : pf_ctrl_wmask;
// triggers ddrpf_job_start ever fires; writeback only ever starts
// in S_RESULT, strictly after that same tile loop has already
// finished) -- safe to select on act_mem_active/wb_mem_active alone.
assign ctrl_req = act_mem_active ? act_ctrl_req : (wb_mem_active ? wb_ctrl_req : pf_ctrl_req);
assign ctrl_wr = act_mem_active ? act_ctrl_wr : (wb_mem_active ? wb_ctrl_wr : pf_ctrl_wr);
assign ctrl_addr = act_mem_active ? act_ctrl_addr : (wb_mem_active ? wb_ctrl_addr : pf_ctrl_addr);
assign ctrl_wdata = act_mem_active ? act_ctrl_wdata : (wb_mem_active ? wb_ctrl_wdata : pf_ctrl_wdata);
assign ctrl_wmask = act_mem_active ? act_ctrl_wmask : (wb_mem_active ? wb_ctrl_wmask : pf_ctrl_wmask);
assign mem_active = (state == S_MEMWAIT) || (state == S_PREFETCH) || act_mem_active;
assign mem_active = (state == S_MEMWAIT) || (state == S_PREFETCH) || act_mem_active || wb_mem_active;
// ---- layer_weight_buffer.v ----
wire [BUFADDRW-1:0] lwb_rd_addr;
@@ -277,6 +319,7 @@ module packed_slot #(
operand_valid<= 1'b0;
tile_last <= 1'b0;
result_ready <= 1'b0;
wb_start <= 1'b0;
job_bias <= {DATA_WIDTH{1'b0}};
job_activation <= ACT_RELU;
tcnt <= 16'd0;
@@ -287,6 +330,7 @@ module packed_slot #(
tile_req <= 1'b0;
ddrpf_job_start <= 1'b0;
ddrpf_tile_consume <= 1'b0;
wb_start <= 1'b0;
case (state)
S_IDLE: begin
@@ -384,6 +428,17 @@ module packed_slot #(
end
end
// EXP-0088: result_data_a/b/result_node_id_a/b/
// result_addr_a_out/b_out remain real output ports
// (unchanged, still updated exactly as before -- kept
// for debug/testbench visibility), but job_done is now
// held back until the real DDR3 writeback itself
// completes (S_WRITEBACK), not just the moment
// result_valid_np fires -- the whole point of this
// module's own EXP-0088 upgrade is that job_done means
// "the result is durably in DDR3", not merely "captured
// in a register only this slot's own top-level pins
// could see".
S_RESULT: begin
if (result_valid_np) begin
result_data_a <= result_data_a_np;
@@ -393,8 +448,15 @@ module packed_slot #(
result_addr_a_out <= result_addr_a_lat;
result_addr_b_out <= result_addr_b_lat;
result_ready <= 1'b0;
job_done <= 1'b1;
state <= S_IDLE;
wb_start <= 1'b1;
state <= S_WRITEBACK;
end
end
S_WRITEBACK: begin
if (wb_done) begin
job_done <= 1'b1;
state <= S_IDLE;
end
end
+192
View File
@@ -0,0 +1,192 @@
`timescale 1ns/1ps
// ============================================================
// V3 -- result_writeback.v: real result-writeback engine, closing the
// gap disclosed since packed_slot.v's own original header ("no
// result-writeback engine exists yet either -- result_addr_a/b are
// passed through unused") and flagged as a hard scaling blocker
// (docs/ARCHITECTURE_ANALYSIS.md S4.6/S5.3): literal top-level
// result_data_a/b pins do not scale past a handful of cores (8 bits x
// 2 lanes x N cores -- at N=16 that's 256 pins on this port alone).
//
// REAL FIX: write each completed job's result INTO DDR3 at the job's
// own result_addr_a/result_addr_b (already carried through packed_
// slot.v's own interface, previously unused), reusing the SAME shared
// ctrl port packed_slot.v already time-multiplexes among its other
// sub-engines (layer_prefetch_ctrl.v / ddr_prefetch_mgr.v) -- same
// architectural shape as the weight-fetch path, in reverse. The host
// reads results back via the ALREADY-EXISTING READ_MEM (0x02) opcode
// -- no new SPI protocol needed.
//
// REAL ADDRESSING (verified against act_tile_fetch.v's/layer_
// prefetch_ctrl.v's own real address-computation code, not guessed):
// result_addr_a/b arrive in packed_slot.v's own JOB_ADDR_WIDTH=26-bit
// convention. Exactly like x_base_a/w_base already do, the LOW
// ADDR_WIDTH=25 bits (dropping the unused top/MSB headroom bit) are
// used DIRECTLY as a ctrl-bus-native 32-bit-word address -- the SAME
// address space act_tile_fetch.v's own ctrl_addr already lives in.
// ONE full 32-bit ctrl-word is written per lane:
// {node_id[15:0], 8'h00, result_data[7:0]} (low 16 bits =
// zero-extended 8-bit result value, high 16 bits = node_id).
//
// REAL, DISCLOSED HOST-FIRMWARE IMPLICATION (not yet built, same as
// this project's other disclosed host-firmware gaps, e.g. JTAG
// bit-banging): reading a written result back via the EXISTING
// READ_MEM (16-bit-word-addressed) opcode needs
// `mem_addr = result_addr[24:0]*2` for the value and
// `mem_addr = result_addr[24:0]*2 + 1` for node_id (2 host reads per
// lane, since READ_MEM's own mem_addr is 16-bit-word-granular while
// this engine writes a native 32-bit ctrl-word -- see host_mem_
// bridge.v's own header for the real reason that halving exists).
//
// WMASK CONVENTION (matches host_mem_bridge.v's own real, already-
// working pattern exactly, not reinvented): 0 = write this byte, 1 =
// masked -- the same DQM-style polarity this project's whole memory
// stack already uses end to end.
//
// TWO LANES, ONE TRANSACTION EACH, SEQUENTIAL: lane A's write
// completes fully (through its own ctrl_ready) before lane B's own
// starts -- mirrors act_tile_fetch.v's own "lane A then lane B"
// sequencing for its two burst reads, the same discipline already
// proven safe on this shared bus.
// ============================================================
module result_writeback #(
parameter BURST_LEN = 8,
parameter DATA_WIDTH = 8,
parameter JOB_ADDR_WIDTH = 26,
parameter ADDR_WIDTH = 25 // ctrl-bus-native word address, matches sdram_arbiter_n.v's own convention
)(
input wire clk,
input wire rst,
// one-shot request: pulse `start` with all fields valid the same
// cycle (matches this project's own established one-shot-pulse-
// requester discipline, EXP-0066).
input wire start,
input wire [JOB_ADDR_WIDTH-1:0] result_addr_a,
input wire [JOB_ADDR_WIDTH-1:0] result_addr_b,
input wire signed [DATA_WIDTH-1:0] result_data_a,
input wire signed [DATA_WIDTH-1:0] result_data_b,
input wire [15:0] result_node_id_a,
input wire [15:0] result_node_id_b,
output wire busy,
output reg done, // one-cycle pulse
// ---- shared ctrl port (packed_slot.v's own local mux gates this
// the same way it already gates pf_ctrl_*/act_ctrl_*) ----
output wire mem_active,
input wire mem_grant,
output reg ctrl_req,
output reg ctrl_wr,
output reg [ADDR_WIDTH-1:0] ctrl_addr,
output reg [32*BURST_LEN-1:0] ctrl_wdata,
output reg [4*BURST_LEN-1:0] ctrl_wmask,
input wire [32*BURST_LEN-1:0] ctrl_rdata,
input wire ctrl_ready,
input wire ctrl_busy
);
localparam ALIGN_BITS = $clog2(BURST_LEN); // 3: which of the BURST_LEN 32-bit words in the burst
localparam S_IDLE = 3'd0,
S_MEMWAIT = 3'd1,
S_XFER_A = 3'd2,
S_GAP = 3'd3, // wait for ctrl_busy to clear before firing lane B's request
S_XFER_B = 3'd4,
S_DONE = 3'd5;
reg [2:0] state;
reg [DATA_WIDTH-1:0] data_a_lat, data_b_lat;
reg [15:0] nid_a_lat, nid_b_lat;
reg [ADDR_WIDTH-1:0] word_addr_a_lat, word_addr_b_lat;
assign busy = (state != S_IDLE);
// real, established discipline (EXP-0066): mem_active must be
// visible to the arbiter the SAME cycle this module first wants
// the bus, i.e. as soon as it leaves S_IDLE -- not only once a
// transaction is actually in flight.
assign mem_active = (state != S_IDLE);
// real ctrl-bus-native word address: low ADDR_WIDTH bits of the
// JOB_ADDR_WIDTH job address -- the exact same truncation act_
// tile_fetch.v/layer_prefetch_ctrl.v already apply to x_base_a/
// w_base (verified against their own real code, not guessed).
wire [ADDR_WIDTH-1:0] word_addr_a = result_addr_a[ADDR_WIDTH-1:0];
wire [ADDR_WIDTH-1:0] word_addr_b = result_addr_b[ADDR_WIDTH-1:0];
always @(posedge clk) begin
if (rst) begin
state <= S_IDLE;
ctrl_req <= 1'b0;
ctrl_wr <= 1'b0;
done <= 1'b0;
end else begin
ctrl_req <= 1'b0;
done <= 1'b0;
case (state)
S_IDLE: begin
if (start) begin
data_a_lat <= result_data_a;
data_b_lat <= result_data_b;
nid_a_lat <= result_node_id_a;
nid_b_lat <= result_node_id_b;
word_addr_a_lat <= word_addr_a;
word_addr_b_lat <= word_addr_b;
state <= S_MEMWAIT;
end
end
// real, established discipline (EXP-0066): never issue
// ctrl_req before mem_grant is actually observed -- a
// blind/early ctrl_req on a shared, arbitrated bus can
// lose the request permanently.
S_MEMWAIT: begin
if (mem_grant) begin
ctrl_req <= 1'b1;
ctrl_wr <= 1'b1;
ctrl_addr <= {word_addr_a_lat[ADDR_WIDTH-1:ALIGN_BITS], {ALIGN_BITS{1'b0}}};
ctrl_wdata <= {(BURST_LEN){nid_a_lat, 8'h00, data_a_lat}};
ctrl_wmask <= ~({{(4*BURST_LEN-4){1'b0}}, 4'hF} << (word_addr_a_lat[ALIGN_BITS-1:0] * 4));
state <= S_XFER_A;
end
end
S_XFER_A: begin
if (ctrl_ready) begin
state <= S_GAP;
end
end
// mig_native_adapter.v's own S_DONE state keeps `busy`
// asserted one cycle past ctrl_ready (act_tile_fetch.v's
// own header/code already established this) -- wait for
// !ctrl_busy before firing lane B's write, instead of
// assuming back-to-back is safe.
S_GAP: begin
if (!ctrl_busy) begin
ctrl_req <= 1'b1;
ctrl_wr <= 1'b1;
ctrl_addr <= {word_addr_b_lat[ADDR_WIDTH-1:ALIGN_BITS], {ALIGN_BITS{1'b0}}};
ctrl_wdata <= {(BURST_LEN){nid_b_lat, 8'h00, data_b_lat}};
ctrl_wmask <= ~({{(4*BURST_LEN-4){1'b0}}, 4'hF} << (word_addr_b_lat[ALIGN_BITS-1:0] * 4));
state <= S_XFER_B;
end
end
S_XFER_B: begin
if (ctrl_ready) begin
state <= S_DONE;
end
end
S_DONE: begin
done <= 1'b1;
state <= S_IDLE;
end
default: state <= S_IDLE;
endcase
end
end
endmodule