micheleandClaude Sonnet 5
ca94083366
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUG92aM9m68TRc4rG55BcC
314 lines
15 KiB
Verilog
314 lines
15 KiB
Verilog
// ============================================================
|
|
// Neural Memory System (NMS) -- shared Activation fill controller.
|
|
//
|
|
// One instance per neural_memory_system (shared across all N_SLOTS),
|
|
// backing nms_activation_replicated.v's single broadcast-write fill
|
|
// port. Owns ONE prefetch_engine.v instance (real word-level PSRAM
|
|
// fetch, DEC-0015 convention, reused verbatim -- it is generic
|
|
// P_IN-byte-tile fetch logic, not weight-specific despite its name).
|
|
//
|
|
// Single-tag design (same honest limitation as the superseded
|
|
// hardware/v2/rtl/activation_cache.v, DEC-0016): tracks ONE resident
|
|
// x_base at a time. Refills (resident_count resets to 0, restarts
|
|
// fetching from tile 0) whenever the lowest-indexed currently-active
|
|
// slot's own x_base differs from what is resident -- correct always,
|
|
// but can thrash under interleaved, genuinely-different-x_base
|
|
// concurrent traffic; not exercised by this project's own realistic
|
|
// dense-layer workloads (shared-producer dispatch, many slots given
|
|
// the SAME x_base together).
|
|
//
|
|
// resident_count extends to the MAX n_tiles needed by any currently
|
|
// active slot that shares resident_tag (not just the reference slot
|
|
// that triggered the refill), so a later-joining slot with a deeper
|
|
// need is served without a second refill.
|
|
// ============================================================
|
|
module nms_activation_fill_ctrl_v3 #(
|
|
parameter DATA_WIDTH = 8,
|
|
parameter P_IN = 8,
|
|
parameter N_SLOTS = 4,
|
|
parameter ADDR_WIDTH = 26,
|
|
parameter MAX_TILES = 16,
|
|
// TIW indexes the SRAM fill address (0..MAX_TILES-1); CNTW is for
|
|
// resident_count, which must represent the VALUE MAX_TILES itself
|
|
// (e.g. a fully-resident 16-tile vector with MAX_TILES=16) -- one
|
|
// bit wider than TIW, same distinction/bug as
|
|
// nms_memory_manager.v's own tile_idx/wgt_fetched (see that file's
|
|
// header for the real deadlock this caused before the fix).
|
|
parameter TIW = (MAX_TILES <= 1) ? 1 : $clog2(MAX_TILES),
|
|
parameter CNTW = $clog2(MAX_TILES+1)
|
|
)(
|
|
input wire clk,
|
|
input wire rst,
|
|
|
|
// ---- per-slot job status (levels, held while that slot's job is active) ----
|
|
input wire [N_SLOTS-1:0] job_active,
|
|
input wire [N_SLOTS*ADDR_WIDTH-1:0] x_base_flat,
|
|
input wire [N_SLOTS*16-1:0] n_tiles_flat,
|
|
|
|
// ---- broadcast status (every slot compares this against its own x_base) ----
|
|
output reg [ADDR_WIDTH-1:0] resident_tag,
|
|
output reg [CNTW-1:0] resident_count,
|
|
|
|
// ---- fill port into nms_activation_replicated.v ----
|
|
output wire fill_we,
|
|
output wire [TIW-1:0] fill_addr,
|
|
output wire [DATA_WIDTH*P_IN-1:0] fill_data,
|
|
|
|
// ---- real word-level PSRAM backend (arbitrated externally) ----
|
|
output wire mem_req,
|
|
output wire mem_wr,
|
|
output wire [ADDR_WIDTH-1:0] mem_addr,
|
|
output wire [15:0] mem_wdata,
|
|
output wire mem_lb_n,
|
|
output wire mem_ub_n,
|
|
input wire [15:0] mem_rdata,
|
|
input wire mem_ready
|
|
);
|
|
|
|
integer i;
|
|
|
|
// ---- desired x_base: lowest-indexed currently-active slot (fixed
|
|
// priority -- simple, not fairness-critical here since this only
|
|
// decides which TAG to chase, not who gets bandwidth) ----
|
|
reg desired_valid;
|
|
reg [ADDR_WIDTH-1:0] desired_x_base;
|
|
|
|
always @* begin
|
|
desired_valid = 1'b0;
|
|
desired_x_base = {ADDR_WIDTH{1'b0}};
|
|
for (i = N_SLOTS-1; i >= 0; i = i - 1) begin
|
|
if (job_active[i]) begin
|
|
desired_valid = 1'b1;
|
|
desired_x_base = x_base_flat[i*ADDR_WIDTH +: ADDR_WIDTH];
|
|
end
|
|
end
|
|
end
|
|
|
|
// ---- STEP14/EXP-0029->EXP-0030: max_n_tiles computation split
|
|
// into TWO pipeline stages, since registering ONLY its final use
|
|
// (v2, DEC-0026) left the computation ITSELF as the new critical
|
|
// path (EXP-0030, N=4 Fmax=72.78MHz, still FAIL@80MHz): the
|
|
// original single-cycle logic mixed, PER SLOT, a 23-bit tag
|
|
// equality check (x_base_flat[i]==resident_tag) together with an
|
|
// N_SLOTS-wide SEQUENTIALLY-CHAINED 16-bit running-max fold (each
|
|
// iteration's update depends on the previous one) -- both
|
|
// combinational, both in the same cycle as the register that
|
|
// captures the result.
|
|
//
|
|
// Stage 1 (independent per-slot work, no chain dependency between
|
|
// slots): register a per-slot "counts toward this refill" mask
|
|
// (job_active[i] && tag-match) and, gated by that mask, each
|
|
// slot's own n_tiles value (0 if it doesn't count) -- N_SLOTS
|
|
// independent 23-bit equality checks, no data dependency between
|
|
// slots, so their combined depth does not grow with N_SLOTS the
|
|
// way a sequential fold does.
|
|
// Stage 2 (the actual reduction): fold the REGISTERED, already-
|
|
// masked per-slot values into max_n_tiles_reg -- still an
|
|
// N_SLOTS-wide sequential chain (same fold as before), but now
|
|
// operating alone, without the equality check sharing the same
|
|
// cycle.
|
|
reg [15:0] n_tiles_masked [0:N_SLOTS-1];
|
|
genvar gsi;
|
|
generate
|
|
for (gsi = 0; gsi < N_SLOTS; gsi = gsi + 1) begin : GEN_MASK
|
|
wire slot_counts = job_active[gsi] &&
|
|
(x_base_flat[gsi*ADDR_WIDTH +: ADDR_WIDTH] == resident_tag);
|
|
always @(posedge clk) begin
|
|
if (rst) n_tiles_masked[gsi] <= 16'h0;
|
|
else n_tiles_masked[gsi] <= slot_counts ? n_tiles_flat[gsi*16 +: 16] : 16'h0;
|
|
end
|
|
end
|
|
endgenerate
|
|
|
|
// Balanced binary max-tree (log2(N_SLOTS) comparison levels)
|
|
// instead of the flat N_SLOTS-wide sequential scan this file's own
|
|
// header comment above already flagged as "an N_SLOTS-wide
|
|
// sequential chain". Found and fixed this session: that chain's
|
|
// own carry-chain critical path became the DOMINANT critical path
|
|
// at N_SLOTS=8 (real nextpnr-ecp5 P&R: Fmax collapsed to ~40MHz,
|
|
// failing the 64MHz target across every measured seed). A tree
|
|
// has the SAME single-cycle combinational timing as the scan it
|
|
// replaces (max_n_tiles_reg is still registered exactly one cycle
|
|
// behind n_tiles_masked -- no FSM/latency change, purely a
|
|
// combinational-depth reduction: log2(N_SLOTS) levels instead of
|
|
// N_SLOTS).
|
|
//
|
|
// Written as explicit, uniquely-named per-level wires (NOT a
|
|
// multi-dimensional generate-indexed array) -- a first attempt
|
|
// using a shared 2D `wire max_tree[level][idx]` array triggered a
|
|
// real simulator UNOPTFLAT "circular combinational logic" warning.
|
|
// The actual dependency graph IS acyclic (level L+1 only ever
|
|
// reads level L), but that tool's array-flattening circularity
|
|
// check could not prove that for a shared 2D array; distinctly-
|
|
// named per-level wires sidestep the ambiguity entirely for both
|
|
// simulation and synthesis. N_SLOTS is a power of two for every
|
|
// real configuration this project uses (1/2/4/8); anything else
|
|
// falls back, explicitly, to the original flat scan (correct but not
|
|
// optimized) rather than silently doing the wrong thing.
|
|
reg [15:0] max_n_tiles_reg;
|
|
generate
|
|
if (N_SLOTS == 1) begin : GEN_MAXTREE_N1
|
|
always @(posedge clk) begin
|
|
if (rst) max_n_tiles_reg <= 16'h0;
|
|
else max_n_tiles_reg <= n_tiles_masked[0];
|
|
end
|
|
end else if (N_SLOTS == 2) begin : GEN_MAXTREE_N2
|
|
wire [15:0] max_final = (n_tiles_masked[0] > n_tiles_masked[1]) ? n_tiles_masked[0] : n_tiles_masked[1];
|
|
always @(posedge clk) begin
|
|
if (rst) max_n_tiles_reg <= 16'h0;
|
|
else max_n_tiles_reg <= max_final;
|
|
end
|
|
end else if (N_SLOTS == 4) begin : GEN_MAXTREE_N4
|
|
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] max_final = (m0 > m1) ? m0 : m1;
|
|
always @(posedge clk) begin
|
|
if (rst) max_n_tiles_reg <= 16'h0;
|
|
else max_n_tiles_reg <= max_final;
|
|
end
|
|
end else if (N_SLOTS == 8) begin : GEN_MAXTREE_N8
|
|
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] m01 = (m0 > m1) ? m0 : m1;
|
|
wire [15:0] m23 = (m2 > m3) ? m2 : m3;
|
|
wire [15:0] max_final = (m01 > m23) ? m01 : m23;
|
|
always @(posedge clk) begin
|
|
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;
|
|
always @* begin
|
|
max_n_tiles_comb_fallback = 16'h0;
|
|
for (j = 0; j < N_SLOTS; j = j + 1)
|
|
if (n_tiles_masked[j] > max_n_tiles_comb_fallback)
|
|
max_n_tiles_comb_fallback = n_tiles_masked[j];
|
|
end
|
|
always @(posedge clk) begin
|
|
if (rst) max_n_tiles_reg <= 16'h0;
|
|
else max_n_tiles_reg <= max_n_tiles_comb_fallback;
|
|
end
|
|
end
|
|
endgenerate
|
|
|
|
localparam ST_IDLE = 1'd0;
|
|
localparam ST_FETCH = 1'd1;
|
|
reg state;
|
|
|
|
reg pf_start;
|
|
reg [ADDR_WIDTH-1:0] pf_addr;
|
|
wire pf_busy, pf_done;
|
|
wire signed [DATA_WIDTH*P_IN-1:0] pf_tile;
|
|
|
|
prefetch_engine #(
|
|
.DATA_WIDTH(DATA_WIDTH), .P_IN(P_IN), .ADDR_WIDTH(ADDR_WIDTH)
|
|
) u_pf (
|
|
.clk(clk), .rst(rst),
|
|
.fetch_start(pf_start), .w_addr(pf_addr),
|
|
.fetch_busy(pf_busy), .fetch_done(pf_done), .tile_w(pf_tile),
|
|
.mem_req(mem_req), .mem_wr(mem_wr), .mem_addr(mem_addr), .mem_wdata(mem_wdata),
|
|
.mem_lb_n(mem_lb_n), .mem_ub_n(mem_ub_n),
|
|
.mem_rdata(mem_rdata), .mem_ready(mem_ready)
|
|
);
|
|
|
|
reg fill_we_reg;
|
|
reg [TIW-1:0] fill_addr_reg;
|
|
reg [DATA_WIDTH*P_IN-1:0] fill_data_reg;
|
|
assign fill_we = fill_we_reg;
|
|
assign fill_addr = fill_addr_reg;
|
|
assign fill_data = fill_data_reg;
|
|
|
|
always @(posedge clk) begin
|
|
if (rst) begin
|
|
resident_tag <= {ADDR_WIDTH{1'b1}}; // sentinel: matches no real x_base at reset
|
|
resident_count <= {CNTW{1'b0}};
|
|
state <= ST_IDLE;
|
|
pf_start <= 1'b0;
|
|
fill_we_reg <= 1'b0;
|
|
end else begin
|
|
pf_start <= 1'b0;
|
|
fill_we_reg <= 1'b0;
|
|
|
|
// latch a completed fetch into the replicated activation
|
|
// memory's broadcast fill port
|
|
if (pf_done) begin
|
|
fill_we_reg <= 1'b1;
|
|
fill_addr_reg <= resident_count[TIW-1:0]; // valid: gated < max_n_tiles <= MAX_TILES
|
|
fill_data_reg <= pf_tile;
|
|
resident_count <= resident_count + 1'b1;
|
|
end
|
|
|
|
// refill trigger: the reference slot wants a DIFFERENT tag,
|
|
// and the fetch engine is genuinely idle (never interrupt an
|
|
// in-flight fetch -- same discipline as memory_manager.v's
|
|
// own pf_pending guard, ERR-0006). Stays in ST_IDLE (not
|
|
// ST_FETCH): only updates resident_tag/resident_count here;
|
|
// the ST_IDLE case below is what actually issues pf_start,
|
|
// reading the NEW resident_tag starting next cycle -- this
|
|
// path must NOT itself jump to ST_FETCH without a matching
|
|
// pf_start, or the engine would sit in ST_FETCH forever
|
|
// waiting for a pf_done that was never triggered.
|
|
if (desired_valid && (desired_x_base != resident_tag) && !pf_busy && (state == ST_IDLE)) begin
|
|
resident_tag <= desired_x_base;
|
|
resident_count <= {CNTW{1'b0}};
|
|
end
|
|
|
|
case (state)
|
|
ST_IDLE: begin
|
|
// stay idle: fetching further tiles for the CURRENT
|
|
// tag (if any active slot still needs more) is
|
|
// handled below, symmetric to the refill case.
|
|
if (!pf_busy && !pf_start && (resident_count < max_n_tiles_reg) &&
|
|
desired_valid && (desired_x_base == resident_tag)) begin
|
|
pf_start <= 1'b1;
|
|
pf_addr <= resident_tag + (resident_count * P_IN[ADDR_WIDTH-1:0]);
|
|
state <= ST_FETCH;
|
|
end
|
|
end
|
|
ST_FETCH: begin
|
|
if (pf_done) begin
|
|
// resident_count already bumped above this cycle;
|
|
// decide whether more remain once back in IDLE.
|
|
state <= ST_IDLE;
|
|
end
|
|
end
|
|
default: state <= ST_IDLE;
|
|
endcase
|
|
end
|
|
end
|
|
|
|
endmodule
|