dv(npu): cocotb suite executing vplan.md against golden model - #14
Merged
Conversation
…room
Implements hw/dv/npu/{Makefile,test_npu.py}: 20 cocotb tests covering
NPU-01..23 (structural/timing rows fully closed; numeric datapath rows
closed only at the acc==0 corner reachable through the documented
interface -- see BUGS.md finding A5). Every expected value comes from
hw/dv/common/models/npu.py; hw/rtl/npu/ was never read, only referenced
by path in VERILOG_SOURCES.
BUGS.md files two findings: A5 (no CPU-visible path ever produces a
nonzero activation byte from cold reset, extending vplan's A4 -- blocks
bit-exact datapath closure and caudses the toggle-coverage gate to miss,
38.5% vs 90%) and A6 (npu.md is silent on inter-group-boundary FIFO
timing; not a spec violation, informational).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
Verdict: clean — merged.
All manifest numbers reproduced exactly. No findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deliverable manifest
Role: dv-agent-Sonnet5-medium
Issue: #13
Artifacts:
Busdriver/scoreboard class, lock-stepped againstNpuModelGates:
make sim MOD=npu= 20/20 PASSmake sim MOD=npu WS_WIDTH=64= 20/20 PASS (confirms NPU-15: 5377 cycles vs WS_WIDTH=32's 9985 cycles for the identical K=48,N=768 descriptor)make sim MOD=npu COVERAGE=1: line 95.0% (57/60, gate 90% — PASS), branch 92.1%, expr 90.8%, toggle 38.5% (1430/3714, gate 90% — FAIL, see BUGS.md A5)Spec refs: NPU-01 through NPU-23 (docs/spec/npu.md), all 23 rows exercised; NPU-20 covered via NPU-21 per vplan. NPU-08/09/11/12/14/15/16's numeric datapath rows are closed only at the acc==0 corner reachable through the documented interface — see Open items.
Open items:
npu.md/soc_1.mdgive this module no CPU-visible path to write a nonzero byte into the 2 kB activation SRAM, and normal-mode writeback (the only other write path) is multiplicative on existing content — so from a cold reset (SRAM empirically reads back all-zero on this build) no nonzero activation operand is ever reachable through the documented interface. This collapsesnpu_requant.sv's toggle coverage to 19.5% (confirmed not a stimulus-variety gap: addedtest_csr_bit_toggle_sweep, a walking-ones/zeros CSR sweep, moved the aggregate number by <1%). Needs a chief-architect ruling: either a human-approved coverage waiver fornpu_requant.sv/npu.svwith this rationale, or a futurenpu.mdrevision adding an activation-load path (would also close vplan's existing A4).npu.mdNPU-16 doesn't specify whether the sequencer's inter-output-group activation re-read has any latency. Empirically it takes one FIFO-non-consuming cycle here. No NPU-06/07 "shall" is violated either way (FIFO occupancy never exceeds 2, no data ever drops) — this suite paces word-offering off the DUT's owno_ws_readyrather than the (unaware) golden model specifically to stay correct regardless. Flagged for a possiblenpu.md§4.2 addendum / future formal NPU-07 bound.Friction:
cocotb_tools/makefiles/Makefile.deprecationsre-derivesCOCOTB_USER_COVERAGE := $(COVERAGE)whenever they differ, undoingflow/sim.mk's existing decoupling (the documented P0 finding in.claude/skills/dv-engineer/references/cocotb-2x-pitfalls.md) —COVERAGE=1alone reproduces the "coverage module not available" hard-crash the pitfalls doc already warns about, just via a different code path than the one already worked around. Worked around locally inhw/dv/npu/Makefile(re-clearCOCOTB_USER_COVERAGEafter theflow/sim.mkinclude); not fixed inflow/, per/CLAUDE.md.~/tools/oss-cad-suite(the pathflow/sim.mk/top Makefile hard-code intoPATH) wasn't present in this environment; the actual install was at a different path. Symlinked, not a code change.wb_ack/CTRL-write side effects are visible on the same edge that captures the request, not one edge later — despite NPU-03's literal "exactly one cycle after" phrasing, that's satisfied by the request being driven stable before the sampled edge, not by a second edge. (2)o_ws_readysampled the same way reflects post-edge (inclusive of that edge's own accept) occupancy, not the pre-edge value that gated the accept decision. Both are documented at length in theBusclass docstring in test_npu.py so a future reader doesn't re-derive them from scratch.Skill candidates:
.claude/skills/dv-engineer/references/cocotb-2x-pitfalls.md— add theMakefile.deprecationsCOCOTB_USER_COVERAGE re-coupling as a second, distinct manifestation of the already-documented COVERAGE/COCOTB_USER_COVERAGE crash (different trigger, same symptom, same fix shape)..claude/skills/dv-engineer/references/cocotb-2x-pitfalls.md— note the general lesson: a DUT's WB-slave outputs (ack, and by extension any single-cycle-latency status output) can be valid on the same sampled edge as the request that caused them, if the request was driven stable before that edge — don't assume "N cycles after" phrasing in a spec implies N additionalawait RisingEdge()calls beyond the one that samples the driven inputs.