fix: filelist and build threads#96
Open
Makiras wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Picker’s export flow by (1) making the SystemVerilog parsing step robust to linker artifacts present in RTL filelists, and (2) adding a user-facing --build-threads option to control parallelism when building generated projects (propagated through generated Makefiles).
Changes:
- Exclude
.so,.a, and.oentries from slang’s RTL parsing inputs while keeping them in generated simulator filelists. - Add
-j,--build-threadstopicker export, plumb it through codegen, and propagate it viaNPROCin generated Makefiles. - Add/extend tests and documentation covering both the filelist behavior and the new build-parallelism option.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/parser/sv.cpp |
Adds slang driver args to exclude linker-input extensions during RTL parsing. |
test/test_parser_sv_unit.cpp |
Adds a unit test ensuring .so/.a/.o in filelists don’t break RTL parsing. |
test/test_codegen_filelist.cpp |
Adds assertions that generated filelists still include .so/.a/.o entries. |
include/type.hpp |
Extends export_opts with build_threads. |
src/picker.cpp |
Adds -j,--build-threads CLI option with validation and default. |
src/codegen/lib.cpp |
Renders __BUILD_THREADS__ for templates (numeric or auto-detect). |
template/lib/Makefile |
Exports NPROC from the generated project root based on __BUILD_THREADS__. |
template/{python,cpp,java,scala,golang,lua}/Makefile |
Switches NPROC to conditional assignment so exported NPROC can override. |
test/scripts/test_export_filelist.sh |
Extends integration test to validate NPROC propagation into generated Makefiles. |
README.md |
Documents the new --build-threads option in the export help snippet. |
README.zh.md |
Documents the new --build-threads option in Chinese documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes RTL filelist handling and adds control over generated-project build parallelism.
The RTL parser now excludes
.so,.a, and.olinker inputs before passing filelists to slang. These filesremain in the generated simulator filelist, so they are still available to Verilator, VCS, and UVS during
compilation and linking.
This PR also adds the
-j,--build-threadsexport option:0, which is the default, continues to use all available CPU cores.make NPROC=<value>.No additional dependencies are required.
Type of change
How Has This Been Tested?
The following checks were performed:
cmake --build build --target picker -j16ctest --test-dir build -R '^test_parser_sv_unit$' --output-on-failurebash test/scripts/test_export_filelist.sh--build-threads 3rendersexport NPROC := 3..so,.a, and.oare ignored by slang but retained in generated simulatorfilelists.
test_codegen_filelistencounters an existing path-string assertion on this environment because/tmp/ picker_test_filelist/and/tmp/picker_test_filelistdiffer only by a trailing slash. This occurs before thenewly added linker-input assertions and is unrelated to this change.
Test Configuration:
Checklist: