Fixes #7562.
This commit is contained in:
Yilou Wang
GitHub
parent
077558a9b0
commit
709c444df3
|
|
@ -0,0 +1,142 @@
|
|||
<!-- DESCRIPTION: Verilator: Repository-wide guidelines for AI coding agents
|
||||
SPDX-FileCopyrightText: 2026-2026 Wilson Snyder
|
||||
SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -->
|
||||
|
||||
# Verilator Guidelines for AI Coding Agents
|
||||
|
||||
These files are the general layer an agent loads first -- nearest file wins, so
|
||||
you read this repository-root file plus the one for the directory you are
|
||||
editing. They stay deliberately high-level: where to start, how the tree is laid
|
||||
out, and the conventions reviewers otherwise enforce by hand. They are an index,
|
||||
not the architecture reference -- for depth (how a pass works internally, the
|
||||
algorithms, node lifetime) they point you to `docs/internals.rst`. When the
|
||||
guidance here is not enough, that is where to look next.
|
||||
|
||||
This file has two parts. **Orientation** gets you productive in the codebase from
|
||||
a cold start. **Before you open a PR** is the checklist of conventions reviewers
|
||||
otherwise have to enforce by hand -- read it before submitting any change.
|
||||
|
||||
Then read the directory guide for the area you are editing:
|
||||
|
||||
- [src/AGENTS.md](src/AGENTS.md) -- compiler C++ sources: AST, visitors, passes, parser, style
|
||||
- [include/AGENTS.md](include/AGENTS.md) -- runtime library (`verilated*`): C++14, MT-safety, fixed-width types
|
||||
- [test_regress/AGENTS.md](test_regress/AGENTS.md) -- regression tests: harness, drivers, golden files
|
||||
- [docs/AGENTS.md](docs/AGENTS.md) -- documentation (`*.rst`)
|
||||
|
||||
---
|
||||
|
||||
# Orientation
|
||||
|
||||
## What Verilator is
|
||||
|
||||
Verilator is a *compiler*, not an interpreter. It translates synthesizable (and
|
||||
much behavioral) SystemVerilog into a cycle-accurate C++ model that you then
|
||||
compile and run. Almost every decision is made at compile ("verilation") time;
|
||||
the generated C++ just advances state each evaluation. Optimize for verilation-
|
||||
time work over runtime work.
|
||||
|
||||
## The pipeline is the spine
|
||||
|
||||
A run is an ordered sequence of passes over one shared AST (abstract syntax
|
||||
tree). In source order:
|
||||
|
||||
| Stage | What it does | Key files |
|
||||
|---|---|---|
|
||||
| Preprocess + parse | Lex and parse text into a raw AST -- builds nodes only, no semantic checks | `verilog.l`, `verilog.y` |
|
||||
| Link / elaborate | Resolve names, scopes, parameters; instantiate the hierarchy | `V3LinkParse`, `V3LinkDot`, `V3Param` |
|
||||
| Width / type | Assign and check data types and bit widths | `V3Width` |
|
||||
| Transform / optimize / schedule | Constant fold, lower language features, schedule events | `V3Const`, `V3Randomize`, `V3Assert*`, `V3Sched`, `V3Timing`, `V3Dfg` |
|
||||
| Emit | Lower the final AST to generated C++ | `V3EmitC*` |
|
||||
| Runtime | Library the generated model links against | `include/verilated*` |
|
||||
|
||||
This table is the map; `docs/internals.rst` has the detail behind each stage.
|
||||
|
||||
## Where to make a change
|
||||
|
||||
Map the symptom to the pass that owns it, then start by reading that pass's
|
||||
top-of-file comment.
|
||||
|
||||
| Symptom or feature area | Start in |
|
||||
|---|---|
|
||||
| Type/width error, "what type is this", implicit conversion | `V3Width` |
|
||||
| Name/scope/parameter resolution ("Can't find...", hierarchy) | `V3LinkDot`, `V3Param` |
|
||||
| `randomize` / `constraint` / `rand` / `randc` | `V3Randomize` |
|
||||
| `assert` / `property` / `sequence` / `cover` | `V3Assert`, `V3AssertPre`, `V3AssertNfa` |
|
||||
| `fork` / timing / `#delay` / NBA / event scheduling | `V3Sched`, `V3Timing`, `V3Fork` |
|
||||
| Syntax wrongly accepted or rejected | `verilog.y`, `verilog.l` |
|
||||
| Wrong generated C++ | `V3EmitC*` |
|
||||
| Runtime model behavior | `include/verilated*` |
|
||||
|
||||
## Build and run a test
|
||||
|
||||
- Build in the source tree: `autoconf && ./configure && make -j8`. Configure with
|
||||
`--enable-ccwarn` so a new compiler warning stops the build.
|
||||
- Run one test from the repository root: `test_regress/t/t_<name>.py`.
|
||||
- Run the full regression with `make test`. The complete suite requires
|
||||
configuring with `--enable-longtests` (works on every OS, including macOS).
|
||||
|
||||
---
|
||||
|
||||
# Before you open a PR
|
||||
|
||||
## Scope and process
|
||||
|
||||
- [ ] Searched open PRs and issues -- duplicating in-flight work wastes review time.
|
||||
- [ ] Fixed the general root cause, not just the reported case -- if it also
|
||||
affects other modules/classes/interfaces, cover them or expect rejection.
|
||||
- [ ] PR is single-purpose. Refactors, drive-by fixes found along the way, and new
|
||||
features each go in separate PRs; land standalone cleanups first.
|
||||
- [ ] Every bug fix has a test that fails *without* the fix; include the issue's
|
||||
own reproducer when possible.
|
||||
- [ ] New code aims for 100% line coverage; branch coverage far below line coverage
|
||||
signals guards callers never violate -- justify or remove them.
|
||||
- [ ] Ran `make format` (clang-format), `make cppcheck`, and `make lint-py`;
|
||||
self-reviewed the diff for leftover debug code, stale comments, and
|
||||
copy-paste errors.
|
||||
- [ ] Ran the full regression on at least one OS before submitting. Partial runs
|
||||
are fine during development, but the submitted PR is expected to pass every
|
||||
test.
|
||||
- [ ] Did not edit `docs/CONTRIBUTORS` (humans only) or `Changes` (maintainer
|
||||
updates it near release).
|
||||
|
||||
## Pick the right diagnostic (and its required test)
|
||||
|
||||
The API you choose determines which test must accompany the change.
|
||||
|
||||
| API | Output | Meaning | Required test |
|
||||
|---|---|---|---|
|
||||
| `v3error("...")` | `%Error:` | User wrote invalid SystemVerilog | `t_*_bad*.v` + `.out` golden |
|
||||
| `v3error("Unsupported: ...")` | `%Error-UNSUPPORTED:` | Legal SV that Verilator does not yet support | `t_*_unsup*.v` + `.out` golden |
|
||||
| `v3warn(CODE, "...")` | `%Warning-CODE:` | Legal but suspicious code | warning test + `.out` golden |
|
||||
| `v3fatalSrc("...")` | `%Error: Internal Error` | Should-never-happen internal assertion | none -- not user-triggerable |
|
||||
|
||||
- Every `v3error`/`v3warn` needs a test in `test_regress/t/` -- enforced by the
|
||||
warn-coverage distribution test. `v3fatalSrc` is exempt.
|
||||
- Reserve "Unsupported:" for not-yet-implemented features, never for user mistakes.
|
||||
- When an error enforces a spec-defined restriction, cite the clause
|
||||
(`IEEE 1800-2023 11.4.7`) so it is verifiable. Update `docs/guide/warnings.rst`
|
||||
when adding or changing a warning.
|
||||
- On error paths, clean up or replace invalid AST (e.g. `AstConst::BitFalse`) so
|
||||
later passes do not crash after the error.
|
||||
|
||||
## Cross-cutting code rules
|
||||
|
||||
- [ ] No non-ASCII characters in C++ sources or headers: write `--` (two ASCII
|
||||
hyphens) rather than a Unicode em-dash, and a plain `'` rather than a smart
|
||||
quote. At write time, not when CI complains.
|
||||
- [ ] Lists stay sorted: lexer/parser tokens, option declarations, enum values,
|
||||
configure feature lists, documented option lists.
|
||||
- [ ] `bin/` scripts are Python (distributed cross-platform); `nodist/` may use
|
||||
bash and platform-specific code (developer-only, not packaged).
|
||||
- [ ] Runtime code in `include/` targets C++14 (`--no-timing` builds must work);
|
||||
C++20 only in timing code paths.
|
||||
- [ ] In `include/` public headers, prefix public classes with `Verilated`/`Vl`
|
||||
and document the API with `///` comments.
|
||||
- [ ] A new code pattern is applied globally or not at all -- no one-off
|
||||
convention in a single file.
|
||||
|
||||
## Commits
|
||||
|
||||
- Subject line is short and imperative and conventionally ends with the PR number:
|
||||
`Support property case (#7721)`. A body is optional and common for non-trivial
|
||||
changes.
|
||||
|
|
@ -0,0 +1,38 @@
|
|||
<!-- DESCRIPTION: Verilator: docs/ guidelines for AI coding agents
|
||||
SPDX-FileCopyrightText: 2026-2026 Wilson Snyder
|
||||
SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -->
|
||||
|
||||
# docs/ Guidelines -- Verilator documentation (*.rst)
|
||||
|
||||
When to check: before editing anything under `docs/`.
|
||||
Read the repository-root [AGENTS.md](../AGENTS.md) first for process and cross-cutting rules.
|
||||
|
||||
## Writing rules
|
||||
|
||||
- Rewrap paragraphs after editing -- keep consistent line length in `*.rst` files.
|
||||
|
||||
- Document only stable, implemented features -- never planned or in-development capabilities; prevents user confusion and support burden.
|
||||
|
||||
- Explain WHAT and WHEN, not HOW -- conceptual purpose and practical use cases over implementation mechanics; describe behavior ("optimized as pure", not "treated as pure") and clarify ambiguous referents ("in the internals of Verilator").
|
||||
|
||||
- Keep terminology consistent -- one term per concept; update docs when renaming code constructs; spell out full terms, avoiding abbreviations like "sim"/"sims".
|
||||
|
||||
- Use "how many" for countable nouns (threads, tasks, workers) and "how much" for uncountable quantities.
|
||||
|
||||
- Mark internal or experimental features "for internal use only" -- prevents user dependence and forced deprecation cycles later.
|
||||
|
||||
- Use specific IEEE references: `IEEE {number}-{year}` plus the section (e.g. `Annex I`) -- a vague "IEEE spec requires" is unverifiable.
|
||||
|
||||
- Document all flags with descriptions, not just syntax.
|
||||
|
||||
## reStructuredText mechanics
|
||||
|
||||
- Use the `:vlopt:` role for Verilator option references -- makes cross-references clickable and consistent.
|
||||
|
||||
- Escape angle brackets (`\<`, `\>`) in link targets -- prevents broken links to command-line options.
|
||||
|
||||
- Generate documentation examples with `test.extract` from `test_regress` test files -- examples stay synced with actually tested behavior.
|
||||
|
||||
## Hard constraint
|
||||
|
||||
- Never edit `docs/CONTRIBUTORS` -- only humans may, after reading and agreeing to its statement; remind the user instead.
|
||||
|
|
@ -0,0 +1,60 @@
|
|||
<!-- DESCRIPTION: Verilator: include/ guidelines for AI coding agents
|
||||
SPDX-FileCopyrightText: 2026-2026 Wilson Snyder
|
||||
SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -->
|
||||
|
||||
# include/ -- Verilator runtime library
|
||||
|
||||
Covers the C++ runtime under `include/` (`verilated.h/.cpp`, `verilated_*.h/.cpp`)
|
||||
that every generated model links against. Read the repository-root
|
||||
[AGENTS.md](../AGENTS.md) first. The rules here differ from `src/`: this code
|
||||
ships to users, runs every simulation cycle, and must stay portable and fast.
|
||||
|
||||
---
|
||||
|
||||
# Orientation
|
||||
|
||||
- **This is the runtime, not the compiler.** The passes in `src/` *emit* C++ that
|
||||
calls into this library; this code then *runs* during simulation. Optimize it
|
||||
for execution speed and portability, not for compile-time clarity.
|
||||
- **Key files:** `verilated.h` (core model API), `verilated_types.h` (data
|
||||
types), `verilated_random.cpp` (constrained-random runtime), `verilated_cov.*`
|
||||
(coverage), `verilated_threads.*` (MT runtime), `verilated_timing.*`
|
||||
(`--timing` runtime), `verilated_vcd_c.*`/`verilated_fst_c.*` (tracing).
|
||||
- A runtime-only fix lives entirely here and does not rebuild `verilator_bin`.
|
||||
|
||||
---
|
||||
|
||||
# Before you open a PR
|
||||
|
||||
- **C++14 baseline.** The runtime must build under `--no-timing` with C++14; C++20
|
||||
features are allowed only in `--timing` code paths.
|
||||
- **Public API naming and docs.** Prefix public classes and types with
|
||||
`Verilated`/`Vl` to avoid collisions with user code, and document the API with
|
||||
`///` comments -- they feed doc generation.
|
||||
- **No exceptions in runtime code** -- use error returns or assertions; exceptions
|
||||
add overhead on every path.
|
||||
- **Use fixed-width model types** (`CData`/`SData`/`IData`/`QData`/`VlWide`), never
|
||||
`size_t`, for model data. Process wide data word-by-word (`VL_ZERO_W`,
|
||||
`VL_MEMCPY_W`), never bit-by-bit or byte-by-byte.
|
||||
- **Do all string parsing at verilation time** -- never parse strings during
|
||||
simulation; emit structured data or compile-time hints instead.
|
||||
- **Keep per-cycle code lean.** Do not add vtables to high-frequency objects
|
||||
(8 bytes per instance); guard optional features behind
|
||||
`hasClasses()`/`hasEvents()`-style checks; functions called per cycle must avoid
|
||||
mutexes -- use atomics or lockless designs.
|
||||
- **Emit no runtime loops** the compiler could have expanded at verilation time;
|
||||
prefer a single runtime call.
|
||||
- **Thread safety.** Annotate with the hierarchy `VL_PURE` > `VL_MT_SAFE` >
|
||||
`VL_MT_STABLE`; annotations must match the implementation. Annotate
|
||||
mutex-protected members with `VL_GUARDED_BY` and document acquisition ordering.
|
||||
Prefer has-a over is-a: a guarded class wrapping the unguarded internal one, with
|
||||
the guarded version as the default public API.
|
||||
- **Keep runtime and compiler headers separate** -- never include `verilated.h`
|
||||
into the compiler in `src/`.
|
||||
|
||||
## File-specific rules
|
||||
|
||||
| File | Rule |
|
||||
|---|---|
|
||||
| `verilated_random.cpp` | Emit only portable SMT-LIB 2.6 -- no solver-specific or MaxSMT extensions; the generated solver text is the model's runtime constraint interface |
|
||||
| `verilated_cov.cpp` | Coverage runtime is shared by all models -- keep per-point overhead minimal and the on-disk format stable for `verilator_coverage` |
|
||||
|
|
@ -0,0 +1,227 @@
|
|||
<!-- DESCRIPTION: Verilator: src/ guidelines for AI coding agents
|
||||
SPDX-FileCopyrightText: 2026-2026 Wilson Snyder
|
||||
SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -->
|
||||
|
||||
# src/ -- Verilator compiler sources
|
||||
|
||||
Covers all C++ under `src/`, including astgen inputs and the parser/lexer
|
||||
(`verilog.y`, `verilog.l`). Read the repository-root [AGENTS.md](../AGENTS.md)
|
||||
first. This file has two parts: **Orientation** explains the AST and pass model;
|
||||
**Before you open a PR** is the style and correctness checklist.
|
||||
|
||||
---
|
||||
|
||||
# Orientation: the AST and the visitor model
|
||||
|
||||
- **Everything is an `AstNode`.** Each construct is an `Ast*` subclass (`AstAdd`,
|
||||
`AstVar`, `AstIf`). The design under analysis is one tree, with statement lists
|
||||
threaded by sibling `nextp()` links.
|
||||
- **Children sit in numbered slots `op1p()`..`op4p()`, accessed by name.** Use the
|
||||
named accessors (`lhsp()`, `condp()`, `thensp()`), not the raw slots -- the
|
||||
numbering is an implementation detail.
|
||||
- **`astgen` generates node boilerplate.** Declare children and cross-node
|
||||
pointers with `@astgen op` / `@astgen ptr` annotations in the `V3AstNode*.h`
|
||||
headers; it emits accessors, clone, and broken-check code. Do not hand-write
|
||||
what astgen can generate.
|
||||
- **A pass is a visitor.** Convention: a class with a private constructor and a
|
||||
static `apply()` entry point, named after its file (`TimingSuspendableVisitor`
|
||||
in `V3Timing.cpp`). It walks the tree through `visit(AstFoo*)` handlers and
|
||||
`iterateChildren()`. To understand a pass, read its top-of-file comment first --
|
||||
every `.cpp` opens with one describing the algorithm.
|
||||
- **Scratch state lives on nodes.** Passes stash data in `user1p()`..`user5p()`
|
||||
(and `user1()`..`user5()`), claimed for the pass lifetime with a `VNUser*InUse`
|
||||
guard. Save and restore visitor members across recursion with `VL_RESTORER`.
|
||||
- **Three downcasts, three null behaviors:** `VN_IS` returns bool, `VN_CAST`
|
||||
returns nullptr on mismatch, `VN_AS` asserts the type. `V3Broken` re-validates
|
||||
tree invariants between passes, so trust resolved pointers (`dtypep()`,
|
||||
`varp()`) instead of adding defensive null checks for impossible cases.
|
||||
- `docs/internals.rst` is the authoritative reference for the AST, the pass list,
|
||||
and node lifetime.
|
||||
|
||||
---
|
||||
|
||||
# Before you open a PR
|
||||
|
||||
## Code style
|
||||
|
||||
- Mark every variable, parameter, pointer, and member function `const` where possible.
|
||||
- Pointer variables take a `p` suffix and pointer locals are doubly const where
|
||||
possible: `AstVar* const varp`; non-pointers never use the `p` suffix.
|
||||
- Do not use `auto` except for iterators or genuinely unwieldy types.
|
||||
- Use pre-increment (`++i`) unless you specifically need post-increment's old value.
|
||||
- Brace-initialize node and struct construction: `new AstIf{fl, condp, thenp, elsep}`.
|
||||
- Never use C-style casts; instead use `static_cast<T>` for non-AST types and
|
||||
`VN_AS`/`VN_CAST` for AST downcasts.
|
||||
- `static constexpr` for compile-time constants, not `#define` or file-scope const.
|
||||
- Mark every `class`/`struct` `final` or `VL_NOT_FINAL` -- a distribution test
|
||||
scans all definitions.
|
||||
- Keep functions under roughly 100-150 lines; thread shared state through a
|
||||
context struct rather than long parameter lists.
|
||||
- Keep headers lean: move implementation to `.cpp`; convert large lambdas into
|
||||
named member functions -- lambdas are opaque in stack traces and block reuse.
|
||||
- Start every new `.cpp` with a top-of-file comment explaining the algorithm.
|
||||
- Comments are capitalized sentences written for an unknown future reader, without
|
||||
"I"/"we"/"our"; remove commented-out code -- version control preserves history.
|
||||
- No `using namespace`; prefix non-namespaced symbols with `VL`/`Vl`.
|
||||
- Prefer semantic predicates over enum comparisons: `varp->isClassMember()`, not
|
||||
`varp->varType() == VVarType::MEMBER`.
|
||||
- `new*` functions return a `new` object; `make*` functions do something more
|
||||
complex -- pick the prefix accordingly.
|
||||
- Name compiler-created temporaries with a `__V` prefix plus a context suffix
|
||||
(`__VInside`, `__VCase`); runtime utility functions use a `vl_` prefix.
|
||||
- Use `VL_*` bit/word macros from `verilatedos.h` (`VL_WORDS_I`, `VL_MASK_I`); do
|
||||
not include `<cstdint>` directly.
|
||||
- Replace magic numbers with named `static constexpr` constants.
|
||||
|
||||
## AST construction and manipulation
|
||||
|
||||
- Build logic as AST nodes, never as raw C text in `AstCStmt` -- later passes
|
||||
(V3Name, `--protect`) rename AST identifiers but cannot see into raw strings.
|
||||
- Know the cast forms (above) and never pair `VN_IS` with `VN_AS` on the same
|
||||
node -- use a single `VN_CAST`:
|
||||
|
||||
```cpp
|
||||
// BAD: redundant double check
|
||||
if (VN_IS(nodep, VarRef)) { AstVarRef* const refp = VN_AS(nodep, VarRef); }
|
||||
// GOOD: single conditional cast
|
||||
if (const AstVarRef* const refp = VN_CAST(nodep, VarRef)) { ... }
|
||||
```
|
||||
|
||||
- Use `UASSERT_OBJ(cond, nodep, "...")` over `UASSERT` when a node is in scope;
|
||||
use `v3fatalSrc("...")` for unreachable paths, never a silent `if (!ptr) return;`.
|
||||
- Use `VL_DO_DANGLING(pushDeletep(nodep), nodep)` instead of `deleteTree()` in
|
||||
visitors -- deferred deletion is safe against re-entry and unlinking order.
|
||||
`deleteTree()` is only for fresh nodes that never entered the tree.
|
||||
- Every new AST member needs both `dump()` and `dumpJson()` support -- never wrap
|
||||
these in `LCOV_EXCL`; cover them by adding a construct to `t_debug_emitv.v`.
|
||||
Override `isSame()` to include any new semantically meaningful field.
|
||||
- Pointers to nodes outside op1p-op4p require a `broken()` override and
|
||||
`cloneRelink()` support -- avoid storing out-of-tree node pointers when possible.
|
||||
- Never allocate AstNode objects on the stack or by value -- always pointers.
|
||||
- Prefer a new `visit()` in an existing visitor over `nodep->foreach(...)` --
|
||||
better for runtime, and handles diverse node types better. Prefer named
|
||||
accessors over `op1p()`..`op4p()`, and verify traversal order is preserved
|
||||
when converting manual iteration.
|
||||
- Prefer `AstForeach` over generating unrolled loop bodies -- constant-size code
|
||||
instead of O(N); wrap the body in `AstBegin` for scope isolation.
|
||||
- Always `skipRefp()` when comparing or resolving dtypes -- missing it breaks
|
||||
typedef support; prove the paths with typedef tests.
|
||||
- Use `num().isOpaque()` rather than `isDouble() || isString()` when guarding
|
||||
V3Number comparisons against non-integer types.
|
||||
- Use `FileLine::operatorCompare` for source-position ordering -- never hand-roll
|
||||
filename/lineno comparisons.
|
||||
- Identify compiler-generated constructs by an attribute flag on the node (with
|
||||
dump support), never by name-pattern matching -- magic names break with escaped
|
||||
identifiers.
|
||||
- Use `V3Number` arithmetic for `AstConst` values wider than 32 bits -- `1 << i`
|
||||
silently overflows at `i >= 32`.
|
||||
- Use `VMemberMap`/`findMember()` for name lookups in structs, classes, modules,
|
||||
and packages -- O(1) versus quadratic linear scans.
|
||||
- Never emit raw source filenames or identifiers in generated code -- pass them
|
||||
through `protect()`/`putsQuoted` so `--protect` does not leak source.
|
||||
|
||||
## Visitors and passes
|
||||
|
||||
- `VL_RESTORER` on every visitor member a `visit()` modifies before iterating
|
||||
children -- even when nesting "cannot happen" today.
|
||||
- Every pass using `userNp()` needs a `VNUserNInUse` guard, and the header
|
||||
documents which user fields it uses.
|
||||
- Use `iterateAndNextNull()` rather than `iterate()` -- the null-safe form
|
||||
prevents copy-paste errors during refactors.
|
||||
- Derive read-only visitors from `VNVisitorConst` with `iterateChildrenConst`.
|
||||
- Reset per-module visitor state in `visit(AstNodeModule*)`, including numeric ID
|
||||
counters, to keep generated names stable.
|
||||
- Capture first-occurrence module state inside the node's own `visit()` handler,
|
||||
not via a `foreach` pre-scan from `visit(AstNodeModule)` -- source order already
|
||||
matches IEEE declaration-before-use.
|
||||
- Avoid `backp()` -- it returns parent or prior sibling depending on position and
|
||||
causes O(n^2) hunts; build maps or capture context during forward traversal.
|
||||
- When raw node pointers key a map or set, erase entries when the node is deleted
|
||||
-- allocators reuse addresses, so stale entries alias new nodes.
|
||||
- Derive graph-shaped passes from V3Graph (`V3GraphVertex`/`V3GraphEdge`) -- it
|
||||
gives dump, color, rank, topological sort, and reachability for free.
|
||||
- When a change outgrows local rewrites, create a dedicated pass instead of
|
||||
growing an existing one.
|
||||
- State explicitly how side effects are preserved in optimizations involving
|
||||
purity, expression lifting, or simplification.
|
||||
|
||||
## Errors and warnings
|
||||
|
||||
(See the root checklist for choosing the diagnostic API and its required test.)
|
||||
|
||||
- Append `nodep->prettyNameQ()` for user-facing names; use `name()` only in
|
||||
debug/UINFO output. Enclose specific values in single quotes: `'value'`.
|
||||
- End messages with periods, never exclamation marks; do not write "Error:" in the
|
||||
text -- the macro prints the prefix.
|
||||
- State what was attempted and what was found: "Instance attempts to connect to
|
||||
'PARAM' as a parameter, but it is a variable". Add a `warnMore()` suggestion
|
||||
where possible.
|
||||
- Name warning codes object-first and short (`ASCRANGE`, not `RANGEASC`); rename
|
||||
via `renamedTo()` so old suppressions keep working.
|
||||
- Set warning suppression on `AstVar`, not `AstVarRef` -- VarRefs get recreated
|
||||
and lose `warnIsOff`.
|
||||
- "Unsupported:" messages describe the specific unsupported context, not just the
|
||||
construct name -- each message must be distinct.
|
||||
- When replacing or refactoring a pass, keep existing user-facing error strings --
|
||||
`.out` goldens and documentation depend on the wording.
|
||||
|
||||
## Performance and memory
|
||||
|
||||
- O(n^2) is never acceptable -- build maps for batch lookups; any quadratic loop
|
||||
needs explicit justification in a comment.
|
||||
- Prefer `std::map` for per-module structures (many small instances); use
|
||||
`unordered_map` only for one-per-netlist data, and never let `unordered_*`
|
||||
iteration order reach generated output.
|
||||
- Prefer `emplace` over `insert` and check the returned `.second` instead of a
|
||||
separate `find()`. `reserve()` strings and vectors when the size is estimable.
|
||||
- Add no new static or global mutable data -- statics are being eliminated for
|
||||
future parallelism.
|
||||
- Use Verilator's fixed-width data types for model data (`CData`/`SData`/`IData`/
|
||||
`QData`/`VlWide`), not `size_t`. Process wide data word-by-word
|
||||
(`VL_ZERO_W`, `VL_MEMCPY_W`), never bit-by-bit.
|
||||
- No exceptions in verilated runtime code; do string parsing at verilation time,
|
||||
never during simulation.
|
||||
- Wrap unlikely hot-path branches in `VL_UNLIKELY`/`VL_LIKELY`.
|
||||
- Count what every new pass does via V3Stats -- stats become deterministic
|
||||
regression anchors.
|
||||
|
||||
## Thread safety
|
||||
|
||||
- Annotate with the hierarchy `VL_PURE` > `VL_MT_SAFE` > `VL_MT_STABLE`: PURE has
|
||||
no side effects and calls only PURE; MT_SAFE is safe under locks; MT_STABLE is
|
||||
safe only while tree topology is stable. Annotations must match the
|
||||
implementation.
|
||||
- Never include `verilated.h` in the compiler itself -- use `verilatedos.h`.
|
||||
- Annotate mutex-protected members with `VL_GUARDED_BY` and document acquisition
|
||||
ordering. `++` on shared state and container `empty()` are not thread-safe.
|
||||
|
||||
## Parser and lexer (verilog.y, verilog.l)
|
||||
|
||||
- Preserve IEEE Appendix A BNF comments (`// IEEE: {rule}`); comment explicitly
|
||||
when accepting syntax beyond IEEE as an extension.
|
||||
- The parser only builds AST nodes -- defer semantic validation, `VN_IS` checks,
|
||||
and context-dependent logic to V3LinkParse/V3Width and later passes.
|
||||
- Represent hierarchical paths as structured nodes (`AstDot`/parse-ref chains via
|
||||
`idDotted`), never concatenated strings -- preserves per-segment FileLine.
|
||||
- Prefer tightening a grammar rule's operand type over a runtime cast-chain guard
|
||||
in a later visitor -- illegal operands then fail with a clean syntax error.
|
||||
- Solve ambiguities with token-pipeline look-ahead (`tokenPipeScan*`) rather than
|
||||
limiting grammar rules; mark unsupported rules with `//UNSUP`.
|
||||
- Sort token declarations alphabetically by string literal; sort `yD_*`
|
||||
productions by token name.
|
||||
- Add a test for every `|` alternative and optional clause of a new or changed
|
||||
grammar rule -- untested alternatives are where parse regressions hide.
|
||||
|
||||
## File-specific rules
|
||||
|
||||
| File | Rule |
|
||||
|---|---|
|
||||
| `src/V3Options.cpp` | Chain `.notForRerun()` onto `DECL_OPTION()` for options that do not affect semantic output |
|
||||
| `src/V3Ast.cpp` | For composite types (queues, dynamic arrays) use `computeCastableImp()` on subtypes -- shallow `width()`/`similarDType()` checks miss nested incompatibility |
|
||||
| `src/V3AstNode*.h` | Every node class gets a what-construct comment and every member a semantic-purpose comment; put enum type definitions in `V3AstAttr.h` |
|
||||
| `src/V3AstNodeExpr.h` | `CCast` is only for basic C types (char/short/int/QData) -- never 4-state logic or structs |
|
||||
| `src/V3AstNodeOther.h` | `cloneRelink` must propagate all stateful flags (e.g. `maybePointedTo`) and update internal references |
|
||||
| `src/V3Const.cpp` | Check `keepIfEmpty` before removing empty functions -- flagged functions must survive for codegen/side effects |
|
||||
| `src/V3Coverage.cpp` | Instrumentation contexts are opt-in (allowlist), never blocklist -- blocklists silently break when new contexts appear |
|
||||
| `src/V3Inline.cpp` | Preserve `VarXRef::varp()` during passes -- pin-reconnection needs it before V3LinkDot re-resolves |
|
||||
| `src/V3Sched*.cpp` | Every change needs a test proving necessity; isolate unrelated scheduler changes into separate PRs -- high-risk area |
|
||||
|
|
@ -0,0 +1,128 @@
|
|||
<!-- DESCRIPTION: Verilator: test_regress/ guidelines for AI coding agents
|
||||
SPDX-FileCopyrightText: 2026-2026 Wilson Snyder
|
||||
SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -->
|
||||
|
||||
# test_regress/ -- regression tests
|
||||
|
||||
Covers `.v`/`.sv` sources, `.py` drivers, and `.out` golden files under
|
||||
`test_regress/`. Read the repository-root [AGENTS.md](../AGENTS.md) first. This
|
||||
file has two parts: **Orientation** explains how the harness runs a test;
|
||||
**Before you open a PR** is the test-authoring checklist.
|
||||
|
||||
---
|
||||
|
||||
# Orientation: how the harness works
|
||||
|
||||
- **A test is a `.v`/`.sv` source plus a matching `.py` driver in `t/`.** The
|
||||
driver calls `test.compile()`, `test.execute()`, and/or `test.lint()`. Without a
|
||||
`.py` the source never runs and is dead code giving false coverage confidence.
|
||||
- **Golden output lives in `.out` files**, compared via `expect_filename`.
|
||||
Generate them with `HARNESS_UPDATE_GOLDEN=1 python3 t/<name>.py` -- never
|
||||
hand-write them; the harness normalizes paths, version markers, and line numbers
|
||||
that hand edits get wrong.
|
||||
- **Run one test from the repository root:** `test_regress/t/t_<name>.py`. When
|
||||
running from a checkout, set `VERILATOR_ROOT` to the checkout root first so the
|
||||
harness compiles the generated C++ against the right headers.
|
||||
- **`test.compile()` defaults are richer than they look:** the vlt driver
|
||||
auto-injects `--exe` and a generated main, and `assert property` fires its
|
||||
action blocks without `--assert` -- try the simple form before adding flags.
|
||||
- **The `t_dist_*` tests enforce repository conventions** (file headers, sorted
|
||||
lists, warning documentation, ASCII-only) -- run the relevant ones before
|
||||
submitting.
|
||||
|
||||
---
|
||||
|
||||
# Before you open a PR
|
||||
|
||||
## Naming
|
||||
|
||||
- Name tests `t_{category}_{description}` in snake_case; the first word groups the
|
||||
category (`t_lint_unused_func_bad`, not `t_unused_func_lint_bad`) so related
|
||||
tests are findable and runnable together.
|
||||
- Use `_bad` when the code is illegal SystemVerilog, `_unsup` when it is legal but
|
||||
unsupported, `_off` for disabled-behavior tests -- never `_fail`.
|
||||
- Keep filenames under roughly 30-35 characters.
|
||||
|
||||
## Files and drivers
|
||||
|
||||
- Every `.v` needs a matching `.py` driver that actually calls `test.compile()`
|
||||
and `test.execute()` (or `test.lint()` for static-analysis-only tests) -- a
|
||||
driver that sets up but never runs hides coverage gaps silently.
|
||||
- Copy the license header from an existing file: `.v` tests carry the CC0 notice,
|
||||
`.py` drivers carry the standard driver header -- distribution tests check
|
||||
headers on every committed file.
|
||||
- Use `--binary` instead of `--exe --main --timing` -- it implies the others.
|
||||
- Error tests use `test.compile(fails=True, expect_filename=test.golden_filename)`
|
||||
(or `test.lint(...)`) and must not call `test.execute()`.
|
||||
- Use `.out` golden files with `expect_filename` instead of inline `expect`
|
||||
regex strings -- goldens are diffable and maintainable.
|
||||
- Use `test.glob_one()`/`test.glob_some()` and `test.timeout()` instead of raw
|
||||
`glob.glob()` and manual `time.time()` measurements.
|
||||
- Coverage tests run `verilator_coverage` and verify individual bins and points,
|
||||
not just aggregate percentages.
|
||||
- Assert optimization stats with exact expected counts:
|
||||
`test.file_grep(test.stats, r'Optimizations, ...\s+(\d+)', N)`.
|
||||
- Add a `_protect_ids` variant when a feature emits user identifiers or filenames;
|
||||
use conservative thread counts (2 or fewer) in multithreaded tests.
|
||||
- Extend existing test files with related cases instead of creating many
|
||||
single-purpose files; keep drivers minimal -- test logic belongs in the `.v`.
|
||||
|
||||
## Golden .out files
|
||||
|
||||
- Never hand-write or hand-edit `.out` goldens -- regenerate with
|
||||
`HARNESS_UPDATE_GOLDEN=1`.
|
||||
- When a feature lands, remove its now-supported entries from `t_*_unsup.v` /
|
||||
`t_*_bad.v` in the same change and regenerate the goldens -- stale entries no
|
||||
longer error and reviewers will flag them.
|
||||
|
||||
## Verilog style
|
||||
|
||||
- 2-space indentation, no tabs.
|
||||
- Declarations are flush-left with a single space between type and name; never
|
||||
column-align:
|
||||
|
||||
```systemverilog
|
||||
// WRONG: column-aligned
|
||||
bit [63:0] crc = 64'h5aef0c8d_d70a4497;
|
||||
int cyc = 0;
|
||||
// RIGHT: flush-left
|
||||
bit [63:0] crc = 64'h5aef0c8d_d70a4497;
|
||||
int cyc = 0;
|
||||
```
|
||||
|
||||
- Run `nodist/verilog_format` on new `.v` files; wrap macro definitions in
|
||||
`// verilog_format: off`/`on` so the formatter does not split them.
|
||||
- Use `$display("%0d", ...)` not `%d` -- avoids leading-space padding.
|
||||
- Wrap Verilator-specific test code (e.g. `$c`) in `` `ifdef VERILATOR ``.
|
||||
- Use inline `// verilator lint_off WARNCODE` only when that warning is itself
|
||||
under test -- fix root causes otherwise.
|
||||
- Use only IEEE 1800-compliant constructs other simulators also accept -- tests
|
||||
validate standard behavior, not Verilator's parser leniency.
|
||||
- Omit optional end labels on `endmodule`/`endclass`/`endtask`/`endfunction`.
|
||||
|
||||
## Self-checking
|
||||
|
||||
- Use the `checkh`/`checkd`/`checks` assertion macros, not manual
|
||||
`if`/`$display`/`$stop` sequences. Note `checkh` prints with `%p`, which renders
|
||||
integers as hex -- use `checkd` for integer comparisons.
|
||||
- Use the `` `stop `` macro rather than direct `$stop`.
|
||||
- Drive logic with runtime-varying inputs (counters, CRC/LFSR registers) so
|
||||
constant folding cannot pre-evaluate the logic under test; check behavior across
|
||||
multiple clock cycles, not just initial values.
|
||||
- For a test whose pass/fail depends on varying or random values, loop enough
|
||||
iterations that the values demonstrably differ and size the value space so a
|
||||
failure is probable per run, then confirm the test fails on an un-fixed tree
|
||||
before submitting.
|
||||
|
||||
## Test design
|
||||
|
||||
- Use non-power-of-2, non-word-aligned widths (7, 15, 31, 33, 63, 65, 95) --
|
||||
exposes masking and word-boundary bugs that 32/64/128 hide.
|
||||
- Test both `[high:low]` and `[low:high]` orderings plus non-zero bounds like
|
||||
`[3:1]`; use different ranges for each axis of multidimensional arrays.
|
||||
- When adding type support, test all basic types (chandle, string, real) and
|
||||
typedef-wrapped variants.
|
||||
- Include the issue's own reproducer as a committed test, and verify it fails
|
||||
without the fix.
|
||||
- Assert non-blocking-assignment results in the cycle immediately after they take
|
||||
effect, before later overwrites, using indices that change post-NBA.
|
||||
Loading…
Reference in New Issue