Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 45 additions & 22 deletions compiler/frontend/pycircuit/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1440,6 +1440,10 @@ def _collect_testbench_payload(
)


def _has_testbench_entrypoint(mod: object) -> bool:
return hasattr(mod, "tb") and callable(getattr(mod, "tb"))


def _emit_testbench_pyc_file(
*,
out_dir: Path,
Expand Down Expand Up @@ -1751,6 +1755,15 @@ def _cmd_build(args: argparse.Namespace) -> int:
)
sys.stdout.write("jit-cache: miss\n")

target = str(args.target)
do_cpp = target in {"cpp", "both"}
do_v = target in {"verilator", "both"}
has_tb = _has_testbench_entrypoint(mod)
if not has_tb and do_v:
raise SystemExit(
"DUT-only build without @testbench currently supports --target cpp only; "
"Verilator/SV simulation requires `@testbench def tb(t: Tb): ...`"
)
pycc = _detect_pycc()
jobs = max(1, int(args.jobs))
if int(args.logic_depth) <= 0:
Expand All @@ -1761,10 +1774,6 @@ def _cmd_build(args: argparse.Namespace) -> int:
device_v_root = out_dir / "device" / "verilog"
device_cpp_root.mkdir(parents=True, exist_ok=True)
device_v_root.mkdir(parents=True, exist_ok=True)

target = str(args.target)
do_cpp = target in {"cpp", "both"}
do_v = target in {"verilator", "both"}
pycc_build_profile = "dev-fast" if str(args.profile) == "dev" else "release"
pycc_hard_hierarchy_flags = [
f"--build-profile={pycc_build_profile}",
Expand Down Expand Up @@ -1846,24 +1855,29 @@ def _cmd_build(args: argparse.Namespace) -> int:
except TraceConfigError as e:
raise SystemExit(f"trace config error: {e}") from e

tb_probes = TbProbes.from_probe_manifest(probe_manifest_obj)
tb_name, tb_payload_json = _collect_testbench_payload(
mod, iface, trace_plan=trace_plan, tb_probes=tb_probes
)
tb_pyc_path = _emit_testbench_pyc_file(
out_dir=out_dir, tb_name=tb_name, payload_json=tb_payload_json
)
manifest["testbench"] = {
"name": tb_name,
"pyc": str(tb_pyc_path.relative_to(out_dir)),
}
tb_name = ""
tb_pyc_path: Path | None = None
if has_tb:
tb_probes = TbProbes.from_probe_manifest(probe_manifest_obj)
tb_name, tb_payload_json = _collect_testbench_payload(
mod, iface, trace_plan=trace_plan, tb_probes=tb_probes
)
tb_pyc_path = _emit_testbench_pyc_file(
out_dir=out_dir, tb_name=tb_name, payload_json=tb_payload_json
)
manifest["testbench"] = {
"name": tb_name,
"pyc": str(tb_pyc_path.relative_to(out_dir)),
}
else:
manifest["testbench"] = None
if trace_plan is not None:
trace_path = out_dir / "trace_plan.json"
_save_json(trace_path, trace_plan.as_dict())
manifest["trace_plan"] = str(trace_path.relative_to(out_dir))

tb_cpp_out = out_dir / "tb" / f"{tb_name}.cpp"
tb_sv_out = out_dir / "tb" / f"{tb_name}.sv"
tb_cpp_out = out_dir / "tb" / f"{tb_name}.cpp" if has_tb else None
tb_sv_out = out_dir / "tb" / f"{tb_name}.sv" if has_tb else None
for sym in sorted(module_paths.keys()):
mp = module_paths[sym]
h = _module_hash(mp)
Expand Down Expand Up @@ -1915,7 +1929,7 @@ def _cmd_build(args: argparse.Namespace) -> int:
)
)

if do_cpp:
if do_cpp and has_tb and tb_pyc_path is not None and tb_cpp_out is not None:
tb_key = f"tb:{tb_name}"
tb_hash = _module_hash(tb_pyc_path)
module_hashes[tb_key] = tb_hash
Expand All @@ -1933,7 +1947,7 @@ def _cmd_build(args: argparse.Namespace) -> int:
],
)
)
if do_v:
if do_v and has_tb and tb_pyc_path is not None and tb_sv_out is not None:
tb_key = f"tb:{tb_name}"
tb_hash = module_hashes.get(tb_key) or _module_hash(tb_pyc_path)
module_hashes[tb_key] = tb_hash
Expand Down Expand Up @@ -1962,7 +1976,7 @@ def _cmd_build(args: argparse.Namespace) -> int:
cpp_sources = _gather_cpp_sources(device_cpp_root)
if not cpp_sources:
raise SystemExit("build(cpp): no generated C++ sources found")
if not tb_cpp_out.is_file():
if has_tb and (tb_cpp_out is None or not tb_cpp_out.is_file()):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check tb_cpp_out is None is redundant here. Since this is inside an if has_tb block, tb_cpp_out is guaranteed to be a Path object based on its initialization at line 1879.

Suggested change
if has_tb and (tb_cpp_out is None or not tb_cpp_out.is_file()):
if has_tb and not tb_cpp_out.is_file():

raise SystemExit(
f"build(cpp): missing generated TB C++ source: {tb_cpp_out}"
)
Expand All @@ -1979,7 +1993,7 @@ def _cmd_build(args: argparse.Namespace) -> int:
build_manifest = {
"version": 3,
"target_name": iface.sym,
"tb_cpp": str(tb_cpp_out),
"tb_cpp": str(tb_cpp_out) if tb_cpp_out is not None else None,
"sources": [str(p) for p in cpp_sources],
"headers": [str(p) for p in cpp_headers],
"include_dirs": include_dirs,
Expand All @@ -1989,6 +2003,15 @@ def _cmd_build(args: argparse.Namespace) -> int:
}
cpp_manifest = out_dir / "cpp_project_manifest.json"
_save_json(cpp_manifest, build_manifest)
manifest["cpp_project_manifest"] = str(cpp_manifest.relative_to(out_dir))

if not has_tb:
manifest["cpp_sources"] = [str(p.relative_to(out_dir)) for p in cpp_sources]
manifest["cpp_headers"] = [str(p.relative_to(out_dir)) for p in cpp_headers]
manifest["cpp_executable"] = None
_save_json(manifest_path, manifest)
sys.stdout.write(f"{manifest_path}\n")
return 0
Comment on lines +2010 to +2016
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The early return for DUT-only builds bypasses the build cache update logic at the end of the function (lines 2149-2161). This means that subsequent builds will not benefit from the cache (e.g., module_hashes and jit_cache_key won't be persisted), leading to unnecessary re-compilations on every run.

Instead of returning early, consider wrapping the testbench-specific logic (the CMake generation/build and Verilator blocks) in an if has_tb: condition so the function can reach the common cache and manifest persistence logic at the end.


gen_script = _tool_script("gen_cmake_from_manifest.py")
cmake_src = out_dir / "cpp_build" / "src"
Expand Down Expand Up @@ -2042,7 +2065,7 @@ def _cmd_build(args: argparse.Namespace) -> int:
manifest["cpp_executable"] = str(cmake_build / "pyc_tb")

if do_v:
if not tb_sv_out.is_file():
if tb_sv_out is None or not tb_sv_out.is_file():
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check tb_sv_out is None is redundant. If do_v is true, the check at line 1762 ensures that has_tb is also true, which in turn guarantees that tb_sv_out is a Path object (initialized at line 1880).

Suggested change
if tb_sv_out is None or not tb_sv_out.is_file():
if not tb_sv_out.is_file():

raise SystemExit(
f"build(verilator): missing generated TB SV source: {tb_sv_out}"
)
Expand Down
11 changes: 10 additions & 1 deletion docs/PIPELINE.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Frontend responsibilities:
- materialize `@module(value_params=...)` as runtime boundary input ports
- emit one `.pyc` per specialized module
- emit a deterministic `project_manifest.json`
- emit a testbench `.pyc` payload from `@testbench`
- emit a testbench `.pyc` payload from `@testbench` when present

All emitted modules are stamped with:
- `pyc.frontend.contract = "pycircuit"`
Expand Down Expand Up @@ -49,6 +49,15 @@ Build a project (multi-module + testbench):
python3 -m pycircuit.cli build <tb_or_top.py> --out-dir <dir> --target cpp|verilator|both --jobs <N>
```

Build a DUT only for an external C++ driver:

```bash
python3 -m pycircuit.cli build <top.py> --out-dir <dir> --target cpp --jobs <N>
```

In DUT-only mode the manifest has `"testbench": null`; C++ device sources and
headers are still generated, but no generated pyCircuit TB executable is built.

Simulation (Verilator):

```bash
Expand Down
8 changes: 7 additions & 1 deletion docs/TESTBENCH.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,13 @@ def tb(t: Tb):
t.finish(at=10)
```

`pycircuit build` expects `tb` to be decorated with `@testbench`.
`pycircuit build` can also compile a DUT-only source that does not define
`tb(...)` when the target is `cpp`. In that mode pyCircuit emits the device
`.pyc` modules, generated C++ device sources, and `cpp_project_manifest.json`,
but it does not generate a testbench executable. `verilator` and `both` targets
still require a decorated `@testbench` because they produce simulation harnesses.

If a `tb(...)` function is present, it must be decorated with `@testbench`.

## Tb API (selected)

Expand Down
Loading