opt_vps tests: scope the self-equivalence proofs to gate - #310
Merged
Conversation
Tests 2, 3, 8, 10 and 13 rename `gold` before reading the design a second time and then run a bare `opt_vps`. With no selection argument the pass runs on every module in the design, so `gold` was optimized too and each miter compared optimized against optimized. These were not merely weaker than intended, they were vacuous. Every one of the ten solves ran on 2 variables and 3-5 clauses, because gold and gate were the same netlist. Scoping the pass to `gate` gives 3965, 7764, 18431, 36212, 38697, 76879, 45027, 90052, 79557 and 159112 variables respectively, and all five tests still pass. Tests 1 and 7 build `gate` before `gold` exists and were always real proofs; their solve sizes are unchanged, which confirms the affected set. Worth stating how much this hid: run the same arm in the bare style against a build of opt_vps with a known unsound gather grouping key, one that leaves four output bits with no driver at all, and it still reports "no model found: SUCCESS!". Co-authored-by: Cursor <cursoragent@cursor.com>
3 tasks
Greptile SummaryThe PR repairs five vacuous
Confidence Score: 5/5The PR appears safe to merge and makes the affected equivalence tests meaningfully exercise The explicit trailing
|
| Filename | Overview |
|---|---|
| tests/silimate/opt_vps.ys | Correctly scopes opt_vps to gate in five self-equivalence arms while preserving full-design cleanup and meaningful gold-versus-gate proofs. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
SRC[Import source] --> GOLD[Rename reference to gold]
SRC --> GATE[Import again and rename to gate]
GATE --> VPS[Run opt_vps on gate only]
GOLD --> MITER[Equivalence miter]
VPS --> MITER
MITER --> SAT[SAT proof]
Reviews (1): Last reviewed commit: "opt_vps tests: scope the self-equivalenc..." | Re-trigger Greptile
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.
Summary
Five of the
opt_vpsself-equivalence tests were proving nothing at all. They were not merely weaker than intended — they were vacuous, and I found it while reconciling two duplicate PRs (#294 / the closed #295).Tests 2, 3, 8, 10 and 13 all follow this shape:
opt_vpsends its argument loop withextra_args(...)and then iteratesdesign->selected_modules(). With no selection argument that is every module in the design,goldincluded. Sogoldwas optimized too,goldandgateended up as the same netlist, and each miter compared optimized against optimized.How vacuous
Every one of the ten solves in those five tests ran on 2 variables and 3–5 clauses — the miter had collapsed to nothing. After scoping:
(variables, base case / induction step)
Tests 1 and 7 build
gatebeforegoldexists, so they were always real proofs against a hand-written reference. Their solve sizes are byte-for-byte unchanged (2182/4229 and 13450/26385), which independently confirms the affected set is exactly {2, 3, 8, 10, 13}.Direct evidence the pass really was touching the reference — from a replay of test 8's shape:
and afterwards
goldcontainsgold/$auto$opt_vps.cc:2069:process_vps_reads$vps_rd_shr$2.What this hid
Worth stating concretely, because it is not hypothetical. Take a build of
opt_vpswith a known-unsound gather grouping key (the per-element-bit keying from the closed #295), on a netlist where it demonstrably leaves four output bits with no driver at all —checkreportsWire ...\y_ovl[3] [1] is used but has no driverfour times. An equivalence arm written in the bare style still reports:Scoped to
gate, the same arm on the same binary reportsmodel found: FAIL!.Change
One line per test:
renametogatefirst, thenopt_vps gate; select -clear; opt_clean. 10 insertions, 5 deletions, no fixture or expected-count changes.Test plan
tests/silimate/opt_vps.yssuite passes, exit 0, with a binary built frommain'sopt_vps.ccThe two instances of this pattern in tests 15 and 17 are added by #294, so they are fixed there rather than here.
Made with Cursor