IExpand: replace the SYB traversal in removeInlinedPositions#9
Open
nanavati wants to merge 1 commit into
Open
Conversation
Under -keep-method-conds, removeInlinedPositions strips the inlined- position bookkeeping from every ICon head once generateMethodPreds has harvested it. It did so with a Data.Generics "everywhere" traversal, which pays a generic-dispatch cost at every node of the module; on a large module the pass alone took about a third of elaboration time. Replace it with a direct traversal over just the fields that carry expressions (defs, rules, interface, state-instance args, clock/reset/ inout wires, and the expression-bearing IConInfo constructors). The traversal is deliberately as lazy as SYB's was: the output must stay byte-identical, and downstream declaration order depends on the order in which the generated names are first forced (FString interning feeds Ord Id, which feeds tsort tie-breaks and the Set-sorted lists in the Verilog backend). An earlier version that indexed the defs eagerly (a Map from def id to stripped body, to share ICValue rebuilds) reordered the generated Verilog; recursing into each ICValue payload directly avoids that and reproduces the SYB output exactly. Correctness is covered by the existing bsc.misc/method_conditions tests (COND signals in the .v and bluetcl position readout from the .ba); this is a performance change with no output difference. Measured on a ~98k-def module compiled with -keep-method-conds, the pass dropped from ~27s (and ~1 GB of rebuilt heap) to under a second. Co-Authored-By: Ravi Nanavati <ravi@matx.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VVN2QMZtLUwkag1vCYLaao
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.
Under
-keep-method-conds,removeInlinedPositionsstrips the inlined-position bookkeeping from everyIConhead oncegenerateMethodPredshas harvested it into the COND defs. It did this with aData.Generics"everywhere" traversal, which pays a generic-dispatch cost at every node of the module; on a large module the pass alone took about a third of elaboration time (and ~1 GB of rebuilt heap).Replace it with a direct traversal over just the fields that carry expressions (local defs, rules, interface, state-instance args, clock/reset/inout wires, and the expression-bearing
IConInfoconstructors). The traversal is deliberately kept as lazy as the SYB one was: the output must stay byte-identical, and downstream declaration order depends on the order in which the generated names are first forced (FString interning feedsOrd Id, which feeds tsort tie-breaks and the Set-sorted lists in the Verilog backend). An earlier version that indexed the defs eagerly — aMapfrom def id to stripped body, to shareICValuerebuilds — reordered the generated Verilog; recursing into eachICValuepayload directly avoids that and reproduces the SYB output exactly.This is a performance change with no output difference. Correctness is covered by the existing
bsc.misc/method_conditionstests, which check both the COND signals in the generated.vand the bluetcl position readout from the.ba(134 pass). Measured on a module elaborating to ~98k defs under-keep-method-conds, the pass dropped from ~27s to under a second.🤖 Generated with Claude Code
https://claude.ai/code/session_01VVN2QMZtLUwkag1vCYLaao
Generated by Claude Code