spec: NPU module spec (npu.md) — CSRs, weight FIFO protocol, quantization arithmetic, result path - #8
Conversation
…tion arithmetic, result path Closes ADR-0002 Q1 (per-tensor fixed-point requantiser, round-half-away- from-zero, sat8), Q2 (32-bit accumulator, overflow bound), Q4 (descriptor/ backpressure contract for the soc_1.md §4.2 dedicated channel), Q8 (lm_head as an ordinary long descriptor). Resolves soc_1.md Q-SOC1-07 (streaming argmax result path, decided with area/bandwidth/orchestration-purity numbers). Closes SOC1-26 (malformed-descriptor error table) and SOC1-20 (compute-done-after-stream-done ordering) for this module. Issue: #5
Integrator review — request changesBoundary: clean (single file, 1.
This doesn't flip the rejection verdict (4.55 mm² is more over the 3.24 mm² core budget than claimed, so the decision in §4.4 is unaffected), but it's a cited, wrong number presented as evidence in a normative design-rejection argument, and should be corrected before this leaves draft. Please fix the figure (and re-verify the surrounding sentence's math) and re-push. |
|
@claude chief-architect: single fix per integrator review — docs/spec/npu.md:364-368, the 32 KB SRAM area is ~4.55 mm2 (16 x 284,538 um2), not 2.27 mm2; correct the figure and re-verify the sentence's arithmetic. The rejection verdict itself stands. Role-signed commit, push. |
|
Claude finished @WJiangH's task in 53s —— View job Fixed per integrator review
Fix: Verdict unaffected: the corrected figure is more over budget than originally stated, so §4.4's rejection of the on-chip-buffer alternative (in favor of streaming argmax) stands unchanged. Commit: |
Integrator review flagged docs/spec/npu.md:364-368: applying ADR-0002's OpenRAM density to 32 KB (16 x 284,538 um^2, results.md §1) gives ~4.55 mm^2, not ~2.27 mm^2. Rejection verdict in §4.4 is unaffected — the corrected figure is more over the 3.24 mm^2 core budget, not less. Co-authored-by: WJHuang <45132014+WJiangH@users.noreply.github.com>
Integrator review — request changesBoundary: clean (single file, 1.
"This alone" = the 32 KB buffer, ≈4.55 mm². Against the 3.24 mm² total core budget ( The underlying rejection verdict is still unaffected (4.55 mm² alone already exceeds the 3.24 mm² budget, so §4.4's conclusion holds) — this is a wording/arithmetic-precision finding on the just-edited sentence, not a verdict challenge. Please replace "more than twice over" with a figure that actually follows from the cited numbers (e.g. "≈1.4× the total core budget, by itself" or similar), and re-push. |
|
@claude chief-architect: one wording fix per integrator — docs/spec/npu.md:369 replace 'more than twice over' with the supported figure (≈1.4× the 3.24 mm2 core budget by itself). Only that edit. Role-signed commit, push. |
|
Claude finished @WJiangH's task in 43s —— View job Fixed per integrator review
Fix: Commit: |
docs/spec/npu.md:369 — the 32 KB on-chip-buffer rejection alternative cited ≈4.55 mm2 against a 3.24 mm2 core budget, but the prose said "more than twice over" (would need ~6.48 mm2). Corrected to the figure the numbers actually support: ≈1.4x the core budget, by itself. Rejection verdict unaffected. Co-authored-by: WJHuang <45132014+WJiangH@users.noreply.github.com>
Integrator review — mergedVerdict: clean, merged. Both prior findings are correctly resolved. Boundary: single file Gates: CI Spot-check evidence (this round):
No further findings. Merged via merge commit, branch deleted. |
Deliverable manifest
Role: chief-architect-agent-Sonnet5-medium
Issue: #5
Artifacts:
Gates:
make lint= PASS (blink) — docs-only change, no RTL touched, existing module unaffectedflow/check_boundaries.sh) = branchspec/npumatches no restrictedcasepattern (docs-only underdocs/spec/), same precedent asspec/soc-1(PR spec: SoC-1 top-level spec — block list, memory map, boot flow #4); verified by reading the script (direct execution required approval not granted in this session) and confirming the diff (docs/spec/npu.mdonly) against its logicmake sim/make formal/make synth= not applicable, no RTL/DV/formal files changedSpec refs:
(M,s)fixed-point requantiser, round-half-away-from-zero,sat8int8 clamp — §4.1, NPU-09–NPU-13), Q2 (32-bit accumulator, overflow boundK ≤ 4096— §4.1, NPU-09/10), Q4 (descriptor/backpressure contract for thesoc_1.md§4.2 dedicated weight-stream channel — §2.3, NPU-06–NPU-08), Q8 (lm_headas an ordinary long descriptor, no sequencer special-case — §4.2)soc_1.mdQ-SOC1-07 (result path: streaming argmax, chosen with area/bandwidth/orchestration-purity numbers against on-chip-buffer and CPU-streamed alternatives — §4.4, NPU-20)soc_1.mdSOC1-26 (malformed-descriptor error table — §5, NPU-21) and SOC1-20 (compute-done strictly after stream-done — §4.3, NPU-18)irq[7](soc_1.md§4.4 table) for a new NPU descriptor-error line (§2.5)Open items:
draft: 4 open items above remain (3 inherited from ADR-0002, 1 new low-priority note)Friction:
flow/check_boundaries.shrequired approval to execute directly; verified by reading the script instead (same friction noted in PR spec: SoC-1 top-level spec — block list, memory map, boot flow #4's manifest forspec/soc-1)llama2.c's actual reference quantizer (runq.c, fetched via WebFetch since it's not in-repo and not otherwise cited) against ADR-0002's costed hardware (one requantiser block, no per-group scale-table storage) — upstream's defaultGS=64grouping doesn't match the costed design; resolved by choosingGS=n(per-tensor/per-channel), a legitimate degenerate case of the same upstream algorithm, not an invented scheme, but this took real investigation rather than being derivable from in-repo docs alone1×8shape (ADR-0002 D2, sized for a future 64-bit/8 B-per-cycle port) is wider thansoc_1.md's actually-committed 32-bit/4 B-per-cycle port (SOC1-19) — not a defect,soc_1.mdalready anticipates this ("future part swap... no datapath redesign"), but it took cross-referencing D4's table against SOC1-19 to confirm the mismatch was intentional rather than a live divergence needing arbitrationSkill candidates:
.claude/skills/chief-architect/references/spec-authoring-patterns.md— when a module's quantization/numerics contract depends on an upstream open-source reference implementation's actual algorithm (not just its stated model format), fetching and citing that source directly (as done here forllama2.c/runq.c) is worth codifying as a standard step before ruling on arithmetic contracts, rather than assuming the DSE/ADR's summary is complete.claude/skills/integrator/references/citation-attribution.md— reconfirmed this run: cite ADR-0002 by its ownD/Qlabels (no numbered §), citeresults.md/profile.mdby§N(both have numbered sections) — consistent with the prior PR's citation fix