diff --git a/.github/workflows/rtl-lint.yml b/.github/workflows/rtl-lint.yml new file mode 100644 index 0000000..a3b9a54 --- /dev/null +++ b/.github/workflows/rtl-lint.yml @@ -0,0 +1,100 @@ +# rtl-lint — Verilator --lint-only gate for the RTL coding-style conventions. +# Implements convention #4 of docs/rtl_conventions.md: the style guide is +# ENFORCED, not just documented. atalla had the guide but not the gate. +# +# ── POSTURE: SOFT (advisory) initially ──────────────────────────────────── +# This gate is ADDITIVE and starts NON-BLOCKING so it cannot red-wall the +# existing, timing-closed blocks (which predate these conventions and are not +# yet lint-clean). It lints every per-top `.f` filelist it can find — today +# that is the worked template; it grows automatically as blocks adopt `.f` +# filelists (convention #3). Nothing here reorganizes or fails a proven block. +# +# ── HOW TO FLIP IT TO BLOCKING (do this once the RTL is lint-clean) ──────── +# 1. delete the `continue-on-error: true` line on the `lint` job below, and +# 2. set LINT_BLOCKING: "1" in the env: block below (turns the soft +# per-filelist failures into a non-zero job exit). +# Until both are done, a lint failure is reported (annotations + summary) but +# the check stays green. + +name: rtl-lint + +on: + pull_request: + branches: [rev0, main] + paths: + - "**/*.sv" + - "**/*.svh" + - "**/*.v" + - "**/*.f" + - "docs/rtl_conventions*/**" + - ".github/workflows/rtl-lint.yml" + push: + branches: [rev0] + workflow_dispatch: {} + +concurrency: + group: rtl-lint-${{ github.ref }} + cancel-in-progress: true + +jobs: + lint: + runs-on: ubuntu-latest + # SOFT GATE: remove this line to make Verilator lint blocking (see header). + continue-on-error: true + env: + # "0" = soft (report, never fail the job). "1" = fail the job on any lint error. + LINT_BLOCKING: "0" + steps: + - uses: actions/checkout@v4 + + - name: Install Verilator + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends verilator + verilator --version + + - name: Lint every per-top .f filelist + shell: bash + run: | + set -uo pipefail + # Discover all per-top filelists. `.f` files are the convention-#3 + # ordered lists that BOTH sim and hardening consume, so they define + # the lintable design set. Skip vendored/build dirs. + mapfile -t FLISTS < <(git ls-files '*.f' | grep -vE '(^|/)(sim_build|node_modules)/' || true) + + if [ "${#FLISTS[@]}" -eq 0 ]; then + echo "No .f filelists found — nothing to lint yet (expected until blocks adopt convention #3)." + exit 0 + fi + + rc=0 + for f in "${FLISTS[@]}"; do + dir=$(dirname "$f") # filelists/ lives beside include/ rtl/ + base=$(basename "$f" .f) # name == filelist stem + root=$(dirname "$dir") # the module tree root (…/ above filelists/) + echo "::group::verilator --lint-only $f (top=$base)" + # Run from the tree root so the .f's relative paths resolve. + ( cd "$root" && verilator --lint-only -Wall --top-module "$base" \ + -f "filelists/$base.f" ) || { + echo "::error file=$f::verilator lint failed for top '$base'" + rc=1 + } + echo "::endgroup::" + done + + if [ "$rc" -ne 0 ]; then + echo "### rtl-lint: Verilator reported lint errors" >> "$GITHUB_STEP_SUMMARY" + echo "Gate is currently **soft** (LINT_BLOCKING=$LINT_BLOCKING). See docs/rtl_conventions.md §4." >> "$GITHUB_STEP_SUMMARY" + else + echo "### rtl-lint: all filelists lint-clean ✅" >> "$GITHUB_STEP_SUMMARY" + fi + + if [ "$LINT_BLOCKING" = "1" ]; then + exit "$rc" + fi + exit 0 + + - name: Advisory RTL-conventions structure scan + # Never fails; mirrors the local `make`-side advisory. Pure signal for reviewers. + run: | + python3 scripts/check_block_structure.py --check-rtl-conventions || true diff --git a/DECISIONS.md b/DECISIONS.md index 4ced357..dc70c41 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -4,6 +4,21 @@ # DECISIONS — chip-wide (do not re-litigate unless the premise changed) +- **RTL-organization conventions adopted (mirrored tree + interface taxonomy + ordered `.f` + + lint-gated style)** · borrowed/trimmed from the peer teaching-tapeout repo Purdue-SoCET/atalla to + make the repo legible for the incoming cohort: (1) mirrored `include↔rtl↔tb` trees sharing a + per-module subpath so ONE generic target (`make {lint,test} MOD=`) builds any module by name; + (2) SV interface taxonomy — `_if.sv` + shared `_pkg` + `_params.svh`, `modport + _` named for the two modules it connects; (3) one ordered per-top `.f` consumed by BOTH sim + and LibreLane so they never disagree on compile order; (4) a SHALL/SHOULD/MAY style guide ENFORCED + by a Verilator `--lint-only` CI gate (the enforcement is the point — atalla had the guide, not the + gate). **ADDITIVE ONLY:** no existing block was reorganized; conventions apply to NEW modules and + are the migration target for existing ones. Spec + runnable template (`make test MOD=regadd` passes + under iverilog 12 + cocotb 2.0.1) in `docs/rtl_conventions.md` + `docs/rtl_conventions_template/`; + structure check gains opt-in advisory `--check-rtl-conventions`; lint CI + (`.github/workflows/rtl-lint.yml`) ships SOFT (non-blocking) so it can't red-wall proven blocks. + Migrating existing blocks to the mirrored tree is proposed but deliberately NOT done here. · + 2026-08-01. - **RAMP (arXiv 2603.17891) evaluated and REJECTED for Lambda** · both of its ideas fail once measured against the chip's real quant. (1) Per-layer weight bit allocation: Qwen2-1.5B layers are near-uniform in weight sensitivity (spread 1.17×) → only ~1.6% gain, and it costs +34% MAC diff --git a/docs/rtl_conventions.md b/docs/rtl_conventions.md new file mode 100644 index 0000000..27b9cbd --- /dev/null +++ b/docs/rtl_conventions.md @@ -0,0 +1,296 @@ +# Lambda RTL organization conventions + +> Status: **adopted 2026-08-01** (see root `DECISIONS.md`). **Additive** — these +> conventions apply to **new** modules and are the *target* for existing blocks; +> the proven, timing-closed RTL in `src/blocks/{kve,tiu,acu,…}` is **not** being +> reorganized under this doc. Migration of an existing block is a separate, +> opt-in effort (see [§6](#6-migration--enforcement-posture)). + +These conventions are borrowed and trimmed from the peer teaching-tapeout repo +**Purdue-SoCET/atalla** and adapted to Lambda's monorepo + open-PDK flow +(iverilog + cocotb + yosys + LibreLane). The goal is **legibility for a large +incoming cohort**: a new member should be able to find any module's interface, +implementation, and testbench by name, build it with one generic command, and +know the house style — without reading a bespoke Makefile per block. + +A **runnable worked example** of everything here lives in +[`rtl_conventions_template/`](./rtl_conventions_template/) (`make test MOD=regadd` +is verified passing under Icarus 12 + cocotb 2.0.1). + +The four conventions: + +1. [Mirrored `include ↔ rtl ↔ tb` tree + generic build](#1-the-mirrored-tree--generic-build-the-big-one) +2. [SV interface taxonomy (`_if` / `_pkg` / `_params`, `modport a_b`)](#2-interface-taxonomy) +3. [Per-top ordered `.f` filelists (one file, sim = harden)](#3-per-top-ordered-f-filelists) +4. [SHALL/SHOULD/MAY coding style, enforced by a Verilator lint gate](#4-rtl-coding-style-shallshouldmay--enforced) + +--- + +## 1. The mirrored tree + generic build (the big one) + +**Rule.** Within a block's `rtl/` area, a module `` occupies the **same +subpath** in three parallel trees: + +``` +/rtl/ +├── include// # what the module PRESENTS: interface + params +│ ├── _if.sv +│ └── _params.svh +├── rtl// # what the module IS: the implementation +│ └── .sv +└── tb// # how the module is PROVEN: its testbench + ├── _tb_top.sv # (sim-only harness, if the sim needs one) + └── test_.py # cocotb, self-checking, multi-seed +``` + +The **shared subpath** (`/`, and deeper: `//`) is the whole +trick. Because `include/`, `rtl/`, and `tb/` agree on it, a **single generic +build target** can locate every artifact for a module from just its **name**: + +``` +make test MOD= # sim tb//test_.py against rtl//.sv +make lint MOD= # lint the module's design filelist +``` + +No per-module Makefile edits, ever. Adding a module = drop files into the three +mirrored subpaths + add one filelist. Compare atalla's `make test folder=…` +(keyed on the shared folder); Lambda keys on the **module name** `MOD=…` because +our filelists are per-top and named for the top. + +### Why this specific shape + +* **Find-by-name.** A newcomer asked "where's the softmax unit's interface?" + answers themselves: `include/vecu_softmax/`. Implementation? `rtl/vecu_softmax/`. + Test? `tb/vecu_softmax/`. No tribal knowledge, no `grep`. +* **Separation of concerns maps to directories.** *Presents / is / is-proven* are + three real audiences (integrators read `include/`, implementers read `rtl/`, + reviewers read `tb/`). Mirroring makes "the interface changed but the TB + didn't" a visible, reviewable diff in parallel trees. +* **Generic tooling scales to a cohort.** One Makefile the whole team learns once. + CI iterates `filelists/*.f` — every module is lint-checked and sim-able the same + way, so a 40-person cohort doesn't produce 40 bespoke build flows. +* **Hardening reuses the same map.** The `.f` (convention #3) lists + `include//…` then `rtl//…`; LibreLane consumes the identical file, so + the physical flow and the sim flow never drift on sources or order. + +### Worked directory example (from the template) + +``` +docs/rtl_conventions_template/ +├── include/ +│ ├── tmpl_pkg.svh # shared BLOCK package (not per-module) +│ └── regadd/ +│ ├── regadd_if.sv # interface + modports +│ └── regadd_params.svh # module-local knobs +├── rtl/ +│ └── regadd/ +│ └── regadd.sv # the implementation (one module per file) +├── tb/ +│ └── regadd/ +│ ├── regadd_tb_top.sv # flat-pin cocotb harness (sim-only) +│ └── test_regadd.py # cocotb: self-checking, multi-seed +├── filelists/ +│ └── regadd.f # ordered: incdirs → pkg → if → leaf +├── Makefile # generic lint/test keyed on MOD +└── sim.mk # cocotb wiring included by `make test` +``` + +Note the one file that is **not** per-module: `include/_pkg.svh` sits at the +`include/` root because it is **block-wide** (shared by every module). Everything +else lives under the module's subpath. + +### Generic Make mechanics + +The generic target does three things, all from `MOD`: + +1. Resolve the module's ordered filelist: `filelists/$(MOD).f`. +2. Parse it into (a) include dirs and (b) ordered source files (order preserved). +3. Hand sources+incdirs to the tool — cocotb's `Makefile.sim` for `test`, + `verilator --lint-only` for `lint`. + +The essential parse (see `sim.mk`): + +```make +FLIST_CLEAN := $(shell grep -v '^//' $(FLIST) | grep -v '^[[:space:]]*$$') +FL_INC := $(filter +incdir+%,$(FLIST_CLEAN)) # include dirs +FL_SRCS := $(filter-out +incdir+%,$(FLIST_CLEAN)) # ordered sources +``` + +`filter`/`filter-out` **preserve list order**, so the `.f`'s compile order is +honored. For sim, the harness `tb/$(MOD)/$(MOD)_tb_top.sv` is appended (it is +sim-only and deliberately absent from the `.f`). + +> **Toolchain gotcha (recorded so nobody re-hits it):** Icarus Verilog 12 on the +> LHS/Spark boxes rejects a bare `+incdir+dir` on the **command line** (it tries +> to open it as a file), but accepts it inside a `-c` **command file**. So the +> generic Makefile translates `+incdir+X → -IX` for the cocotb/Icarus path, while +> the `.f` keeps `+incdir+` (valid for `iverilog -c` and `verilator -f`). + +--- + +## 2. Interface taxonomy + +Three file kinds, three jobs. Every one carries an **author-email header** and an +`` `ifndef `` include guard. + +| File | Kind | Holds | Guard | +|------|------|-------|-------| +| `_pkg.svh` | SV `package` | block-wide typedefs + widths, imported everywhere | `` `ifndef _PKG_SVH `` | +| `_if.sv` | SV `interface` | one module's port bundle + `modport`s | `` `ifndef _IF_SV `` | +| `_params.svh` | include header | module-local `` `define ``/localparams | `` `ifndef _PARAMS_SVH `` | + +**`_pkg.svh` — one shared package per block.** Widths and typedefs that more +than one module agrees on live here **once**. Modules `import _pkg::*;`; they +never re-declare a shared width locally. A width change happens in exactly one file. +It is a real `package` (compiled first in every `.f`), not a loose header. + +**`_if.sv` — the module's port bundle.** Bundling ports into an interface makes +a module's contract a single named object. It declares the signals plus **modports** +naming each endpoint's view. + +**`modport _` naming — named for the two modules the port connects.** A modport +is one *endpoint's view* of an interface that sits between module `a` and module `b`. +Name it `_` reading "the `a` end of the `a`↔`b` link". A bus between a source +and the `regadd` core therefore declares: + +```systemverilog +modport src_regadd (input clk, rst_n, valid_out, sum, output valid_in, a, b); // source's view +modport regadd_src (input clk, rst_n, valid_in, a, b, output valid_out, sum); // regadd's view +``` + +For a manager/subordinate link, that is `modport mgr_sub` / `modport sub_mgr` +(Lambda uses **manager/subordinate**, not master/slave — see §4). The pair-naming +tells a reader, at the port, *which two modules this wire sits between and which +end this is* — far more legible than a bare `modport in`/`out`. + +**`_params.svh` — module-local knobs.** Parameters that belong to **one** module +(pipeline depth, a local FIFO size) go in an `` `ifndef ``-guarded header, `` `include ``d +where needed and found via a `+incdir+` in the `.f`. Do **not** put block-wide widths +here — those are the package's job. + +> **Icarus caveat (see §1 and the template README).** Icarus 12 cannot elaborate an +> interface/modport as a module **port**. Lambda's open sim is iverilog+cocotb, so +> real leaves use **flat ports** with the interface as the **harness fabric**; +> Verilator/LibreLane accept modport ports if a block targets those flows. The +> `_if.sv` + `modport a_b` taxonomy is still authored for every module — it is the +> reviewed contract regardless of which sim consumes it. + +--- + +## 3. Per-top ordered `.f` filelists + +**Rule.** Each synthesizable top has **one** filelist, `filelists/.f`, listing +its sources in **dependency order**: include dirs → **packages** → interfaces → +leaf modules → **top**. That **one file is consumed by BOTH** the simulator and the +hardening flow, so sim and synthesis can never disagree on which files compile or in +what order. + +``` +// filelists/regadd.f — ordered DESIGN filelist for the `regadd` top. ++incdir+include ++incdir+include/regadd +include/tmpl_pkg.svh // packages first (compiled once, imported) +include/regadd/regadd_if.sv // interfaces (depend on packages) +rtl/regadd/regadd.sv // leaf / top (depends on pkg + if + params) +``` + +Same **content**, two invocations: + +```bash +iverilog -g2012 -c filelists/regadd.f # sim (Icarus command file) +verilator --lint-only -f filelists/regadd.f # lint / hardening front-end +``` + +Both tools accept `//` comments and `+incdir+` inside the file. Rules: + +* **Design-only.** The `.f` lists synthesizable RTL. Sim-only harnesses + (`*_tb_top.sv`) are added by the Makefile, never by the `.f`, so hardening never + sees testbench code. +* **Order is load-bearing.** Packages first (everything imports them); a module + after the interface/params it depends on. The order in the file *is* the compile + order — do not rely on the tool to reorder. +* **One `.f` per top.** A block with three hardenable tops has three filelists. + +--- + +## 4. RTL coding style (SHALL/SHOULD/MAY) — enforced + +Trimmed from atalla's style guide. The **enforcement** is the point: a +**Verilator `--lint-only` CI gate** (`.github/workflows/rtl-lint.yml`) runs on +changed RTL. atalla *had* the guide but not the gate; Lambda ships both. Keywords +per RFC 2119. + +### SHALL (lint-enforced / reviewer-blocking) + +* **SHALL** be **Verilator lint-clean** (`--lint-only -Wall`), no waivers without a + `/* verilator lint_off */` + a one-line reason. +* **SHALL** use `always_ff` for sequential logic and `always_comb` for + combinational logic. **No bare `always`** in RTL. +* **SHALL NOT** infer latches — every `always_comb` output assigned on every path + (default assignments at the top). +* **SHALL** fully reset all state; **no X on reset**. +* **SHALL** be **one module per file**, the file named for the module + (`.sv` contains `module `). +* **SHALL NOT** use `fork/join`, tri-state (`z`) logic, or `===`/`!==` in + **synthesizable** RTL (`===` is fine in TBs). No `#` delays in RTL. +* **SHALL** encode FSM states as an **`enum`** (typed states), not bare + `localparam` bit patterns. +* **SHALL** use **manager/subordinate** (`mgr`/`sub`) naming, never master/slave. +* **SHALL** carry an **author-email header** comment (`// author: name `). + +### SHOULD (reviewer-expected) + +* **SHOULD** source widths from the block `_pkg` and knobs from `_params.svh`, not + magic numbers. +* **SHOULD** name signals `snake_case`, active-low with `_n` (`rst_n`), pipeline + stages with a consistent suffix (`_q` for registered). +* **SHOULD** keep one clock + one reset per module where practical; cross-domain + crossings get an explicit synchronizer module. +* **SHOULD** connect modules through their `_if` interface + a `modport`. + +### MAY (allowed, at author discretion) + +* **MAY** use generate loops / parameterization for width- or lane-scaling. +* **MAY** add `` `ifdef ``-guarded assertions / debug that are stripped for + hardening. + +### Testbench rules + +* TBs **SHALL** be **self-checking** — assert against an **independent golden**. + Lambda's golden is the block's **Python reference model** under + `sw/reference_model/` (the spec). Import it; do **not** re-derive the golden + inside the TB. +* TBs **SHALL** run **multiple seeds** (a single-seed pass is not a pass); a failure + **SHALL** print the seed + the diverging vector. +* TBs **SHOULD** be cocotb (`test_.py`) driving the module named ``. + +--- + +## 5. Quick checklist for a new module `` in block `` + +- [ ] `include//_if.sv` — interface + `modport _` pairs, guarded, authored. +- [ ] `include//_params.svh` — module-local knobs, guarded. +- [ ] widths/typedefs it shares live in `include/_pkg.svh` (not re-declared). +- [ ] `rtl//.sv` — one module, `always_ff`/`always_comb`, fully reset, enum FSM. +- [ ] `tb//test_.py` — cocotb, imports the Python golden, multi-seed. +- [ ] `filelists/.f` — ordered (pkg → if → leaf), no TB files. +- [ ] `make lint MOD=` clean; `make test MOD=` passes. +- [ ] docs + `DECISIONS.md` updated in the SAME PR (root lab-notebook rule). + +--- + +## 6. Migration + enforcement posture + +* **New modules:** follow this doc. +* **Existing blocks (kve/tiu/acu/…):** **not** reorganized by this doc. They are + timing-closed and their build flows work; a forced move would break them. Migrating + a block to the mirrored tree is a **separate, opt-in** task, done per-block with its + own sign-off reproduced. +* **Structure check:** `scripts/check_block_structure.py --check-rtl-conventions` adds + an **advisory (warn-only)** report on interface-taxonomy presence + the mirrored + subpaths. It does **not** fail existing blocks. A future `--strict-rtl-conventions` + can make it blocking once blocks have migrated. +* **Lint gate:** `.github/workflows/rtl-lint.yml` runs Verilator `--lint-only` on + changed RTL. It starts **non-blocking** (`continue-on-error: true`) so it does not + red-wall existing PRs; flip the documented switch to make it blocking once the tree + is lint-clean. diff --git a/docs/rtl_conventions_template/Makefile b/docs/rtl_conventions_template/Makefile new file mode 100644 index 0000000..ee02095 --- /dev/null +++ b/docs/rtl_conventions_template/Makefile @@ -0,0 +1,53 @@ +# Makefile — generic, module-keyed build/lint/sim for the mirrored-tree layout. +# author: themoddedcube +# +# The whole point of the mirrored `include/ <-> rtl/ <-> tb/` tree (all sharing +# the same `/` subpath) is that ONE generic target builds ANY module by +# NAME — no per-module Makefile edits. Everything keys on MOD: +# +# make lint MOD=regadd # verilator --lint-only on the module's design .f +# make test MOD=regadd # cocotb sim (Icarus) of tb/regadd/test_regadd.py +# make help # list modules discovered from filelists/ +# +# A module is "known" iff filelists/.f exists. Adding a module = drop its +# files into include//, rtl//, tb// and add filelists/.f. + +MOD ?= +VERILATOR ?= verilator + +FLIST = filelists/$(MOD).f + +# Every module that has an ordered filelist. +MODULES := $(patsubst filelists/%.f,%,$(wildcard filelists/*.f)) + +.PHONY: all help lint test clean _require_mod + +all: help + +help: + @echo "Mirrored-tree generic Makefile (key on MOD=)" + @echo " make lint MOD= verilator --lint-only on filelists/.f" + @echo " make test MOD= cocotb (Icarus) sim of tb//test_.py" + @echo "" + @echo "known modules: $(MODULES)" + +_require_mod: + @test -n "$(MOD)" || { echo "error: set MOD=, e.g. make test MOD=regadd"; exit 2; } + @test -f "$(FLIST)" || { echo "error: no filelist $(FLIST) (known: $(MODULES))"; exit 2; } + +# --- lint: SHALL be clean (docs/rtl_conventions.md §4). Same .f as hardening. --- +lint: _require_mod + @if command -v $(VERILATOR) >/dev/null 2>&1; then \ + echo "$(VERILATOR) --lint-only -Wall --top-module $(MOD) -f $(FLIST)"; \ + $(VERILATOR) --lint-only -Wall --top-module $(MOD) -f $(FLIST); \ + else \ + echo "[lint] verilator not found — skipping locally; the rtl-lint CI gate runs it. MOD=$(MOD)"; \ + fi + +# --- test: self-checking, multi-seed cocotb sim over the same ordered .f. --- +test: _require_mod + $(MAKE) -f sim.mk sim MOD=$(MOD) + +clean: + rm -rf sim_build results.xml + find . -name '__pycache__' -type d -prune -exec rm -rf {} + diff --git a/docs/rtl_conventions_template/README.md b/docs/rtl_conventions_template/README.md new file mode 100644 index 0000000..29086ef --- /dev/null +++ b/docs/rtl_conventions_template/README.md @@ -0,0 +1,79 @@ +# `rtl_conventions_template` — a worked example of the Lambda RTL conventions + +This is a **self-contained, runnable** demonstration of the four RTL-organization +conventions in [`../rtl_conventions.md`](../rtl_conventions.md). It is a *template +/ teaching artifact* — it lives under `docs/`, touches **no real block**, and is +safe to copy when starting a new module. + +The example module is **`regadd`**: a 2-input, 16-bit **registered adder** with a +1-cycle valid pipeline. Trivial on purpose — the point is the *layout*, not the DSP. + +## The mirrored tree (convention #1) + +Three parallel trees share the **exact same `/` subpath**, so one generic +target builds any module by name: + +``` +docs/rtl_conventions_template/ +├── include/ +│ ├── tmpl_pkg.svh # shared block package (widths/typedefs) — convention #2 +│ └── regadd/ # ── subpath "regadd" ──┐ +│ ├── regadd_if.sv # interface + modports │ (interface / params +│ └── regadd_params.svh # module-local knobs │ for the regadd module) +├── rtl/ +│ └── regadd/ # ── same subpath ───────┤ +│ └── regadd.sv # the implementation │ +├── tb/ +│ └── regadd/ # ── same subpath ───────┘ +│ ├── regadd_tb_top.sv # flat-pin cocotb harness (sim-only) +│ └── test_regadd.py # self-checking, multi-seed cocotb test +├── filelists/ +│ └── regadd.f # ordered design filelist (sim + harden share it) — convention #3 +├── Makefile # generic `make lint/test MOD=` +└── sim.mk # cocotb wiring for `make test` +``` + +To add a module `foo`: create `include/foo/`, `rtl/foo/foo.sv`, `tb/foo/`, and +`filelists/foo.f`. No Makefile edits — `make test MOD=foo` just works. + +## Run it + +```bash +# from this directory, with cocotb + iverilog on PATH +make help # list discovered modules +make test MOD=regadd # cocotb sim (Icarus), self-checking, multi-seed +make lint MOD=regadd # verilator --lint-only (skips cleanly if verilator absent) +``` + +`make test MOD=regadd` is **verified passing** here (Icarus Verilog 12.0 + +cocotb 2.0.1): 4 seeds, bit-exact vs a Python golden. + +### Icarus caveat (important, and honest) + +**Icarus Verilog 12 cannot elaborate a SystemVerilog interface/modport as a module +PORT** (it *does* support interface *instantiation* + member access). Lambda's open +sim flow is **iverilog + cocotb**, and the real blocks (kve/tiu/…) use **flat leaf +ports** for exactly this reason. So in this template: + +* `regadd.sv` (the leaf that actually simulates) has **flat ports**, widths pulled + from `tmpl_pkg`; +* `regadd_if.sv` ships the **interface + `modport regadd_src`/`src_regadd`** as the + convention artifact, and `regadd_tb_top.sv` wires the leaf **through** an + instantiated `regadd_if` bus — so the interface is genuinely exercised in sim. + +**Verilator** (the lint gate) and **LibreLane/yosys** both accept modport *ports*, +so a block that targets those flows may put the interface directly on the leaf port +list. Choose per your sim: interface-on-port needs Verilator-backed cocotb; the +iverilog flow keeps flat leaf ports + the interface as harness fabric. + +## What each file demonstrates + +| File | Convention | +|------|-----------| +| `include/`, `rtl/`, `tb/` sharing `regadd/` | #1 mirrored tree + generic build | +| `tmpl_pkg.svh` (`package`, imported) | #2 shared `_pkg` | +| `regadd_if.sv` (`modport src_regadd` / `regadd_src`) | #2 `_if` + `modport a_b` naming | +| `regadd_params.svh` (`ifndef`-guarded header) | #2 `_params` | +| `filelists/regadd.f` (pkg → if → leaf) | #3 one ordered `.f` for sim + harden | +| `regadd.sv` (`always_ff`, full reset, one module/file) | #4 SHALL/SHOULD/MAY style | +| `test_regadd.py` (asserts vs golden, multi-seed) | #4 self-checking TB | diff --git a/docs/rtl_conventions_template/filelists/regadd.f b/docs/rtl_conventions_template/filelists/regadd.f new file mode 100644 index 0000000..3332748 --- /dev/null +++ b/docs/rtl_conventions_template/filelists/regadd.f @@ -0,0 +1,28 @@ +// regadd.f — ordered DESIGN filelist for the `regadd` top. +// +// Convention demo (docs/rtl_conventions.md §3): ONE ordered `.f` per top, +// consumed by BOTH the simulator and the hardening flow so they can never +// disagree on compile order. Order is: include dirs -> packages -> interfaces +// -> leaf modules -> top. `regadd` is itself a leaf, so it is last here. +// +// Same CONTENT, two invocations: +// iverilog -g2012 -c filelists/regadd.f ... (sim / Icarus command file) +// verilator --lint-only -f filelists/regadd.f (lint / hardening front-end) +// Both tools also accept `+incdir+` and `//` comments in a command file. +// +// This is the DESIGN list (synthesizable RTL only). The sim harness +// tb/regadd/regadd_tb_top.sv is added by the Makefile for `make test`; it is +// deliberately NOT here so hardening never sees testbench code. + +// --- include search paths (so `include finds the *_params.svh headers) --- ++incdir+include ++incdir+include/regadd + +// --- packages first (compiled once, imported everywhere) --- +include/tmpl_pkg.svh + +// --- interfaces (depend on packages) --- +include/regadd/regadd_if.sv + +// --- leaf / top module (depends on package + interface + params header) --- +rtl/regadd/regadd.sv diff --git a/docs/rtl_conventions_template/include/regadd/regadd_if.sv b/docs/rtl_conventions_template/include/regadd/regadd_if.sv new file mode 100644 index 0000000..f8e4c78 --- /dev/null +++ b/docs/rtl_conventions_template/include/regadd/regadd_if.sv @@ -0,0 +1,44 @@ +// regadd_if.sv — SystemVerilog interface + modports for `regadd`. +// author: themoddedcube +// +// Convention demo (docs/rtl_conventions.md §2): `_if.sv` bundles a module's +// port group into one interface and names the two endpoints with +// `modport _`, where a/b are the two MODULES the port sits between. +// +// This bus sits between an upstream source (`src`) and the `regadd` core: +// * modport `src_regadd` — the SOURCE's view: it DRIVES the operands, READS +// the result. (Used by testbench / upstream producer.) +// * modport `regadd_src` — the REGADD's view: it READS the operands, DRIVES +// the result. (Used by the DUT.) +// The pair reads a→b = "the a side of the a–b link", so `src_regadd` is the +// src end and `regadd_src` is the regadd end of the same src↔regadd interface. +`ifndef REGADD_IF_SV +`define REGADD_IF_SV + +interface regadd_if ( + input logic clk, + input logic rst_n +); + import tmpl_pkg::*; + + logic valid_in; // operands a/b are valid this cycle + data_t a; + data_t b; + logic valid_out; // sum is valid this cycle (latency-aligned) + sum_t sum; + + // SOURCE end of the src↔regadd link. + modport src_regadd ( + input clk, rst_n, valid_out, sum, + output valid_in, a, b + ); + + // REGADD (DUT) end of the src↔regadd link. + modport regadd_src ( + input clk, rst_n, valid_in, a, b, + output valid_out, sum + ); + +endinterface : regadd_if + +`endif // REGADD_IF_SV diff --git a/docs/rtl_conventions_template/include/regadd/regadd_params.svh b/docs/rtl_conventions_template/include/regadd/regadd_params.svh new file mode 100644 index 0000000..51a1094 --- /dev/null +++ b/docs/rtl_conventions_template/include/regadd/regadd_params.svh @@ -0,0 +1,16 @@ +// regadd_params.svh — module-local parameters for `regadd`. +// author: themoddedcube +// +// Convention demo (docs/rtl_conventions.md §2): `_params.svh` holds the +// knobs that belong to ONE module (not the whole block). It is `include`-style +// (loose macros / localparams, `ifndef`-guarded), pulled in with `include` +// where needed and found via a `+incdir+` in the `.f`. Block-wide widths live +// in the shared `tmpl_pkg`, NOT here. +`ifndef REGADD_PARAMS_SVH +`define REGADD_PARAMS_SVH + +// Pipeline latency of the registered adder, in clocks. The valid flag is +// delayed by exactly this many cycles to stay aligned with `sum`. +`define REGADD_LATENCY 1 + +`endif // REGADD_PARAMS_SVH diff --git a/docs/rtl_conventions_template/include/tmpl_pkg.svh b/docs/rtl_conventions_template/include/tmpl_pkg.svh new file mode 100644 index 0000000..27aea44 --- /dev/null +++ b/docs/rtl_conventions_template/include/tmpl_pkg.svh @@ -0,0 +1,27 @@ +// tmpl_pkg.svh — shared block package for the `tmpl` example block. +// author: themoddedcube +// +// Convention demo (docs/rtl_conventions.md §2): ONE shared `_pkg.svh` +// per block holds the block-wide typedefs / widths that more than one module +// agrees on. Modules `import tmpl_pkg::*;` — they never re-declare these widths +// locally, so a width change happens in exactly one place. This file is a real +// SystemVerilog `package` (compiled once, first in every `.f`), NOT an +// `include`-style header of loose `localparam`s. +`ifndef TMPL_PKG_SVH +`define TMPL_PKG_SVH + +package tmpl_pkg; + + // Block-wide operand width. Every datapath module in `tmpl` speaks this. + parameter int unsigned DATA_W = 16; + + // Sum width: one extra bit so a full-scale a+b never overflows. + parameter int unsigned SUM_W = DATA_W + 1; + + // Operand / result typedefs — modules use these names, not raw `logic [..]`. + typedef logic [DATA_W-1:0] data_t; + typedef logic [SUM_W-1:0] sum_t; + +endpackage : tmpl_pkg + +`endif // TMPL_PKG_SVH diff --git a/docs/rtl_conventions_template/rtl/regadd/regadd.sv b/docs/rtl_conventions_template/rtl/regadd/regadd.sv new file mode 100644 index 0000000..f87a3cb --- /dev/null +++ b/docs/rtl_conventions_template/rtl/regadd/regadd.sv @@ -0,0 +1,60 @@ +// regadd.sv — a 2-input registered adder. The worked example leaf module. +// author: themoddedcube +// +// Convention demo (docs/rtl_conventions.md §4, the SHALL/SHOULD/MAY guide): +// * one module per file, file named for the module; +// * widths come from `tmpl_pkg` (imported), knobs from `regadd_params.svh`; +// * `always_ff` for state, `always_comb` for logic — no bare `always`; +// * fully reset, so no inferred latches / no X on reset; no `===`, tri-state, +// or fork/join in RTL. +// +// PORTS: this leaf uses a FLAT port list. The block's `regadd_if` interface +// (include/regadd/regadd_if.sv) declares the same signal bundle + the +// `modport regadd_src` naming — the harness wires this leaf up THROUGH that +// interface. See the doc's "Icarus caveat": Icarus Verilog 12 cannot elaborate +// an interface/modport as a module PORT, and Lambda's open sim flow is +// iverilog+cocotb, so leaves keep flat ports and the interface is the harness +// fabric. Verilator (the lint gate) and LibreLane both accept either form. +// +// Behaviour: sum = a + b, registered, with a `REGADD_LATENCY`-cycle valid +// pipeline so `valid_out`/`sum` stay aligned. +`include "regadd_params.svh" + +module regadd + import tmpl_pkg::*; +( + input logic clk, + input logic rst_n, + input logic valid_in, + input data_t a, + input data_t b, + output logic valid_out, + output sum_t sum +); + + // Registered sum. Extra bit (SUM_W) means a+b never overflows. + sum_t sum_q; + logic valid_q; + + always_ff @(posedge clk or negedge rst_n) begin + if (!rst_n) begin + sum_q <= '0; + valid_q <= 1'b0; + end else begin + sum_q <= sum_t'(a) + sum_t'(b); + valid_q <= valid_in; + end + end + + assign sum = sum_q; + assign valid_out = valid_q; + + // Latency is a compile-time contract of this module; keep the knob honest. + // (Elaboration-time check — no runtime cost, lint-clean.) + initial begin : latency_contract + if (`REGADD_LATENCY != 1) + $fatal(1, "regadd models exactly 1 pipeline stage; REGADD_LATENCY=%0d", + `REGADD_LATENCY); + end + +endmodule : regadd diff --git a/docs/rtl_conventions_template/sim.mk b/docs/rtl_conventions_template/sim.mk new file mode 100644 index 0000000..c0dc438 --- /dev/null +++ b/docs/rtl_conventions_template/sim.mk @@ -0,0 +1,39 @@ +# sim.mk — cocotb wiring for the generic `make test MOD=` target. +# author: themoddedcube +# +# Invoked as `$(MAKE) -f sim.mk sim MOD=` by the top Makefile. It parses the +# module's ordered design filelist (filelists/.f), adds the sim-only harness +# tb//_tb_top.sv, and hands the result to cocotb's stock Makefile.sim. +# Keeping this in its own file means the top Makefile's `lint` target does not +# drag in cocotb's include. + +ifndef MOD +$(error set MOD=, e.g. make test MOD=regadd) +endif + +FLIST := filelists/$(MOD).f + +# Parse the .f: drop `//` comment lines and blank lines, then split include dirs +# from source files. `filter`/`filter-out` preserve the ordered-compile order. +FLIST_CLEAN := $(shell grep -v '^//' $(FLIST) 2>/dev/null | grep -v '^[[:space:]]*$$') +FL_INC := $(filter +incdir+%,$(FLIST_CLEAN)) +FL_SRCS := $(filter-out +incdir+%,$(FLIST_CLEAN)) +# Icarus on this box wants attached -Idir on the command line (bare +incdir+ is +# only honoured inside a -c command file), so translate +incdir+X -> -IX. +FL_INCDIRS := $(patsubst +incdir+%,-I%,$(FL_INC)) + +SIM ?= icarus +TOPLEVEL_LANG ?= verilog + +# Design sources (ordered) + the sim-only flat-port harness. +VERILOG_SOURCES := $(abspath $(FL_SRCS)) $(abspath tb/$(MOD)/$(MOD)_tb_top.sv) +TOPLEVEL := $(MOD)_tb_top +MODULE := test_$(MOD) + +# Include dirs -> Icarus. +COMPILE_ARGS += -g2012 $(FL_INCDIRS) + +# cocotb finds test_.py on PYTHONPATH. +export PYTHONPATH := $(CURDIR)/tb/$(MOD):$(PYTHONPATH) + +include $(shell cocotb-config --makefiles)/Makefile.sim diff --git a/docs/rtl_conventions_template/tb/regadd/regadd_tb_top.sv b/docs/rtl_conventions_template/tb/regadd/regadd_tb_top.sv new file mode 100644 index 0000000..af2b7ad --- /dev/null +++ b/docs/rtl_conventions_template/tb/regadd/regadd_tb_top.sv @@ -0,0 +1,49 @@ +// regadd_tb_top.sv — flat-pin cocotb harness that routes the `regadd` DUT +// THROUGH the `regadd_if` interface bundle. +// author: themoddedcube +// +// cocotb's TOPLEVEL must be a MODULE and it drives top-level pins directly, so +// this harness exposes flat pins. Internally it instantiates `regadd_if bus` +// (the block's interface — the convention artifact) and connects the leaf +// through the bus members, demonstrating the interface as the connection +// fabric. (Icarus 12 supports interface INSTANTIATION + member access; it does +// not support interface/modport module PORTS — see the doc's Icarus caveat — +// which is why the leaf's own ports are flat and the bundle lives here.) +// This is a SIM-only file: it is NOT in regadd.f, so hardening never sees it. +module regadd_tb_top #( + parameter int unsigned P_DATA_W = 16, // mirrors tmpl_pkg::DATA_W + parameter int unsigned P_SUM_W = 17 // mirrors tmpl_pkg::SUM_W +) ( + input logic clk, + input logic rst_n, + input logic valid_in, + input logic [P_DATA_W-1:0] a, + input logic [P_DATA_W-1:0] b, + output logic valid_out, + output logic [P_SUM_W-1:0] sum +); + + // The block interface as the harness fabric (modports declared inside it). + regadd_if bus (.clk(clk), .rst_n(rst_n)); + + // Flat pins -> interface (the src end drives operands). + assign bus.valid_in = valid_in; + assign bus.a = a; + assign bus.b = b; + + // DUT connected through the interface members. + regadd u_regadd ( + .clk (bus.clk), + .rst_n (bus.rst_n), + .valid_in (bus.valid_in), + .a (bus.a), + .b (bus.b), + .valid_out (bus.valid_out), + .sum (bus.sum) + ); + + // Interface -> flat pins (read the regadd end's results). + assign valid_out = bus.valid_out; + assign sum = bus.sum; + +endmodule : regadd_tb_top diff --git a/docs/rtl_conventions_template/tb/regadd/test_regadd.py b/docs/rtl_conventions_template/tb/regadd/test_regadd.py new file mode 100644 index 0000000..2bad121 --- /dev/null +++ b/docs/rtl_conventions_template/tb/regadd/test_regadd.py @@ -0,0 +1,80 @@ +# test_regadd.py — self-checking, multi-seed cocotb test for the regadd leaf. +# author: themoddedcube +# +# Convention demo (docs/rtl_conventions.md §4): testbenches are SELF-CHECKING +# (they assert against an independent golden, here plain Python integer add) and +# run MULTIPLE SEEDS so a pass is not a single lucky vector. The golden here is +# trivial on purpose; a real block imports the block's Python reference model +# from sw/reference_model/ instead of re-deriving the golden in the TB. +# +# Run: make test MOD=regadd (from docs/rtl_conventions_template/) + +import os +import random + +import cocotb +from cocotb.clock import Clock +from cocotb.triggers import RisingEdge, Timer + +DATA_W = 16 +MASK = (1 << DATA_W) - 1 +LATENCY = 1 # matches `REGADD_LATENCY in include/regadd/regadd_params.svh + + +async def _reset(dut): + dut.valid_in.value = 0 + dut.a.value = 0 + dut.b.value = 0 + dut.rst_n.value = 0 + # Hold reset a few clocks, then release on a clean edge. + for _ in range(3): + await RisingEdge(dut.clk) + dut.rst_n.value = 1 + await RisingEdge(dut.clk) + + +async def _drive_and_check(dut, seed, n=64): + """Feed n random operand pairs, checking each latency-aligned result.""" + rng = random.Random(seed) + pipe = [] # in-flight expected (valid, sum) pairs, one per pipeline stage + + for _ in range(n): + a = rng.randint(0, MASK) + b = rng.randint(0, MASK) + dut.a.value = a + dut.b.value = b + dut.valid_in.value = 1 + pipe.append((1, a + b)) + await RisingEdge(dut.clk) + + # Once the pipeline is primed, the output at this edge corresponds to + # the input LATENCY cycles earlier. + if len(pipe) > LATENCY: + exp_valid, exp_sum = pipe.pop(0) + got_valid = int(dut.valid_out.value) + got_sum = int(dut.sum.value) + assert got_valid == exp_valid, ( + f"seed={seed}: valid_out {got_valid} != {exp_valid}") + assert got_sum == exp_sum, ( + f"seed={seed}: sum {got_sum} != {exp_sum} " + f"(a+b={exp_sum & 0x1FFFF})") + + # Drain: valid should drop LATENCY cycles after valid_in goes low. + dut.valid_in.value = 0 + for _ in range(LATENCY + 1): + await RisingEdge(dut.clk) + + +@cocotb.test() +async def test_regadd_multiseed(dut): + """Registered add is bit-exact vs Python golden across several seeds.""" + cocotb.start_soon(Clock(dut.clk, 10, unit="ns").start()) + await _reset(dut) + + # Multi-seed: a pass must hold across independent random streams. + seeds = [int(s) for s in os.environ.get("SEEDS", "1,2,3,4").split(",")] + for seed in seeds: + await _reset(dut) + await _drive_and_check(dut, seed) + + dut._log.info(f"regadd OK across seeds {seeds}") diff --git a/scripts/check_block_structure.py b/scripts/check_block_structure.py index 1d3f60b..0ed0590 100644 --- a/scripts/check_block_structure.py +++ b/scripts/check_block_structure.py @@ -5,13 +5,25 @@ letting team leads coordinate versioned changes without per-block surprises. Levels: - ERROR — a required file/dir is missing. Exit code 1. - WARN — a convention deviation (filename schema, dangling config, date precision). Exit 0 - unless --strict. + ERROR — a required file/dir is missing. Exit code 1. + WARN — a convention deviation (filename schema, dangling config, date precision). Exit 0 + unless --strict. + ADVISE — an RTL-conventions nudge (docs/rtl_conventions.md). Opt-in, purely advisory: + NEVER affects the exit code (not even under --strict), so it can never fail an + existing, not-yet-migrated block. See --check-rtl-conventions below. Usage: - python3 scripts/check_block_structure.py # report; exit 1 on any ERROR - python3 scripts/check_block_structure.py --strict # exit 1 on ERROR or WARN + python3 scripts/check_block_structure.py # report; exit 1 on any ERROR + python3 scripts/check_block_structure.py --strict # exit 1 on ERROR or WARN + python3 scripts/check_block_structure.py --check-rtl-conventions # + advisory RTL-conv scan + python3 scripts/check_block_structure.py --check-rtl-conventions --strict-rtl-conventions + # promote RTL-conv ADVISE->WARN + # (blocking under --strict; for + # future use once blocks migrate) + +The RTL-conventions scan (docs/rtl_conventions.md) is OFF by default and ADVISORY when on — +it does not reorganize or fail the proven, timing-closed blocks; it only reports how far each +block is from the mirrored-tree / interface-taxonomy conventions for the incoming cohort. Stdlib only. Locates the monorepo root via this file's path. """ @@ -66,6 +78,10 @@ def warn(block: str, msg: str) -> None: findings.append(("WARN", block, msg)) +def advise(block: str, msg: str) -> None: + findings.append(("ADVISE", block, msg)) + + def check_functional(block: str) -> None: bdir = ROOT / bpath(block) for f in REQUIRED_FILES: @@ -138,14 +154,70 @@ def check_decision_dates(block: str, bdir: Path) -> None: f"...{loose.group(0).strip()}") +# --------------------------------------------------------------------------- +# Opt-in RTL-conventions scan (docs/rtl_conventions.md). ADVISORY by default — +# NEVER gates the exit code, so it can't fail a not-yet-migrated block. +# `--strict-rtl-conventions` promotes these to WARN (blocking under --strict); +# that switch is for future use once blocks have migrated. +# --------------------------------------------------------------------------- +MODPORT_RE = re.compile(r"\bmodport\s+([A-Za-z_]\w*)") + + +def check_rtl_conventions(block: str, promote: bool) -> None: + """Advisory nudges toward docs/rtl_conventions.md. Additive; presence-only.""" + lvl = warn if promote else advise + bdir = ROOT / bpath(block) + rtldir = bdir / "rtl" + if not rtldir.is_dir(): + return + + # §2 — one shared _pkg.sv{,h} package. + if not list(rtldir.rglob("*_pkg.sv")) and not list(rtldir.rglob("*_pkg.svh")): + lvl(block, "rtl-conv §2: no _pkg.sv{,h} shared package found") + + # §2 — _if.sv interfaces. + ifs = list(rtldir.rglob("*_if.sv")) + if not ifs: + lvl(block, "rtl-conv §2: no _if.sv interface files") + + # §2 — modport _ naming (only flag interfaces that exist). + for iff in ifs: + try: + text = iff.read_text(errors="ignore") + except OSError: + continue + for mp in MODPORT_RE.findall(text): + if "_" not in mp: + lvl(block, f"rtl-conv §2: modport '{mp}' in " + f"{iff.relative_to(ROOT)} is not _") + + # §1 — mirrored include/ <-> rtl/ <-> tb/ tree (subpath-shared). + if not (rtldir / "include").is_dir(): + lvl(block, "rtl-conv §1: no rtl/include/ tree (mirrored include↔rtl↔tb)") + if not (rtldir / "tb").is_dir(): + lvl(block, "rtl-conv §1: no rtl/tb/ tree (mirrored include↔rtl↔tb)") + + # §3 — per-top ordered .f filelists. + if not list(rtldir.rglob("*.f")): + lvl(block, "rtl-conv §3: no per-top .f filelist(s) under rtl/") + + def main() -> int: ap = argparse.ArgumentParser(description=__doc__) ap.add_argument("--strict", action="store_true", help="exit 1 on WARN too") + ap.add_argument("--check-rtl-conventions", action="store_true", + help="also run the advisory RTL-conventions scan " + "(docs/rtl_conventions.md); ADVISE findings never gate") + ap.add_argument("--strict-rtl-conventions", action="store_true", + help="promote RTL-conv ADVISE->WARN (blocking under --strict); " + "future use once blocks migrate") args = ap.parse_args() for b in FUNCTIONAL_BLOCKS: if (ROOT / bpath(b)).is_dir(): check_functional(b) + if args.check_rtl_conventions: + check_rtl_conventions(b, promote=args.strict_rtl_conventions) else: err(b, "block directory does not exist") for b in INTEGRATION_BLOCKS: @@ -154,10 +226,13 @@ def main() -> int: errors = [f for f in findings if f[0] == "ERROR"] warns = [f for f in findings if f[0] == "WARN"] + advises = [f for f in findings if f[0] == "ADVISE"] for level, block, msg in sorted(findings): - print(f"{level:5} [{block}] {msg}") - print(f"\n{len(errors)} error(s), {len(warns)} warning(s)") + print(f"{level:6} [{block}] {msg}") + print(f"\n{len(errors)} error(s), {len(warns)} warning(s), " + f"{len(advises)} advisory") + # ADVISE never gates — only ERROR (always) and WARN (under --strict) do. if errors or (args.strict and warns): return 1 return 0