Skip to content

[ImportVerilog] Add capture analysis pre-pass for functions#10094

Merged
fabianschuiki merged 1 commit intomainfrom
fschuiki/capture-analysis
Apr 1, 2026
Merged

[ImportVerilog] Add capture analysis pre-pass for functions#10094
fabianschuiki merged 1 commit intomainfrom
fschuiki/capture-analysis

Conversation

@fabianschuiki
Copy link
Copy Markdown
Contributor

Add a pre-pass over the slang AST that determines which non-local, non-global variables each function captures, either directly or transitively through calls. This walks the entire AST, collecting variable references and call graph edges, then propagates captures through the inverse call graph using a worklist until stable.

This analysis is a prerequisite for switching ImportVerilog to a two-phase declare-then-define approach for functions. Currently, function bodies are converted eagerly during package/module member iteration, which can fail when a function transitively references a variable that hasn't been declared yet (e.g., UVM's m_uvm_core_state). With the capture sets known upfront, function declarations can include the correct capture parameters from the start, and body conversion can be deferred.

Also add MapVector, SmallMapVector, and SmallSetVector re-exports.

The analysis is not yet directly used in ImportVerilog. This will be done in a follow-up commit.

Comment thread unittests/Conversion/CMakeLists.txt Outdated
Comment thread unittests/Conversion/ImportVerilog/CaptureAnalysisTest.cpp
Comment thread unittests/Conversion/ImportVerilog/CaptureAnalysisTest.cpp Outdated
Copy link
Copy Markdown
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking at all but it’s a bit unfortunate that we have to run this level of analysis at the Slang AST. While the current complexity is manageable, if it continues to grow, I wonder if it's a good idea to introduce an intermediate operation to capture variables (e.g., a function-like operation without IsolatedFromAbove, maybe moore.closure) so we can progressively lower it.

@fabianschuiki
Copy link
Copy Markdown
Contributor Author

@uenoku I went back and forth on this a bit. Intuitively, I thought lowering everything and then finding captured variables as SSA value uses that cross the function boundary sounded pretty good. But the problem with that is that we need to keep a map from AST nodes to SSA values around, instead of just keeping scoped hash maps and popping scopes as we leave them. That whole thing started to feel very ugly and messy compared to analyzing the captures at the frontend language level using the AST, and then just emitting the right IR for that directly 🤔

@fabianschuiki fabianschuiki force-pushed the fschuiki/capture-analysis branch from 4882b1a to 34ecdfc Compare April 1, 2026 17:46
@fabianschuiki fabianschuiki requested a review from uenoku April 1, 2026 17:46
@fabianschuiki
Copy link
Copy Markdown
Contributor Author

Thanks for the comments @uenoku -- I've addressed them 👍

@fabianschuiki
Copy link
Copy Markdown
Contributor Author

Results of circt-tests run for 34ecdfc compared to results for 56e2170: no change to test results.

@fabianschuiki fabianschuiki force-pushed the fschuiki/capture-analysis branch from 34ecdfc to 17871eb Compare April 1, 2026 19:01
@fabianschuiki
Copy link
Copy Markdown
Contributor Author

Results of circt-tests run for 17871eb compared to results for 56e2170: no change to test results.

@fabianschuiki fabianschuiki force-pushed the fschuiki/capture-analysis branch from 17871eb to 9bebba2 Compare April 1, 2026 20:44
Add a pre-pass over the slang AST that determines which non-local,
non-global variables each function captures, either directly or
transitively through calls. This walks the entire AST, collecting
variable references and call graph edges, then propagates captures
through the inverse call graph using a worklist until stable.

This analysis is a prerequisite for switching ImportVerilog to a
two-phase declare-then-define approach for functions. Currently,
function bodies are converted eagerly during package/module member
iteration, which can fail when a function transitively references a
variable that hasn't been declared yet (e.g., UVM's `m_uvm_core_state`).
With the capture sets known upfront, function declarations can include
the correct capture parameters from the start, and body conversion can
be deferred.

Also add `MapVector`, `SmallMapVector`, and `SmallSetVector` re-exports.

The analysis is not yet directly used in ImportVerilog. This will be
done in a follow-up commit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fabianschuiki fabianschuiki force-pushed the fschuiki/capture-analysis branch from 9bebba2 to 88297f5 Compare April 1, 2026 20:48

/// Test fixture that skips all tests when running under Valgrind, since slang
/// triggers Valgrind's uninitialized value detection.
class CaptureAnalysisTest : public testing::Test {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

@fabianschuiki
Copy link
Copy Markdown
Contributor Author

Results of circt-tests run for 88297f5 compared to results for 56e2170: no change to test results.

@fabianschuiki fabianschuiki merged commit 658cae8 into main Apr 1, 2026
11 of 12 checks passed
@fabianschuiki fabianschuiki deleted the fschuiki/capture-analysis branch April 1, 2026 21:43
joaovam pushed a commit to joaovam/circt that referenced this pull request Apr 10, 2026
Add a pre-pass over the slang AST that determines which non-local,
non-global variables each function captures, either directly or
transitively through calls. This walks the entire AST, collecting
variable references and call graph edges, then propagates captures
through the inverse call graph using a worklist until stable.

This analysis is a prerequisite for switching ImportVerilog to a
two-phase declare-then-define approach for functions. Currently,
function bodies are converted eagerly during package/module member
iteration, which can fail when a function transitively references a
variable that hasn't been declared yet (e.g., UVM's `m_uvm_core_state`).
With the capture sets known upfront, function declarations can include
the correct capture parameters from the start, and body conversion can
be deferred.

Also add `MapVector`, `SmallMapVector`, and `SmallSetVector` re-exports.

The analysis is not yet directly used in ImportVerilog. This will be
done in a follow-up commit.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants