Skip to content

Commit 17871eb

Browse files
fabianschuikiclaude
andcommitted
[ImportVerilog] Add capture analysis pre-pass for functions
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>
1 parent 56e2170 commit 17871eb

File tree

8 files changed

+517
-2
lines changed

8 files changed

+517
-2
lines changed

include/circt/Support/LLVM.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,26 @@ using mlir::TypeSwitch; // NOLINT(misc-unused-using-decls)
7272
// Forward declarations of LLVM classes to be imported in to the circt
7373
// namespace.
7474
namespace llvm {
75+
template <typename KeyT, typename ValueT, typename MapType, typename VectorType>
76+
class MapVector;
7577
template <typename KeyT, typename ValueT, unsigned InlineBuckets,
7678
typename KeyInfoT, typename BucketT>
7779
class SmallDenseMap;
80+
template <typename KeyT, typename ValueT, unsigned N>
81+
struct SmallMapVector;
7882
template <typename T, unsigned N, typename C>
7983
class SmallSet;
84+
template <typename T, unsigned N>
85+
class SmallSetVector;
8086
} // namespace llvm
8187

8288
// Import things we want into our namespace.
8389
namespace circt {
84-
using llvm::SmallDenseMap; // NOLINT(misc-unused-using-decls)
85-
using llvm::SmallSet; // NOLINT(misc-unused-using-decls)
90+
using llvm::MapVector; // NOLINT(misc-unused-using-decls)
91+
using llvm::SmallDenseMap; // NOLINT(misc-unused-using-decls)
92+
using llvm::SmallMapVector; // NOLINT(misc-unused-using-decls)
93+
using llvm::SmallSet; // NOLINT(misc-unused-using-decls)
94+
using llvm::SmallSetVector; // NOLINT(misc-unused-using-decls)
8695
} // namespace circt
8796

8897
// Forward declarations of classes to be imported in to the circt namespace.

lib/Conversion/ImportVerilog/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ include(SlangCompilerOptions)
22

33
add_circt_translation_library(CIRCTImportVerilog
44
AssertionExpr.cpp
5+
CaptureAnalysis.cpp
56
Expressions.cpp
67
FormatStrings.cpp
78
HierarchicalNames.cpp
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "CaptureAnalysis.h"
10+
#include "slang/ast/ASTVisitor.h"
11+
#include "llvm/ADT/MapVector.h"
12+
#include "llvm/Support/SaveAndRestore.h"
13+
14+
using namespace slang::ast;
15+
using namespace circt;
16+
using namespace circt::ImportVerilog;
17+
18+
/// Check whether `var` is local to `func`. Walk up from the variable's parent
19+
/// scope; if we reach `func` before hitting another function boundary, the
20+
/// variable is local.
21+
static bool isLocalToFunction(const ValueSymbol &var,
22+
const SubroutineSymbol &func) {
23+
for (const Scope *scope = var.getParentScope(); scope;
24+
scope = scope->asSymbol().getParentScope()) {
25+
if (&scope->asSymbol() == &func)
26+
return true;
27+
if (scope->asSymbol().kind == SymbolKind::Subroutine)
28+
return false;
29+
}
30+
return false;
31+
}
32+
33+
/// Workaround for a slang deficiency: when accessing a member of a virtual
34+
/// interface (e.g., `vif.data`), slang resolves the entire dotted path during
35+
/// name lookup and produces a `NamedValueExpression` that directly references
36+
/// the signal symbol inside the interface's `InstanceBody`. Unlike struct
37+
/// fields and class properties, which produce a `MemberAccessExpression`, there
38+
/// is no syntactic indication on the expression that this was a member
39+
/// projection.
40+
///
41+
/// This check matches variables that live inside an interface instance body.
42+
/// A `NamedValueExpression` referencing such a symbol is the result of slang's
43+
/// virtual interface member resolution, not a genuine variable capture. This is
44+
/// expected to be fixed upstream in slang.
45+
///
46+
/// See https://github.com/MikePopoloski/slang/discussions/1770
47+
static bool isVirtualInterfaceMemberAccess(const ValueSymbol &var) {
48+
// Walk up from the variable to find the nearest enclosing InstanceBody.
49+
for (const Scope *scope = var.getParentScope(); scope;
50+
scope = scope->asSymbol().getParentScope()) {
51+
auto *body = scope->asSymbol().as_if<InstanceBodySymbol>();
52+
if (!body)
53+
continue;
54+
return body->getDefinition().definitionKind == DefinitionKind::Interface;
55+
}
56+
return false;
57+
}
58+
59+
/// Check whether `var` is a global variable. Walk up from the variable's parent
60+
/// scope; if we hit a function or instance body, it's not global. Otherwise
61+
/// (package, compilation unit, root) it is.
62+
static bool isGlobalVariable(const ValueSymbol &var) {
63+
for (const Scope *scope = var.getParentScope(); scope;
64+
scope = scope->asSymbol().getParentScope()) {
65+
switch (scope->asSymbol().kind) {
66+
case SymbolKind::Subroutine:
67+
case SymbolKind::InstanceBody:
68+
return false;
69+
default:
70+
break;
71+
}
72+
}
73+
return true;
74+
}
75+
76+
namespace {
77+
78+
/// Walk the entire AST to collect captured variables and the call graph for
79+
/// each function. Uses slang's `ASTVisitor` with both statement and expression
80+
/// visiting enabled so that we recurse into all function bodies.
81+
struct CaptureWalker
82+
: public ASTVisitor<CaptureWalker, /*VisitStatements=*/true,
83+
/*VisitExpressions=*/true> {
84+
85+
/// The function whose body we are currently inside, or nullptr if we are at
86+
/// a scope outside any function.
87+
const SubroutineSymbol *currentFunc = nullptr;
88+
89+
/// Captured variables per function.
90+
CaptureMap capturedVars;
91+
92+
/// Inverse call graph: maps each callee to the set of callers that call it.
93+
/// Used to propagate captures from callees to their callers. Uses MapVector
94+
/// for deterministic iteration order during propagation.
95+
MapVector<const SubroutineSymbol *,
96+
SmallSetVector<const SubroutineSymbol *, 4>>
97+
callers;
98+
99+
/// When we enter a function body, record it as the current function and
100+
/// recurse into its members and body statements.
101+
void handle(const SubroutineSymbol &func) {
102+
llvm::SaveAndRestore guard(currentFunc, &func);
103+
visitDefault(func);
104+
}
105+
106+
/// When we see a named value reference inside a function, check if it needs
107+
/// to be captured.
108+
void handle(const NamedValueExpression &expr) {
109+
if (!currentFunc)
110+
return;
111+
112+
auto &var = expr.symbol;
113+
114+
// Class properties are accessed through `this`, not captured.
115+
if (var.kind == SymbolKind::ClassProperty)
116+
return;
117+
118+
// Function arguments are local by definition.
119+
if (var.kind == SymbolKind::FormalArgument)
120+
return;
121+
122+
// Only capture variables that are non-local and non-global.
123+
if (isLocalToFunction(var, *currentFunc) || isGlobalVariable(var))
124+
return;
125+
126+
// Work around a slang deficiency where virtual interface member accesses
127+
// are resolved to NamedValueExpressions referencing symbols inside the
128+
// interface's instance body, indistinguishable from direct variable
129+
// references. See isVirtualInterfaceMemberAccess for details.
130+
if (isVirtualInterfaceMemberAccess(var))
131+
return;
132+
133+
capturedVars[currentFunc].insert(&var);
134+
}
135+
136+
/// Record call graph edges when we see a function call.
137+
void handle(const CallExpression &expr) {
138+
if (currentFunc)
139+
if (auto *const *callee =
140+
std::get_if<const SubroutineSymbol *>(&expr.subroutine))
141+
callers[*callee].insert(currentFunc);
142+
visitDefault(expr);
143+
}
144+
145+
/// Propagate captures transitively through the call graph. For each callee
146+
/// that has captures, push each captured variable upward through all
147+
/// transitive callers using a worklist. A captured variable is only
148+
/// propagated to a caller if it is not local to that caller.
149+
void propagateCaptures() {
150+
using WorkItem = std::pair<const SubroutineSymbol *, const ValueSymbol *>;
151+
SmallSetVector<WorkItem, 16> worklist;
152+
153+
for (auto &[func, _] : callers) {
154+
// Check if this function captures any variables. Nothing to do if it
155+
// doesn't.
156+
auto it = capturedVars.find(func);
157+
if (it == capturedVars.end())
158+
continue;
159+
160+
// Prime the worklist with the captured variables.
161+
for (auto *var : it->second)
162+
worklist.insert({func, var});
163+
164+
// Push each captured variables to the func's callers transitively.
165+
while (!worklist.empty()) {
166+
auto [func, cap] = worklist.pop_back_val();
167+
auto callersIt = callers.find(func);
168+
if (callersIt == callers.end())
169+
continue;
170+
for (auto *caller : callersIt->second)
171+
if (!isLocalToFunction(*cap, *caller))
172+
if (capturedVars[caller].insert(cap))
173+
worklist.insert({caller, cap});
174+
}
175+
}
176+
}
177+
};
178+
179+
} // namespace
180+
181+
CaptureMap ImportVerilog::analyzeFunctionCaptures(const RootSymbol &root) {
182+
CaptureWalker walker;
183+
root.visit(walker);
184+
walker.propagateCaptures();
185+
return std::move(walker.capturedVars);
186+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// Pre-pass over the slang AST to determine which non-local, non-global
10+
// variables each function captures, either directly or transitively through
11+
// calls to other functions.
12+
//
13+
// This information is needed before any MLIR conversion happens so that
14+
// function declarations can be created with the correct signature (including
15+
// extra capture parameters) upfront, enabling a clean two-phase
16+
// declare-then-define approach for functions.
17+
//
18+
//===----------------------------------------------------------------------===//
19+
20+
// NOLINTNEXTLINE(llvm-header-guard)
21+
#ifndef CONVERSION_IMPORTVERILOG_CAPTUREANALYSIS_H
22+
#define CONVERSION_IMPORTVERILOG_CAPTUREANALYSIS_H
23+
24+
#include "circt/Support/LLVM.h"
25+
#include "llvm/ADT/SetVector.h"
26+
27+
namespace slang {
28+
namespace ast {
29+
class RootSymbol;
30+
class SubroutineSymbol;
31+
class ValueSymbol;
32+
} // namespace ast
33+
} // namespace slang
34+
35+
namespace circt {
36+
namespace ImportVerilog {
37+
38+
/// The result of capture analysis: for each function, the set of non-local,
39+
/// non-global variable symbols that the function captures directly or
40+
/// transitively through calls.
41+
using CaptureMap = DenseMap<const slang::ast::SubroutineSymbol *,
42+
SmallSetVector<const slang::ast::ValueSymbol *, 4>>;
43+
44+
/// Analyze the AST rooted at `root` to determine which variables each function
45+
/// captures. A variable is considered captured by a function if it is
46+
/// referenced inside the function's body (or transitively through called
47+
/// functions) and is neither local to the function nor a global variable
48+
/// (package-scope or compilation-unit-scope variables that are lowered via
49+
/// `get_global_signal`).
50+
CaptureMap analyzeFunctionCaptures(const slang::ast::RootSymbol &root);
51+
52+
} // namespace ImportVerilog
53+
} // namespace circt
54+
55+
#endif // CONVERSION_IMPORTVERILOG_CAPTUREANALYSIS_H

unittests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ function(add_circt_unittest test_dirname)
99
add_unittest(CIRCTUnitTests ${test_dirname} ${ARGN})
1010
endfunction()
1111

12+
add_subdirectory(Conversion)
1213
add_subdirectory(Dialect)
1314
add_subdirectory(Support)
1415
add_subdirectory(Tools)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
if(CIRCT_SLANG_FRONTEND_ENABLED)
2+
add_subdirectory(ImportVerilog)
3+
endif()
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
include(SlangCompilerOptions)
2+
3+
add_circt_unittest(CIRCTImportVerilogTests
4+
CaptureAnalysisTest.cpp
5+
)
6+
7+
target_include_directories(CIRCTImportVerilogTests
8+
PRIVATE
9+
${CIRCT_MAIN_SRC_DIR}
10+
)
11+
12+
target_link_libraries(CIRCTImportVerilogTests
13+
PRIVATE
14+
CIRCTImportVerilog
15+
LLVMSupport
16+
slang_slang
17+
)

0 commit comments

Comments
 (0)