rtl: scaffold InterChipFabric skeleton + chiseltest bench #82
No reviewers
Labels
No labels
adr
agent:blocked-ci
agent:blocked-human
agent:blocked-resolver
agent:done
agent:in-progress
agent:no-touch
agent:pinged
agent:pr-open
agent:queued
agent:wip
area:board
area:funding
area:infra
area:phy
area:poplink
area:rtl
area:software
area:supply-chain
area:verification
ci-failed
ci-timeout
docs
do-not-merge
human-approved
needs-human-approval
needs-rebase
needs-triage
phase:1
ready-for-review
review:findings
review:pass
risk:tripwire
swarm:quarantined
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Fluid/fluidpop-v1!82
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "auto/issue-53-20260526T001906Z_issue53"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implements the Chisel skeleton for
InterChipFabricperrtl/src/pop/specs/InterChipFabric.SPEC.md, satisfying thePLAN.md §12.3 step-1/step-2 floor (module elaborates; chiseltest
harness exists).
Files
rtl/src/pop/InterChipFabric.scala— module + IO bundle groupedon the four bullets of SPEC §Interface (N/S/E/W port quad per
ADR-010, per-VC credit/return, PHY-facing PopLinkPHYIO attach per
§13.3, routing/telemetry CSR window). All sub-bundles are empty
placeholders tied to
DontCare. Each TBD is annotated with theSPEC § it traces back to.
rtl/tests/InterChipFabric/InterChipFabricSpec.scala— chiseltestAnyFlatSpecwith the elaborate-only assertion.Local validation
sbt compileandsbt 'testOnly pop.InterChipFabricSpec'bothgreen under the standalone
rtl/build (Scala 2.13.12 / Chisel6.5.0 / chiseltest 6.0.0, pinned to Chipyard 1.13.0). The
elaborate test passes.
Out of scope
Per the issue: no behaviour, no golden model, no coverage. No SPEC
edits. No invented widths / port counts / VC count / CSR map for
spec
TBDitems — they remainDontCarewith// TBD per SPEC §…comments. Follow-ups will lift each facet once the SPEC promotes
out of stub and once the PopLinkPHY behavioral skeleton (issue #51)
publishes the canonical
PopLinkPHYIObundle type.Closes #53
VERDICT: FINDINGS
PR #82 adds the §12.3 step-1 scaffold for
InterChipFabric— module + chiseltest harness — and is mechanically clean: SPDX headers correct (CHARRUA-1.2for the RTL source,AGPL-3.0-or-laterfor the test), both files inpackage pop, IO bundle mirrors the four bullets of SPEC §Interface (ports,credits,phy,csr) as empty sub-bundles tied toDontCare, each TBD field annotated with the SPEC § it tracks, no widths/VC counts/flit layouts/CSR map invented, chiseltest bench has the prescribedit should "elaborate"floor, scope matches Issue #53, no AI attribution, no off-limits paths. The deferral ofPopLinkPHYIOmaterialisation is justified honestly (canonical bundle awaits Issue #51 / behavioral PHY skeleton). One inline comment should be checked before merge: theVcCreditIOblock attributes a quoted phrase —ADR-010 fixes "deadlock-free via XY routing + 2 VCs"— to ADR-010, but the SPEC's own ADR-010 reference is described only as "2D-torus topology" and the SPEC §Behavior carries VC allocation as TBD. The "2 VCs" is a specific number that may or may not be in ADR-010's actual text, and the same comment contradicts itself by saying "the bundle stays parametrised until §Behavior commits the VC count" — if ADR-010 already fixes 2 VCs, the bundle wouldn't need to wait on §Behavior. May be incorrect — please verify ADR-010 actually contains the "2 VCs" decision.Findings
rtl/src/pop/InterChipFabric.scala(inline comment insideclass VcCreditIO) — The comment puts a quoted string"deadlock-free via XY routing + 2 VCs"next toADR-010 fixes, attributing the specific VC count of 2 to ADR-010. The SPEC's own Plan ref describes ADR-010 only as "2D-torus topology"; SPEC §Behavior carries VC allocation as TBD. The same comment then says the bundle "stays parametrised until §Behavior commits the VC count" — which contradicts the preceding clause that says ADR-010 already commits "2 VCs". Either ADR-010 contains the 2-VC fix (in which case the bundle is not still waiting on §Behavior for that number, and the comment should be tightened) or it doesn't (in which case the "2 VCs" is a fabricated specific number per rule 7). May be incorrect — please verify ADR-010's actual text. Suggested fix if ADR-010 doesn't cover VC count:// TBD per SPEC §Interface — per-VC credit / return interface (number of VCs, credit counter width, return-grouping policy). ADR-010 mandates dimension-ordered routing with VC partitioning for deadlock-freedom; the VC count itself is deferred to §Behavior. Bundle stays parametrised until §Behavior and §Invariants pin both the VC count and the credit-replenishment latency bound.VERDICT: PASS
PR #82 has been resubmitted at a new head SHA (
abdb0755...vs the priorb6537cd3...) with my earlier finding addressed. TheVcCreditIOinline comment inrtl/src/pop/InterChipFabric.scalahas been rewritten from the self-contradictory version I flagged previously — which simultaneously claimedADR-010 fixes "deadlock-free via XY routing + 2 VCs"and that the bundle stays parametrised pending§Behavior— to a clean conservative deferral:ADR-010 mandates 2D-torus topology with deadlock-free dimension-ordered routing; the VC count itself is not pinned by ADR-010 and stays parametrised until §Behavior commits a concrete number and §Invariants commits the credit-replenishment latency bound. This removes the self-contradiction and matches the exact fix-suggestion shape I proposed. The companion SPEC (InterChipFabric.SPEC.md) is now at Draft status (promoted by PR #91, which I PASSed and which retroactively confirmed that "2 VCs" does appear in both PLAN.md §3.5 and ADR-010 Consequences), so the current inline comment is in fact slightly under-attributing to ADR-010 — but that's a conservative direction that under-commits rather than over-commits, and it's no longer self-contradictory. Mechanical guardrails all pass: SPDXCHARRUA-1.2for the RTL source andAGPL-3.0-or-laterfor the test, both inpackage pop, IO bundle mirrors the four bullets of SPEC §Interface (ports/credits/phy/csr) as emptyDontCare-tied sub-bundles, each TBD annotated with the SPEC § it tracks, no widths/VC counts/flit layouts/CSR map invented, chiseltest bench has the prescribedit should "elaborate"floor, scope matches Issue #53 (two files added), no AI attribution, no off-limits paths, no Chisel module rules violated. The deferral ofPopLinkPHYIOmaterialisation remains honestly tied to Issue #51's behavioral-PHY skeleton.Findings
None.
CI green (head
abdb0755af), auto-approvingabdb0755af83403633feCI green (head
83403633fe), auto-approving