Repository navigation
perf(sv-analyzer): materialize referenced parameter values - #1193
Merged
Merged
Conversation
tignear
marked this pull request as ready for review
October 11, 2026 05:56
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Generate-item collection expanded every parameter into a literal or symbolic expression during preliminary function discovery. An initializer using a function not yet installed retained symbolic calls, so storing all dependency prefixes copied progressively longer trees. Reusing the previous value twice could multiply unused expression trees at every declaration.
Collect references from non-parameter items and materialize their iterative initializer-dependency closure. Preserve the full declaration-order type prefix and inherited numeric precedence. Package bodies, array initializers, escaped identifiers, and nested/inactive generate branches remain conservative reference roots. The full-map API stays available to other consumers. Unrequested unbounded values are omitted while requested values and aliases retain their original representation.
Synced master through
bd10acc4c, including #1175, #1182, $unit support, and$isunboundedsupport. The merge resolved appended-test conflicts and adjusted the new sparse path for the unbounded early return.Validation after synchronization:
cargo fmt --all -- --checkcargo clippy --locked -p celox-sv-analyzer -p celox-frontend-sv --all-targets --all-features -- -D warningscargo test --locked -p celox-sv-analyzer -p celox-frontend-sv --all-features --quiet: 259 passed; four manual timing probes excluded from the normal suite.cargo test --locked -p celox --features sv-dpi --test systemverilog --test sv_unpacked_array --test sv_dpi --test sv_packages --test interface --test advanced_interface --quiet: SystemVerilog 1,740, DPI 52, arrays 7, packages 46, interfaces 216 passed; 39 existing interface exclusions.cargo test --locked -p celox-test-suite --features verilator,icarus --quiet: 56 tests and two doctests passed; nine existing external-oracle tests remain ignored. This diff changes no shared executable cases or adapters, so external evidence is reused under CONTRIBUTING.md.cargo doc --locked -p celox-sv-analyzer -p celox-frontend-sv --no-depsandpnpm docs:buildpassed.Owning-crate tests compare projected entries with the full map for four-state, signed/wide values, selects, conditionals, forward/self/cyclic references, repeated names, inferred types with numeric overrides, and unbounded aliases. Cold item collection stores no unused literals for incrementing and doubling constant-function chains at 16/64/256 parameters. Every resulting IR value is checked, including signed 32-bit wraparound. The 256-element chains now pass on the default test-thread stack.
The optimized paired probe
compare_full_and_referenced_parameter_values -- --ignored --nocapturepassed: for a 512-parameter uninstalled-function chain with only P0 requested, full materialization took 79.692 ms versus 0.148 ms for projection (seven alternating-run medians). It checks the retained value and includes map destruction, excluding parsing, declaration collection, and numeric function execution. The old deep comparison uses a 16 MiB thread stack.Before the subsequent
$isunboundedmaster synchronization, the standalone probe compared65dff8a23with this projection change. At 512 parameters, separate three-sample medians were 325.627 → 222.730 ms for AST lowering (parse 61.172 → 57.386 ms), with IR value assertions. Full tables and reproduction commands are documented. Shared-machine timing varies; requested unresolved roots still expand their dependency prefixes, repeated live references can still multiply those trees, and conservative roots can retain extra values. This removes unused expression expansion rather than establishing universal linear scaling.