Conversation
This commit includes two main fixes:
1. **fix(cmake): Prevent unnecessary recompilation with Verilator**
Sets CMake policy CMP0058 to NEW in the Verilator configuration.
This corrects an issue where CMake would incorrectly detect that
Verilator-generated files were newer than the dependency database,
causing unnecessary recompilations and slowing down the build process.
2. **fix(makefile): Improve xcomm dependency branch handling**
The Makefile logic for checking out the dependency branch
has been made more robust. It now correctly handles detached HEAD
states (common in CI/CD environments) by safely falling back to the
master branch. This prevents build failures in automated environments.
The previous 'git fetch' command was not specific enough, causing
failures when the local remote-tracking branch for the xcomm dependency
was stale or did not exist.
This change uses an explicit refspec ('<src>:<dst>') to ensure the
remote branch is fetched and the local remote-tracking ref is
correctly created or updated before the checkout command is run.
This makes the branch alignment logic more robust.
Add a new `atClone()` method to the simulation core and expose it through all language bindings. This method is intended to be called in a child process after a fork() to re-initialize the state of the Verilated model. This is crucial for supporting robust multi-process simulation environments, such as Python's `multiprocessing`, where forking can lead to an inconsistent state in the simulator's child instances. Additionally, this commit includes several stability and memory management improvements: - Fix memory leaks in `DutUnifiedBase` and the C++ wrapper by ensuring allocated resources are properly deallocated. - Add a null-pointer check in `FlushWaveform` to prevent crashes when a waveform dumper is not present. - Expose the `atClone` functionality to C++, Go, Java, Lua, Python, and Scala wrappers.
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive support for multi-process simulation through the atClone() API across all language bindings (C++, Go, Java, Lua, Python, Scala), fixes memory leaks in the C++ wrapper and DutUnifiedBase, improves waveform dump handling in Verilator, expands filelist parsing with include directory support, adds extensive unit tests for parser and codegen functionality, and streamlines the initialization workflow.
Changes:
- Adds atClone() hook for post-fork state re-initialization across all language bindings and fixes memory leaks in C++ wrapper destructor
- Enhances filelist parsing to handle include directories (+incdir+, -I flags) and adds related unit tests
- Adds 10+ new unit tests covering parser (Verilator/gsim/SV/internal config) and codegen (filelist/SV params/recursive render) functionality with shared test utilities
- Defers Verilator waveform pause until dumper exists to prevent crashes and adds null check in FlushWaveform
- Refactors initialization workflow into scripts/init.sh with more robust xcomm dependency fetching and branch alignment logic
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| template/lib/dut_base.hpp | Adds atClone() virtual method to DutBase and DutVerilatorBase; adds wave_pause_deferred and wave_pause_warned flags |
| template/lib/dut_base.cpp | Implements atClone() for DutVerilatorBase and DutUnifiedBase; adds deferred wave pause logic; fixes argv memory leak; adds null check in FlushWaveform |
| template/cpp/dut.hpp | Exposes AtClone() method in wrapper class |
| template/cpp/dut.cpp | Implements proper cleanup in destructor for signal_map and xcfg; exposes AtClone() |
| template/python/dut.py | Adds AtClone() method wrapper with documentation |
| template/java/dut.java | Adds AtClone() method wrapper |
| template/scala/dut.scala | Adds AtClone() method wrapper |
| template/golang/dut.go | Adds AtClone() method wrapper |
| template/lua/dut.lua | Adds AtClone() method wrapper |
| template/mem_direct/Makefile | Reorders linker flags (CXXFLAGS moved to end) |
| template/lib/cmake/verilator.cmake | Adds CMake policy CMP0058 and --MMD --MP flags to Verilator |
| src/codegen/lib.cpp | Adds include directory parsing (+incdir+, -I) to gen_filelist and append_incdirs_to_vflag function |
| scripts/init.sh | New script consolidating verible check and xcomm dependency management with branch alignment logic |
| Makefile | Refactors init target to use scripts/init.sh; adds explicit build step to unit_tests target |
| CMakeLists.txt | Adds enable_testing() guarded by is_top_level_project check |
| test/test_common.cpp | New shared test utilities providing stubs for globals and functions needed by test executables |
| test/test_parser_verilator_root_unit.cpp | New unit test for Verilator header parsing |
| test/test_parser_gsim_root_unit.cpp | New unit test for gsim code parsing |
| test/test_parser_sv_collect_unit.cpp | New unit test for SV file collection from filelists |
| test/test_parser_map_unit.cpp | New unit test for parser factory functions |
| test/test_parser_internalcfg_unit.cpp | New unit test for internal pin configuration parsing |
| test/test_codegen_filelist.cpp | New unit test for filelist generation with include directory handling |
| test/test_codegen_sv_param.cpp | New unit test for SV parameter generation |
| test/test_codegen_sv_param_multi.cpp | New unit test for multi-instance SV parameter generation |
| test/test_codegen_recursive_render.cpp | New unit test for recursive template rendering |
| test/test_parser_internalcfg.cpp | Updates path to use PICKER_EXAMPLE_DIR macro instead of relative path |
| test/CMakeLists.txt | Adds new unit tests to build and test configuration; adds PICKER_EXAMPLE_DIR and PICKER_TEST_DATA_DIR |
| test/cmake/*.cmake | New CMake configuration files for each unit test |
| test/data/VTop___024root.h | New test data file for Verilator parser tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static void write_text(const std::filesystem::path &p, const std::string &t) | ||
| { | ||
| std::ofstream o(p, std::ios::trunc); | ||
| o << t; | ||
| } |
There was a problem hiding this comment.
The write_text helper does not check if the file stream opened successfully before writing. If the file cannot be created or opened for writing, the write operation will silently fail. Consider adding error checking.
Description
Summary of changes:
Motivation/Context:
Dependencies:
Fixes # (issue)
Type of change
Checklist: