Semantic port properties: derive from the APackage, feed every backend#1058
Closed
nanavati wants to merge 25 commits into
Closed
Semantic port properties: derive from the APackage, feed every backend#1058nanavati wants to merge 25 commits into
nanavati wants to merge 25 commits into
Conversation
The interface-inout definitions (io_ds) were missing from getIOProps' use tracking, so an argument inout whose net is re-exposed at an interface inout pin appeared to have no uses and was labeled "unused" -- one net at two pins, mislabeled at one of them, and the mislabel propagated up parent compiles through the .bo. Record the signals referenced by the interface-inout definitions as live inout sinks (the analog of the existing output_pairs), so both pins are reported live -- agreeing with getIOPropsA, whose accuracy here was previously documented as a divergence. Co-authored-by: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
Cover the fix from more angles: an argument inout fed through to an interface inout across a module boundary (the leaf's argument pin is the regression guard; the parent's interface inout is fed from the leaf's interface inout rather than an argument), two independent argument/interface inout pairs in one module (multiple interface-inout definitions), and an argument inout exposed inside a subinterface. Co-authored-by: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
The existing getIOProps computes Verilog port properties (the "Ports:" comment in generated Verilog and the port properties for the import-BVI) from the final ASPackage, at the end of the Verilog backend pipeline. This adds getIOPropsA, which computes the same structure from an APackage, before scheduling info is folded in by AState. This first version is a basic approximation: it mirrors AState's port construction to enumerate the ports that the generated module will have (method results, output clocks/gates/resets, module arguments, method arguments and enables, and inouts, with the same names and in the same order), and assigns the properties which are structurally known at the APackage level (clock, clock gate, reset, inout, and any properties declared in the VArgInfo/VFieldInfo), rather than deducing properties from the generated netlist. The result is dumped under a new flag, -dAPackageIOproperties, which allows comparison against the -dIOproperties dump of the ASPackage-based pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
Extend the APackage version of the VIOProps pass beyond structurally declared properties, mirroring the deduction that getIOProps performs on the final ASPackage: For outputs, follow the interface value definitions (and the wires of output clocks, resets, and inouts) through the local defs, deducing "const" for constant expressions, and taking the declared properties of the connected ports for method calls on state instances (e.g. "reg" for a register's Q_OUT), for special outputs (clocks, resets, inouts) of state instances, and for output clock gates. Concats and extractions join properties as in getIOProps. For inputs, classify every use of a signal in the package: uses in local defs are followed transitively; direct connections (through concat/extract only, as in okUse) to state instance arguments or to method input ports contribute those ports' declared properties; and any other use (predicates, enables, foreign calls, non-trivial logic) contributes no properties but counts as a use. An input with no uses is "unused"; properties common to all uses are assigned, with the same unused-filtering as getIOProps' joinInProps. Connections to method arguments only count as direct when the method has enough port copies for all its call sites (otherwise AState will insert muxes). The structural properties are merged with the deduced ones, so clock, gate, and reset ports keep their structural labels even where the netlist deduction of getIOProps would lose them (e.g. an unused input clock is now "clock unused"). A sweep over the testsuite (~110 designs reaching codegen) shows the port enumeration always matches getIOProps, and the properties match except for that intentional retention of structural labels and a small number of safe-direction misses where the property only becomes deducible after scheduling and netlist optimization (noted in the module comment). Also lift joinInProps and joinOutProps to the top level so both passes share them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
RWire, RWire0, and BypassWire instances are inlined away after AState (by InlineWires), so the ASPackage-based getIOProps sees through them as plain wires. Teach the APackage-based deduction to do the same: * A "wget" result is a reference to the wire's data node: for a single setter the "wset" argument flows through directly (so an input which passes through a wire into a register's D_IN is now labeled "reg"); with multiple setters the value passes through a mux, so properties do not flow, but use/unused information still does. A never-set wire's value is a constant. * A "whas" result is enable logic (or constant False if the wire is never set) and uses no signals. * An input whose only use is setting a wire that is never read is now labeled "unused" (matching what getIOProps sees after the dead wire is dropped). The look-through honors the same flags as the inlining itself (removeRWire, and removeCross for clock-crossing BypassWires). CReg instances (inlined by InlineCReg) are not looked through. Also label any primitive whose operands are all constant as "const", since netlist optimization will fold it (e.g. a fromMaybe of a never-set RWire, where both the validity and the data are constants). A sweep over ~260 testsuite designs reaching codegen shows no crashes, identical port enumeration, no properties asserted that getIOProps would contradict, and the remaining misses reduced to cases requiring scheduling knowledge (e.g. ready signals that scheduling reduces to constants) plus the intentional foreign-call difference noted in the module comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
…rules) The "unused" property describes the generated hardware, so align getIOPropsA's notion of a use with getIOProps': * Arguments (and conditions) of foreign function and task calls exist only in simulation (system tasks and imported DPI functions), so they are no longer counted as uses; a signal consumed only by a $display is now "unused", as getIOProps reports. Method calls appearing inside such arguments still wire up the method's input ports in the hardware, so their arguments are still classified. * Model rule liveness: the WILL_FIRE of a rule is used only by the hardware its actions create (method enables and argument muxes, and the data/validity wires of inlined-wire sets, which live or die with their readers), while foreign actions contribute no hardware use. References to WILL_FIREs from conflicting rules' scheduling logic are already visible in the defs added by AAddScheduleDefs. Since those defs also connect each method's WILL_FIRE to its enable port, enable inputs can now be deduced like any other input: the enable of a method whose actions produce no hardware (e.g. only a $display, or only setting a wire that is never read) is "unused". * When logic (rather than a direct connection) consumes a signal, the use now lives or dies with the definition or wire the logic feeds (opaqueOf), so "unused" propagates backwards through dropped logic, e.g. from an unused definition to the signals it reads. With this, a sweep over ~260 testsuite designs reaching codegen shows the deduced properties match getIOProps everywhere except the intentional structural labels on clock/gate/reset ports and six port lines whose properties require the scheduler's boolean simplification (e.g. canScheduleFirst readies reducing to constants). No property is asserted that getIOProps would contradict. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
AAddScheduleDefs applies the schedule to the APackage before this pass runs, recording it as CAN_FIRE/WILL_FIRE definitions (including conflict suppression, e.g. a WILL_FIRE of constant 0 for a rule that can never fire, or constant 1 for a conflict-free always-enabled rule). Teach getIOPropsA to realize the consequences: * Add a small memoized constant evaluator (evalConstA) over the defs, folding 1-bit boolean structure (AND/OR/NOT/IF) and the values of inlined-wire reads: "whas" is the OR over the setters of (WILL_FIRE AND condition), and a single setter's "wget" is its data argument. * A selection (PrimIf) whose condition evaluates to a constant reduces to the selected branch when deducing output properties, so e.g. a bypass-register read whose bypass wire is set only by a never-firing rule is labeled "reg", and the ready of a wire-backed method whose setter always fires is labeled "const". * A rule whose WILL_FIRE evaluates to 0 contributes no hardware uses at all, since all of its logic is dropped. Also memoize the def-following analyses (getOutPropsA over defs, getInPropsA over uses) via lazy maps, so shared definition chains are computed once. On the testsuite sweep (258 designs reaching codegen, ~2900 port lines compared), the deduction now differs from getIOProps only in the intentional structural clock/gate/reset labels and a single line whose "const" requires recognizing a boolean tautology (the ready of a split method, an OR of complementary conditions), which is noted in the module comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
Extend the APackage-based VIOProps deduction with the remaining
inlined primitive and the boolean reductions observable in a sweep of
the full testsuite:
* Look through CReg instances (inlined by InlineCReg, which retains a
plain register): port 0's read is the register output ("reg"),
and port N's read bypasses through a selection between port (N-1)'s
write data and port (N-1)'s read, which reduces when the schedule
makes the intervening write enables constant. Writes feed the
retained register through the bypass chain: a use, but never a
direct port connection. (Note that getIOProps does not label CReg
port-0 reads "reg", because the CReg primitive's import does not
declare that property on its Q_OUT ports; the APackage deduction
labels them from the structure.)
* Count only live call sites: a rule whose WILL_FIRE the schedule
reduces to 0 is dropped entirely, so its calls do not force muxes
onto methods called elsewhere, and its wire sets do not count as
setters. (Evaluated lazily per rule, since one rule's WILL_FIRE
can depend on the setters of a wire read in its predicate.)
* An unconditional method call with its own port copy connects the
caller's WILL_FIRE directly to the method's enable port, so the
enable's declared properties (e.g. "inhigh") reach the module's
own enable inputs through the WILL_FIRE definitions.
* Recognize more reductions that the netlist optimization performs:
selections between equal branches, 1-bit AND/OR over constant
operands (identity operands drop, annihilators make it constant,
a single remaining operand passes through), and complementary
operands (x and !x, recognized through def references), which fold
the ready of a split method to "const".
Add testsuite cases (bsc.verilog/portprops/APkgProps_*) which dump
both analyses side by side over golden files, covering the wire,
CReg, schedule, split-tautology, and foreign-call behaviors.
A sweep of every synthesizable design in the testsuite (2124 designs
reaching code generation, ~18300 port lines) shows no internal
errors, the port enumeration always matches getIOProps, and the
properties match on all but ~130 lines: the intentional structural
labels, and documented cases needing what the APackage cannot see --
chiefly priority-mux arbitration among callers whose WILL_FIREs are
all constant 1, where the surviving arm depends on the earliness
order recorded only in the schedule, plus CSE-level aOpt rewriting
and inout nets through InoutConnect. The module comment records
these limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
Co-authored-by: Ravi Nanavati <ravi@matx.com>
The AScheduleInfo is available where getIOPropsA is called (it is about to be recorded in the .ba file), so pass it in (as a Maybe, for callers without one) and model the argument muxes that AState will create for calls to state instance methods, using the same ingredients AState uses: the port allocation from ratToBlobs, the exclusivity test which chooses between a parallel and a priority mux, and the priority (earliness) order from the schedule. Each call site (arm) is classified by what the netlist optimization leaves of it once constant selectors (WILL_FIRE AND condition) are folded: a single or winning arm is a direct connection to the method's ports (so its arguments take the ports' properties, and its WILL_FIRE connects to the enable), a losing arm is dropped entirely (so its arguments make no uses), and anything else feeds a surviving mux. This resolves the cases where multiple always-firing callers are arbitrated by priority: for example, a register written by two always_enabled methods, where the more urgent method's argument is a direct "reg" connection and the less urgent method's write is dropped -- previously only deducible from the optimized netlist. AState now exports ratToBlobs, genMethodMult, and the blob types for this use. Add a testsuite case (APkgProps_Arb) covering the arbitration, whose two dumps match exactly. On the full-testsuite sweep (2124 designs, ~18300 port lines), the remaining differences are the intentional structural labels plus 110 lines in four known categories (enables of methods whose effects die in value-method muxes, values folded by CSE-level rewriting, inout nets through InoutConnect, and reads simplified by deeper aOpt logic rewriting), with no properties asserted that getIOProps would contradict. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
A wire or CReg is connected to the same things whether or not InlineWires/InlineCReg will inline it away: the primitives' Verilog is just wiring (around a register, for CReg), so the properties of the module's ports do not depend on where the module boundary falls. Remove the removeRWire/removeCross/removeCReg gating from the look-through, so the deduced properties are now insensitive to the -no-inline-rwire and -no-inline-creg flags -- unlike getIOProps, whose deductions weaken when the instances remain in the netlist (it does not follow connectivity into an instance). With the flags at their defaults nothing changes (all testsuite goldens are identical); with inlining disabled, getIOPropsA now keeps reporting e.g. "reg" for a value read through a bypass wire, where it previously matched getIOProps in dropping the properties. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
Restate the header comment as the specification: a property is asserted only when it is entailed by the structure, dataflow, and schedule of the design -- never by which optimizations the backend performs or where module boundaries fall among wiring primitives -- so the answers are stable across optimization and inlining settings and across backends. Define each property (structural roles, "reg", "const", "unused") in those terms, state the conservative soundness contract and its known limits (boolean minimization of guards over dynamic values, register value analysis, InoutConnect nets), and describe the relation to getIOProps -- which reports whatever the optimized netlist happens to show, and so can differ in both directions -- as a consequence rather than as the definition. No code changes; all testsuite golden files are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
Propose defining the port properties denotationally -- by what each property means for the hardware described by the design and its schedule -- and computing them with the conservative APackage-based analysis, instead of measuring the optimized netlist. The document records the definitions, the soundness and stability contracts, the testsuite evidence, how narrow the optimizer's remaining edge is, a migration plan for retiring getIOProps, the known limitations, and the future work this enables (flop-in/flop-out boundary checking for physical-design blocks, boundary registration reporting, and port properties for Bluesim-compiled modules). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
Measuring both analyses over the existing portprops testsuite directory: of the 29 compiling designs, 17 match getIOProps byte-for-byte, 11 differ only by the retained structural labels, and 1 (InoutProps_ArgToIfc) differs because getIOProps labels one pin of an aliased inout pair "unused" as an artifact of the alias collapse (the argument and interface inout are one net exposed at two pins; the semantic analysis reports both as live). Covering the directory under the proposal is therefore a golden-file regeneration, not an analysis change. Add the alias-collapse artifact to the documented divergence classes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
Audit which of getIOPropsA's mechanisms the portprops directory
exercises and add tests for the four that were missing:
* APkgProps_SubClock: output clock, gate, and reset ports whose wires
come from a submodule's special outputs (a gated clock and a reset
generator); both analyses agree exactly.
* APkgProps_DeadLogic: "unused" propagating backwards through dead
arithmetic into a $display -- the golden documents the intentional
difference (getIOPropsA labels the argument unused; getIOProps only
traces direct connections; both label the enable unused).
* APkgProps_Mux: surviving muxes -- both setters of a register and a
wire are live, so the arguments are used but carry no properties,
in both analyses (the contrast to APkgProps_Arb).
* APkgProps_Args: parameters are not ports; module argument ports
deduce like other inputs ("reg" and "unused").
With these, the directory exercises every deduction mechanism, and
its accounting (recorded in the proposal) is: 33 compiling designs,
18 byte-identical between the analyses, 13 differing only by the
retained structural labels, 1 inout alias-collapse artifact, and 1
dead-logic unused difference. All 67 checks pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
Co-authored-by: Ravi Nanavati <ravi@matx.com>
The complete dejagnu testsuite was run locally on this branch: 855 test groups, 17,897 passes, 128 expected failures, and no failures attributable to the change (the only failing tests are ones which cannot run in the build container: SystemC linking, and tests which require unreadable files while running as root). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
Complete the wire-transparency story: the deduction now propagates actual expressions through wire and CReg boundaries, not just properties and constant values, so reductions which only become applicable after wire inlining are still recognized: * derefA follows a wire read to the wire's data expression, follows selections with constant conditions to the taken branch, and drops the identity operands of 1-bit AND/OR to a lone survivor -- so a tautology carried through a wire (e.g. a guard of b || fromMaybe(False, w.wget) where the wire carries !b from an always-firing rule) is recognized and the ready is "const". * A merge point is crossed exactly when all arms carry the same expression: a wire (or CReg port) set to the same expression by several rules is an alias of that expression, matching AState's own collapsing of equal-expression mux arms (the method-argument arbitration already inherited this from ratToBlobs' unique-use blobs). The validity of such a wire also folds when the setters' WILL_FIREs are complementary (rules split over a condition). * getOutPropsA consults the evaluator on whole primitives, so any expression whose value the evaluator determines is "const" -- previously the boolean-structure reductions were only visible when something else evaluated the expression. The analysis deliberately does not reason by enumerating the values which reach a merge (a wire set to 1, 2, or 3 is not deduced to be nonzero): such properties come from the particular values written, not from the structure of the design, and are exactly what the stability contract excludes. The proposal records this boundary, and the tractability evidence (memoized per definition; unmeasurable against compile noise on the largest testsuite design). Add APkgProps_WireExpr covering the tautology-through-wire and the same-expression-multiple-setters cases; both analyses agree on all deduced properties. All 69 portprops checks pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
With expressions propagating through wires, the analysis's remaining gaps split cleanly along the semantic boundary the proposal draws: what it misses is exactly what "agreement, not enumeration" excludes on principle (boolean minimization over independent dynamic values, and value enumeration at merges), plus two items which are deducible under the contract but not yet implemented (InoutConnect nets and value-method mux arbitration). Restructure the Limitations section accordingly, and note in the optimizer's-edge discussion that hiding a foldable shape behind a wire no longer works. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky Co-authored-by: Ravi Nanavati <ravi@matx.com>
…ding Extend the arm-classification model (the same port allocation, exclusivity test, and order that AState uses) from action methods to the argument muxes of value methods: arms are keyed by the call expression (value calls are shared through defs), selectors use the RDY for interface value-method users and the WILL_FIRE otherwise, and the selector references of surviving muxes are recorded as uses exactly from the arms -- which also covers calls reached through defs that the previous syntactic scan of a rule's expressions missed. Model the fate of the references AState itself creates for a caller's WILL_FIRE, so that an enable whose effects fold away is deduced unused: * a port's enable (the OR of every arm's WILL_FIRE AND condition) folds when a conjunct is constant true or the unconditional WILL_FIREs contain a complementary pair (as wireHasVal); * the data node of a wire is an alias of its setter's argument (referencing no WILL_FIRE) when the setters agree on one expression, and otherwise follows the setter's arm class; * both mux kinds fold as the selection chain the netlist realizes them as: constant selectors fold from the front (muxWalk, unifying the former priWalk/parMux), a lone surviving arm is a direct connection, and the last surviving arm's selector is absorbed into the mux's don't-care default. A constant-true arm of a parallel mux deliberately does NOT fold the other arms away: the arms' exclusivity is a schedule assumption about the callers, not a fact of the netlist (two always-enabled methods can even contradict it), so nothing entitles that folding structurally. Validated by re-sweeping all 2124 code-generating testsuite designs: the enable category (49 lines in the proposal's earlier accounting) is fully closed; the remaining getIOProps-only lines are exactly the two documented categories (boolean minimization 33, InoutConnect 19); no line asserts a property getIOProps contradicts. Two new golden tests (APkgProps_VMux, APkgProps_EnFold) cover the new mechanisms and are stable under -no-inline-rwire and -keep-fires. Co-authored-by: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
Compute getIOPropsA before the backend split and use its results for the import-BVI wrapper attributes (veriPortProps) and for the Verilog "Ports:" comment. Bluesim compiles now record port properties in their .bo files -- previously an XXX; a parent compiled with either backend consumes a child's properties regardless of which backend compiled the child. getIOProps remains for comparison, computed (lazily) only when its dump is requested with -dIOproperties. Co-authored-by: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
The "Ports:" comment in generated Verilog now comes from the APackage-based analysis, which keeps the structural labels (clock, reset, gate, inout) on ports whose connectivity the netlist measurement loses. The Verilog bodies are unchanged; only the header comments differ. Co-authored-by: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
…mulator These compare_verilog checks only execute when a Verilog simulator is configured (vtest), so the goldens had rotted: beyond the new Ports comments, they disagreed with the compiler's current generated-name counters (__hN heap positions, ___dN def numbers) -- a drift which predates this branch (verified by rebuilding the pre-change compiler and library: both produce today's numbering). The Verilog is otherwise identical; regenerate. Co-authored-by: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
Co-authored-by: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EajBnZQ1gitQ1wXz5h6jky
Port splitting (PR 849) gave methods multiple input and output ports: VFieldInfo now carries vf_outputs :: [VPort] and per-argument port groups vf_inputs :: [[VPort]], and a split method's value is an ATuple selected by ATupleSel. Follow that shape in getIOPropsA, which was written before the split landed: * getMethOut emits one entry per output port, pairing the value tuple's types and elements with the declared ports (mirroring AState's outputADefToADefs); declaredOutProps is per-port. * argPortProps flattens the per-argument port groups to pair with aIfaceArgs' flattened port list. * A selected result (ATupleSel around a method call) takes that port's declared properties (methOutPortPropsA); an unselected reference to a multi-port method keeps the properties common to all of its ports. * The expression traversals (exprCalls, classifyExpr, classifyForeignExpr) walk ATuple/ATupleSel instead of treating them as no-use, and a split argument's ATuple elements pair with the argument's ports in directMethArgs. * aIfaceName was removed upstream; use aif_name (as AState does). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016sDiGGpNpEup2huNG7LYW7
The mkDesign.v.expected comparison is dormant without a simulator, so upstream did not regenerate it when port splitting (PR 849) changed method-output name resolution (the __h numbering). With the port properties now fed from the APackage analysis, the Ports comment also gains the structural "clock"/"reset" labels on the unused clk and reset ports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016sDiGGpNpEup2huNG7LYW7
The inout fix was extracted from this branch and landed separately (B-Lang-org/bsc PR 1057, now in the base); the commit dropped from this branch in that extraction also carried the proposal's rewritten sweep accounting (the 19 InoutConnect lines were the getIOProps mislabel propagating up parent compiles, eliminated by the fix; 52 -> 33 getIOProps-only lines, all in the boolean-minimization category). Restore those hunks, wording the fix as part of the base rather than this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016sDiGGpNpEup2huNG7LYW7
Collaborator
Author
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
Port properties (
clock,reset,reg,const,unused,inhigh,...) are now derived by
getIOPropsA, a semantic analysis of theAPackageand the schedule, instead of being measured off the optimizednetlist at the end of the Verilog backend. A property is asserted only
when it is entailed by the design's structure, dataflow, and schedule —
so the answers are stable across optimization settings, inlining
settings, and backends. The results feed the import-BVI wrapper
attributes in the
.bofor both backends (Bluesim compiles gainport properties, closing the long-standing
XXXinbsc.hs) and theVerilog "Ports:" comment. The old
getIOPropsremains available forcomparison behind
-dIOproperties, computed only when that dump isrequested.
A design document (
doc/proposals/port-properties.md) records theproperty definitions, the soundness/stability contracts, the validation
evidence, and the migration plan (this PR is steps 1–3; steps 4–5 retire
getIOPropsafter a release of coexistence).Stacked on #1057, whose two commits this branch contains; the
commits under review here begin at "Add getIOPropsA". This PR can be
rebased to drop them once #1057 merges.
Why
The measured properties are functions of a particular compile, not of
the design: an unused input clock loses its
clocklabel because nosurviving netlist connection witnesses it;
-no-inline-rwiremakes avalue read through a bypass wire lose its
reglabel although theconnectivity is unchanged; a ready can be
constonly because of a CSErewrite. The properties are recorded in the
.boand consumed byparent compiles (
inhighlegality, readiness reasoning), so theinstability propagates up the hierarchy.
How it works
AStateconstructs them.the wire/field info and are always present — an unused input clock is
clock unused.reg/const/unusedare deduced by dataflow: wire and CReginstances are looked through regardless of the inlining flags; a
memoized evaluator folds schedule-time constants (never/always-firing
WILL_FIREs, wire validity, complementary conditions); and the argument
muxes of state-instance methods are modeled with AState's own port
allocation, exclusivity test, and order — for action methods and value
methods (RDY-based selectors for interface value-method users). The
references AState itself creates for a caller's WILL_FIRE are folded
semantically: enables absorbed by constant or complementary conjuncts,
selectors of direct connections, losing arms, and last mux arms
(absorbed into don't-care defaults).
collapse; value sets are never enumerated. This boundary is what makes
the answers stable (see the design document).
Rebase notes (onto current
main+ #1057)landed upstream as PR 902 and were dropped from the series.
series ends with an adaptation commit ("Adapt getIOPropsA to split
method ports") migrating the pass to per-port method outputs
(
vf_outputs), per-argument input port groups (vf_inputs), and theATuple/ATupleSelexpression forms, mirroringAState's ownhandling. It includes a regression test (
APkgProps_SplitSel) that aselected result port takes the selected port's declared property
(
ATupleSelindices are 1-based). One known conservative case: asplit argument reached through a CSE'd tuple-typed definition falls
back to an opaque use (live, no properties asserted) rather than
pairing per-port declared properties through the definition.
bsc.bugs/bluespec_inc/b302/mkDesign.v.expectedis regenerated: itscomparison is dormant without a Verilog simulator, so it was not
regenerated upstream when port splitting changed the
__hnamenumbering (verified against a pre-split compiler).
Validation
lines) comparing both analyses: identical port enumeration everywhere;
1,686 lines differ, of which 1,638 are the richer structural labels,
15 are strictly more accurate (dead-logic
unused, CReg port-0reg,a register repacked through an identity case), and 33 are the
documented boolean-minimization non-goal. No line asserts a property
getIOPropscontradicts.-no-inline-rwire,-no-inline-creg,-keep-fires, where the netlist measurement weakens.testsuite/bsc.verilog/portprops: golden tests dumping both analysesside by side cover each deduction mechanism, including new tests for
value-method argument muxes (
APkgProps_VMux) and enable/selectorfolding (
APkgProps_EnFold).configured (which also enables the otherwise-dormant
compare_veriloggolden checks): 20313 PASS / 0 FAIL / 134 XFAIL / 0 XPASS,
including
bsc.verilog/portprops85/85.-simand a parent with-verilog— the parent deduces its inputunusedfrom the child's.bo.largest testsuite design (h264 deblocking filter).
Questions for reviewers
doc/proposals/an acceptable home for the design document?-dAPackageIOproperties/getIOPropsA.getIOProps).🤖 Generated with Claude Code
https://claude.ai/code/session_016sDiGGpNpEup2huNG7LYW7