Conversation
kroening
marked this pull request as draft
July 21, 2026 14:37
kroening
marked this pull request as ready for review
July 21, 2026 15:47
kroening
force-pushed
the
kroening/monolithic-verilog-synthesis
branch
from
July 21, 2026 15:48
30ce281 to
6b505f1
Compare
tautschnig
reviewed
Jul 22, 2026
Comment on lines
+4111
to
+4114
| // clean up | ||
| assignments.clear(); | ||
| invars.clear(); | ||
|
|
Collaborator
There was a problem hiding this comment.
It's a somewhat hidden invariant that those need to be cleaned first. Maybe the should actually become local variables and be passed to synth_module_items and synth_assignments? Though I also wonder whether invars is necessary at all - couldn't trans.invar() be populated directly?
kroening
force-pushed
the
kroening/monolithic-verilog-synthesis
branch
from
August 11, 2026 02:21
6b505f1 to
6600a4b
Compare
Synthesis used to be modular: each module instance was synthesized in its own verilog_synthesist object, with its constraints generated before being folded into its parent. This meant an assignment made via a hierarchical identifier that reaches across an instance boundary (in either direction) was invisible to whichever module's constraints had already been finalized, silently producing an over-constrained transition system rather than an error. Synthesis is now monolithic: the whole instance hierarchy is expanded using one shared verilog_synthesist object (synth_module_items), and constraints are only generated once all assignments have been collected. This fixes regression/verilog/interface/instance4.desc (previously a KNOWNBUG), and adds hierarchical_identifiers5, which pins down the same defect for a plain module instance whose own always block competes with a hierarchical assignment from the enclosing module. regression/verilog/modules/ref_port1.desc is moved to KNOWNBUG: making synthesis monolithic exposes a pre-existing gap in how 'ref' ports are modeled (as a driven connection rather than a true alias), which is deferred to a follow-up change.
kroening
force-pushed
the
kroening/monolithic-verilog-synthesis
branch
from
September 14, 2026 18:06
6600a4b to
13eac1f
Compare
5 tasks
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
Synthesis used to be modular:
synth_module_instancerecursively invoked a freshverilog_synthesistobject per instantiated module, generating that module's constraints (synth_assignments) before folding the result into its parent. Because the assignment bookkeeping (assignments,local_symbols) was scoped to each of these separate objects, an assignment reaching across an instance boundary via a hierarchical identifier (in either direction) was invisible to whichever module's constraints had already been finalized. This silently produced an over-constrained transition system (e.g. a spurious self-hold alongside the real next-state equation) rather than an error — in the worst case making the transition relation UNSAT beyond the initial state, so every safety property is vacuously, wrongly "proved".synth_module_itemsnow recursively expands the entire instance hierarchy in place, using one sharedverilog_synthesistobject, soassignments/local_symbolsaccumulate across the whole design.synth_assignments(which generates the actual constraints) now runs exactly once, after the whole hierarchy has been visited.default_disable_iffis explicitly saved/restored per module, since it's no longer implicitly scoped by a fresh object per module.typecheck_module, later wrapped under$root) is preserved.Test plan
regression/verilog/interface/instance4.desc: promoted fromKNOWNBUGtoCORE(this is exactly the bug — an interface's counter driven both by its ownalwaysblock and a hierarchical identifier from the enclosing module; previously stayed vacuouslyPROVEDeven at bound 20). Bound corrected from 10 to 11 to match the counter's actual dynamics.regression/verilog/hierarchical_identifiers/hierarchical_identifiers5.{sv,desc}: same shape using a plain module instance rather than an interface. Verified it reports the wrongPROVED/EXIT=0on the old code and the correctREFUTED/EXIT=10with this change.regression/verilog/modules/ref_port1.descmoved toKNOWNBUG: making synthesis monolithic exposes a pre-existing gap in howrefports are modeled (as a driven/continuous connection rather than a true alias for the bound variable), which now surfaces as a conversion error instead of silently working by accident. Deferred to a follow-up change; the comment in the.descdocuments the cause.regression/verilogsuite (917 tests, all levels) passes.regression/ebmcsuite passes.