Fix two quadratic blowups in foreign-block Verilog generation#8
Open
nanavati wants to merge 3 commits into
Open
Fix two quadratic blowups in foreign-block Verilog generation#8nanavati wants to merge 3 commits into
nanavati wants to merge 3 commits into
Conversation
vForeignBlock chooses the defs to inline into a system-task always block by intersecting the defs reachable from an ActionValue call's result with the defs an fcall depends on, and getAVDependDefs removes the avalues from a reachable set. Both used Data.List (intersect and \\) on lists that, for a rule with many foreign-call-dependent defs, are both O(n) long -- so the selection was O(n^2) in the number of such defs. Use Data.Set for both. The inputs come from closeOverMap (Set-backed, so duplicate-free and sorted), so S.toList . S.intersection and a S.notMember filter produce exactly the same lists, in the same order, that the Data.List versions did -- the generated Verilog is byte-identical. Measured on a module with ~98k such defs, this alone cut the intersect cost from ~3 minutes to seconds. 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
The vDef closure decides, per def, whether a def is assigned inside a foreign (system-task) always block -- in which case it is declared "reg" with no continuous assignment. The membership set (defs_in_foreign_blocks) was a list consulted with elem once per def, so a module whose defs are mostly foreign-block-assigned (an ActionValue result flowing through a large body of defs) spent O(n^2) here. Convert it to a Set once and test with S.member. Output is unchanged. This is the high-end companion to the vForeignBlock selection fix: on a ~98k-def module the two together bring Verilog generation down from unfinished-after-an-hour to about a minute. 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
Add motivating tests for the foreign-block inline codegen fixes. bsc.verilog/foreign_inline is a correctness test: a rule takes an ActionValue system task result ($stime) through a Vector fold into a later task, so the fold's defs are inlined into the negedge block and declared reg. It checks the reg-declaration count (all inline defs present, none dropped or duplicated) and simulates the result. This exercises the vForeignBlock selection and the vDef reg-marking with a small module, independent of the performance fixes. bsc.long_tests/verilog_foreign_inline_large is the scale tripwire: the same shape with an 8192-element vector elaborates to ~24k inlined defs from a compact source. With the Set-based membership tests Verilog generation is near-linear; a regression to the old list-based tests makes it quadratic and turns tens of seconds into many minutes. 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
nanavati
force-pushed
the
claude/verilog-codegen-perf-rw8p7e
branch
from
July 17, 2026 05:46
041dc67 to
556683d
Compare
nanavati
changed the title
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
2 participants
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.
When a rule takes the result of an ActionValue system task through a body of defs into a later system task, those defs are inlined into the negedge foreign always-block. Generating that block was quadratic in the number of such defs, from two separate list operations. On a module with ~100k foreign-block defs (which a compact source can reach via a
Vectorfold), Verilog generation went from unfinished-after-an-hour to about a minute.Both fixes keep the generated Verilog byte-identical; only the data structures behind membership/selection change.
AVerilogUtil: Set-based def selection invForeignBlockThe set of defs to inline is
av_depend_defs ∩ fcall_depend_defs, andgetAVDependDefsremoves the avalues from a reachable set. Both usedData.List(intersect,\\) on lists that are each O(n) for such a module, so selection was O(n²). Both inputs come fromcloseOverMap(Set-backed: duplicate-free and sorted), soS.intersectionand anS.notMemberfilter produce the same lists in the same order — identical output.AVerilog: Set-based reg-marking of foreign-block defsThe
vDefclosure tests, per def, whether the def is assigned in a foreign block (and so declaredregwith no continuous assignment). The membership set was a list consulted withelemonce per def — O(n²) for a module whose defs are mostly foreign-block-assigned. It is now aSettested withS.member.Tests
bsc.verilog/foreign_inline— correctness: a$stime-fedVectorfold whose defs are inlined into the system-task block; checks the reg-declaration count (no defs dropped or duplicated) and simulates.bsc.long_tests/verilog_foreign_inline_large— scale tripwire: the same shape at 8192 elements (~24k inlined defs) from a compact source, near-linear with these fixes and quadratic without.Both quadratics are longstanding and independent of any in-flight work. (A related but separate SYB-traversal cost in
IExpand, which shows up under-keep-method-conds, is addressed in its own PR.)🤖 Generated with Claude Code
https://claude.ai/code/session_01VVN2QMZtLUwkag1vCYLaao