rtl(PopSoCConfig): v1 Field keys + Master/Slave Config classes #108

Merged

navigator merged 1 commit from auto/issue-105-20260526T163459Z_issue105 into main

2026-05-26 13:47:45 -03:00

navigator commented

2026-05-26 13:44:20 -03:00

Owner

Closes #105.

Implements the v1 of PopSoCConfig pinned by the Draft SPEC at rtl/src/pop/specs/PopSoCConfig.SPEC.md and PLAN.md §8.2 (custom module list).

What lands

Field key declarations under package pop (PopSolutions-local namespace per rtl/build.sbt organization := "coop.pop.fluidpop"):
- TileCountKey extends Field[Int] (ADR-001)
- ScratchpadSizeKey extends Field[Int] (ADR-004, bytes)
- FabricPortCountKey extends Field[Int] (ADR-010)
- IsMasterKey extends Field[Boolean] (ADR-008)
- VariantKey extends Field[Variant]
  Every key carries the SPEC §Interface Open-question marker the issue requires; no upstream Chipyard key names are invented.
sealed trait Variant with Edu / Pro case objects, adjacent to the keys (per the issue's no-separate-file note). The trade-off vs a Boolean encoding is annotated inline per the SPEC.
Three Config classes:
- FluidPopBaseConfig chains the variant-invariant keys to Edu defaults from ADR-001 (4 tiles), ADR-004 (256 KiB scratchpad in bytes), ADR-010 (4 PopLink ports), Variant.Edu.
- FluidPopMasterConfig overlays IsMasterKey := true (ADR-008 master, instantiates PCIeHostBridge in FluidPopSoC).
- FluidPopSlaveConfig overlays IsMasterKey := false (ADR-008 slave, gates the PCIe path).

In-file CDE shim

The Chipyard pin is not yet on the build classpath per PLAN.md §6 Week 4 and rtl/README.md ("Nothing in this tree depends on Chipyard yet"); build.sbt is untouched per the issue's AC. To still ship the Field/Config keys honestly today, PopSoCConfig.scala carries a thin in-file CDE-shaped shim (Field, Parameters, Config) that mirrors the upstream org.chipsalliance.cde.config API surface. When the Chipyard pin lands, the shim block is deleted and the imports are rewritten to org.chipsalliance.cde.config.{Field, Config, Parameters}; the key declarations, Variant and the three Config classes keep their shape.

The Config shim uses a () => PartialFunction[Field[_], Any] thunk for its overlay parameter — same shape CDE upstream uses — to defer PartialFunction instantiation past the subclass super-constructor call (Scala 2.13 forbids an anonymous PF literal as a super-arg with "Implementation restriction: requires premature access").

Tests

rtl/tests/PopSoCConfig/PopSoCConfigSpec.scala is extended (no Chisel elaboration of FluidPopSoC):

FluidPopMasterConfig(IsMasterKey) == true — ADR-008 master
FluidPopSlaveConfig(IsMasterKey) == false — ADR-008 slave
FluidPopBaseConfig resolves the four chained keys to the ADR-pinned defaults
Master / Slave inherit the chained keys from Base

Full test suite from rtl/:

$ sbt -no-colors test
[info] Total number of tests run: 8
[info] Suites: completed 5, aborted 0
[info] Tests: succeeded 8, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.

sbt -no-colors compile is also green.

Scope

Edits are bounded to:

rtl/src/pop/PopSoCConfig.scala
rtl/tests/PopSoCConfig/PopSoCConfigSpec.scala

No other files touched. The Edu / Pro vs Master / Slave orthogonality, the upstream-key-reuse choice, and the Pro-side specialisation of the chained keys remain Open per SPEC §Interface, and are deferred to the follow-up ADR named there.

Closes #105. Implements the v1 of `PopSoCConfig` pinned by the Draft SPEC at `rtl/src/pop/specs/PopSoCConfig.SPEC.md` and PLAN.md §8.2 (custom module list). ## What lands - Field key declarations under package `pop` (PopSolutions-local namespace per `rtl/build.sbt` `organization := "coop.pop.fluidpop"`): - `TileCountKey extends Field[Int]` (ADR-001) - `ScratchpadSizeKey extends Field[Int]` (ADR-004, bytes) - `FabricPortCountKey extends Field[Int]` (ADR-010) - `IsMasterKey extends Field[Boolean]` (ADR-008) - `VariantKey extends Field[Variant]` Every key carries the SPEC §Interface Open-question marker the issue requires; no upstream Chipyard key names are invented. - `sealed trait Variant` with `Edu` / `Pro` case objects, adjacent to the keys (per the issue's no-separate-file note). The trade-off vs a Boolean encoding is annotated inline per the SPEC. - Three `Config` classes: - `FluidPopBaseConfig` chains the variant-invariant keys to Edu defaults from ADR-001 (4 tiles), ADR-004 (256 KiB scratchpad in bytes), ADR-010 (4 PopLink ports), `Variant.Edu`. - `FluidPopMasterConfig` overlays `IsMasterKey := true` (ADR-008 master, instantiates PCIeHostBridge in `FluidPopSoC`). - `FluidPopSlaveConfig` overlays `IsMasterKey := false` (ADR-008 slave, gates the PCIe path). ## In-file CDE shim The Chipyard pin is not yet on the build classpath per PLAN.md §6 Week 4 and `rtl/README.md` ("Nothing in this tree depends on Chipyard yet"); `build.sbt` is untouched per the issue's AC. To still ship the Field/Config keys honestly today, `PopSoCConfig.scala` carries a thin in-file CDE-shaped shim (`Field`, `Parameters`, `Config`) that mirrors the upstream `org.chipsalliance.cde.config` API surface. When the Chipyard pin lands, the shim block is deleted and the imports are rewritten to `org.chipsalliance.cde.config.{Field, Config, Parameters}`; the key declarations, `Variant` and the three Config classes keep their shape. The `Config` shim uses a `() => PartialFunction[Field[_], Any]` thunk for its overlay parameter — same shape CDE upstream uses — to defer PartialFunction instantiation past the subclass super-constructor call (Scala 2.13 forbids an anonymous PF literal as a super-arg with "Implementation restriction: requires premature access"). ## Tests `rtl/tests/PopSoCConfig/PopSoCConfigSpec.scala` is extended (no Chisel elaboration of `FluidPopSoC`): - `FluidPopMasterConfig(IsMasterKey) == true` — ADR-008 master - `FluidPopSlaveConfig(IsMasterKey) == false` — ADR-008 slave - `FluidPopBaseConfig` resolves the four chained keys to the ADR-pinned defaults - Master / Slave inherit the chained keys from Base Full test suite from `rtl/`: ``` $ sbt -no-colors test [info] Total number of tests run: 8 [info] Suites: completed 5, aborted 0 [info] Tests: succeeded 8, failed 0, canceled 0, ignored 0, pending 0 [info] All tests passed. ``` `sbt -no-colors compile` is also green. ## Scope Edits are bounded to: - `rtl/src/pop/PopSoCConfig.scala` - `rtl/tests/PopSoCConfig/PopSoCConfigSpec.scala` No other files touched. The Edu / Pro vs Master / Slave orthogonality, the upstream-key-reuse choice, and the Pro-side specialisation of the chained keys remain Open per SPEC §Interface, and are deferred to the follow-up ADR named there.

navigator added 1 commit

2026-05-26 13:44:20 -03:00

rtl(PopSoCConfig): v1 Field keys + Master/Slave Config classes

build / scalafmt-check (pull_request) Successful in 4s

Details

build / sbt-compile (pull_request) Successful in 3s

Details

build / shell-lint (pull_request) Successful in 11s

Details

53a3e2045a

Promote `PopSoCConfig.scala` from §12.3 elaboration stub to the v1 form
pinned by the Draft SPEC and PLAN.md §8.2:

- Declare PopSolutions-local Field keys (`TileCountKey`,
  `ScratchpadSizeKey`, `FabricPortCountKey`, `IsMasterKey`,
  `VariantKey`) under package `pop`. Concrete Chipyard `Config` key
  reuse remains Open per SPEC §Interface; each declaration carries the
  inline Open-question marker the issue requires.
- Add the `Variant` sealed trait (`Edu` / `Pro`) adjacent to the keys,
  per the issue's "do not promote to a separate file" note. Picking
  the sealed-trait enum form over a Boolean is the trade-off captured
  in SPEC §Interface — annotated inline.
- Ship the three Config classes:
  `FluidPopBaseConfig` chains the variant-invariant keys to the Edu
  defaults from ADR-001 / ADR-004 / ADR-010; `FluidPopMasterConfig`
  and `FluidPopSlaveConfig` overlay `IsMasterKey` per ADR-008.
- Carry an in-file CDE-shaped shim (`Field`, `Parameters`, `Config`)
  mirroring the upstream `org.chipsalliance.cde.config` surface. The
  shim is deleted and the imports are rewritten when the Chipyard pin
  lands per PLAN.md §6 Week 4 (per `rtl/README.md` "Nothing in this
  tree depends on Chipyard yet" — `build.sbt` is untouched).
- Extend `PopSoCConfigSpec` with the IsMasterKey resolution assertions
  the issue's AC requires, plus base-key chain coverage; no Chisel
  elaboration of `FluidPopSoC`.

Scope is bounded to the two files the issue authorises.

navigator added the

ready-for-review

label

2026-05-26 13:44:29 -03:00

navigator commented

2026-05-26 13:46:18 -03:00

Author

Owner

VERDICT: PASS

PR #108 lands the v1 of PopSoCConfig — Field keys, Variant sealed trait, and three Config classes — pinned by the Draft SPEC (PR #97) and PLAN.md §8.2, scoped to exactly the two files Issue #105 asks for. Mechanical guardrails pass: existing SPDX headers preserved (CHARRUA-1.2 for the RTL source, AGPL-3.0-or-later for the test), both files in package pop, no AI/Anthropic attribution, no off-limits paths (and explicitly no build.sbt edits per the issue AC), test scaffold present. Rule 4 doesn't strictly apply since this isn't a Chisel Module with an IO bundle (the SPEC itself flags "PopSoCConfig is not a chisel3.Module in the conventional sense"), but the spirit is honored: all five SPEC §Interface "build-time configuration keys" — tile count, scratchpad size, fabric port count, master-vs-slave selector, Edu-vs-Pro selector — are materialised as Field keys (TileCountKey, ScratchpadSizeKey, FabricPortCountKey, IsMasterKey, VariantKey), with the explicit Open-question inline comment the SPEC requires. Rule 6's chiseltest scaffold is appropriately replaced with Parameters-resolution tests since there's no Chisel Module to elaborate — the test now exercises the actual config-resolution semantics (IsMasterKey true for Master, false for Slave, chained defaults inherited into both fragments), which is more meaningful for a Config-fragment file. Rule 7 is meticulously handled: every default value cross-checks against a previously-approved ADR cite — TileCountKey=4 matches ADR-001 Edu (PR #92/#93/#96/#97), ScratchpadSizeKey=256*1024 matches ADR-004's 256 KiB SP per PR #92/#93 (and the PR explicitly notes capacity-only per the MultiGemminiCluster §Behavior Open on port shape), FabricPortCountKey=4 matches ADR-010 N/S/E/W (PR #87/#91/#97), IsMasterKey Master=true/Slave=false matches ADR-008 (PR #87/#94/#97). The Edu/Pro vs Master/Slave orthogonality recorded as Open in the SPEC is faithfully deferred — the PR implements the orthogonal IsMasterKey ⊕ VariantKey form and explicitly defers the four-class form (FluidPopMasterEduConfig etc.) to the follow-up ADR. The CDE shim is the one non-trivial design choice and is handled with disciplined transparency: documented as a temporary in-file placeholder (abstract class Field[T], trait Parameters, class Config) mirroring the upstream org.chipsalliance.cde.config API surface, with an explicit migration recipe (when Chipyard pin lands, delete the shim block, rewrite imports, keep everything else). The Scala 2.13 PF-literal-as-super-arg constraint that forces the () => PartialFunction[...] thunk shape is documented inline — this is genuine engineering trade-off documentation, not fabrication. The PR honestly stops at the Parameters-lookup level for tests and explicitly leaves "full Chisel elaboration of FluidPopSoC against these Configs" to a follow-up once FluidPopSoC is itself lifted from Stub. The sbt-test output (8 tests passed across 5 suites) is provided.

Findings

None.

**VERDICT:** PASS PR #108 lands the v1 of `PopSoCConfig` — Field keys, `Variant` sealed trait, and three `Config` classes — pinned by the Draft SPEC (PR #97) and PLAN.md §8.2, scoped to exactly the two files Issue #105 asks for. Mechanical guardrails pass: existing SPDX headers preserved (`CHARRUA-1.2` for the RTL source, `AGPL-3.0-or-later` for the test), both files in `package pop`, no AI/Anthropic attribution, no off-limits paths (and explicitly no `build.sbt` edits per the issue AC), test scaffold present. Rule 4 doesn't strictly apply since this isn't a Chisel Module with an IO bundle (the SPEC itself flags "PopSoCConfig is not a `chisel3.Module` in the conventional sense"), but the spirit is honored: all five SPEC §Interface "build-time configuration keys" — tile count, scratchpad size, fabric port count, master-vs-slave selector, Edu-vs-Pro selector — are materialised as Field keys (`TileCountKey`, `ScratchpadSizeKey`, `FabricPortCountKey`, `IsMasterKey`, `VariantKey`), with the explicit Open-question inline comment the SPEC requires. Rule 6's chiseltest scaffold is appropriately replaced with `Parameters`-resolution tests since there's no Chisel Module to elaborate — the test now exercises the actual config-resolution semantics (IsMasterKey true for Master, false for Slave, chained defaults inherited into both fragments), which is more meaningful for a Config-fragment file. Rule 7 is meticulously handled: every default value cross-checks against a previously-approved ADR cite — `TileCountKey=4` matches ADR-001 Edu (PR #92/#93/#96/#97), `ScratchpadSizeKey=256*1024` matches ADR-004's 256 KiB SP per PR #92/#93 (and the PR explicitly notes capacity-only per the MultiGemminiCluster §Behavior Open on port shape), `FabricPortCountKey=4` matches ADR-010 N/S/E/W (PR #87/#91/#97), `IsMasterKey` Master=true/Slave=false matches ADR-008 (PR #87/#94/#97). The Edu/Pro vs Master/Slave orthogonality recorded as Open in the SPEC is faithfully deferred — the PR implements the orthogonal `IsMasterKey ⊕ VariantKey` form and explicitly defers the four-class form (`FluidPopMasterEduConfig` etc.) to the follow-up ADR. The CDE shim is the one non-trivial design choice and is handled with disciplined transparency: documented as a temporary in-file placeholder (`abstract class Field[T]`, `trait Parameters`, `class Config`) mirroring the upstream `org.chipsalliance.cde.config` API surface, with an explicit migration recipe (when Chipyard pin lands, delete the shim block, rewrite imports, keep everything else). The Scala 2.13 PF-literal-as-super-arg constraint that forces the `() => PartialFunction[...]` thunk shape is documented inline — this is genuine engineering trade-off documentation, not fabrication. The PR honestly stops at the Parameters-lookup level for tests and explicitly leaves "full Chisel elaboration of `FluidPopSoC` against these Configs" to a follow-up once `FluidPopSoC` is itself lifted from Stub. The sbt-test output (8 tests passed across 5 suites) is provided. ## Findings _None._

navigator added the

review:pass

label

2026-05-26 13:46:18 -03:00