Skip to content

Commit 8011515

Browse files
committed
[#92378] Add ExternInliner pass
Add ExternInliner pass that attemts to inline externs. ExternRemover is no longer necessary, because ExternInliner allows DeclRemover to remove externs. Signed-off-by: Artur Bieniek <abieniek@antmicro.com>
1 parent 6025474 commit 8011515

10 files changed

Lines changed: 267 additions & 44 deletions

File tree

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ add_executable(sv-bugpoint
1717
source/SvBugpoint.cpp
1818
source/Utils.cpp
1919
source/SetRemovers.cpp
20+
source/ExternInliner.cpp
2021
source/BodyRemover.cpp
2122
source/LabelRemover.cpp
2223
source/BodyPartsRemover.cpp

source/ExternInliner.cpp

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
#include <slang/ast/ASTVisitor.h>
3+
#include <unordered_map>
4+
#include "IncrementalRewriter.hpp"
5+
6+
static bool hasExternQualifier(const TokenList& qualifiers) {
7+
for (const auto token : qualifiers) {
8+
if (token.kind == parsing::TokenKind::ExternKeyword) {
9+
return true;
10+
}
11+
}
12+
return false;
13+
}
14+
15+
static std::vector<parsing::Trivia> mergeExternLeadingTrivia(
16+
const std::vector<parsing::Trivia>& movedTrivia,
17+
std::span<const parsing::Trivia> originalTrivia) {
18+
std::vector<parsing::Trivia> mergedTrivia;
19+
mergedTrivia.reserve(movedTrivia.size() + originalTrivia.size());
20+
mergedTrivia.insert(mergedTrivia.end(), movedTrivia.begin(), movedTrivia.end());
21+
22+
size_t firstTrivia = 0;
23+
if (!movedTrivia.empty() && !originalTrivia.empty() &&
24+
originalTrivia.front().kind == parsing::TriviaKind::Whitespace) {
25+
// Remove the single separator space that used to be between `extern` and
26+
// the next token (for example, `extern virtual` -> `virtual`).
27+
firstTrivia = 1;
28+
}
29+
30+
mergedTrivia.insert(mergedTrivia.end(), originalTrivia.begin() + firstTrivia,
31+
originalTrivia.end());
32+
return mergedTrivia;
33+
}
34+
35+
struct ExternInlineCandidate {
36+
// Clone the declaration body, but remove the whole source node that owns it.
37+
not_null<const FunctionDeclarationSyntax*> implementationDecl;
38+
not_null<const SyntaxNode*> implementationRemovalNode;
39+
std::string methodName;
40+
};
41+
42+
using ExternInlineMap =
43+
std::unordered_map<const ClassMethodPrototypeSyntax*, ExternInlineCandidate>;
44+
45+
class ExternInlineMapper final : public ASTVisitor<ExternInlineMapper, true, true, true> {
46+
public:
47+
ExternInlineMap candidates;
48+
49+
void handle(const GenericClassDefSymbol& node) {
50+
ASTVisitor<ExternInlineMapper, true, true, true>::visitDefault(node);
51+
if (node.numSpecializations() == 0) {
52+
// In order to visit members of not specialized class we create an artificial
53+
// specialization.
54+
node.getInvalidSpecialization().visit(*this);
55+
}
56+
}
57+
58+
void handle(const MethodPrototypeSymbol& proto) {
59+
const auto* protoSyntax =
60+
proto.getSyntax() ? proto.getSyntax()->as_if<ClassMethodPrototypeSyntax>() : nullptr;
61+
const auto* impl = proto.getSubroutine();
62+
const auto* implSyntax = impl ? impl->getSyntax() : nullptr;
63+
const auto* implDecl =
64+
implSyntax ? implSyntax->as_if<FunctionDeclarationSyntax>() : nullptr;
65+
const SyntaxNode* implNode = implSyntax;
66+
67+
if (const auto* classMethodDecl =
68+
implSyntax ? implSyntax->as_if<ClassMethodDeclarationSyntax>() : nullptr) {
69+
implDecl = classMethodDecl->declaration;
70+
implNode = classMethodDecl;
71+
}
72+
73+
if (protoSyntax && hasExternQualifier(protoSyntax->qualifiers) && implDecl && implNode &&
74+
implNode != protoSyntax &&
75+
(implDecl->kind == SyntaxKind::FunctionDeclaration ||
76+
implDecl->kind == SyntaxKind::TaskDeclaration)) {
77+
candidates.emplace(protoSyntax,
78+
ExternInlineCandidate{implDecl, implNode, std::string(proto.name)});
79+
}
80+
81+
visitDefault(proto);
82+
}
83+
};
84+
85+
static ExternInlineMap makeExternInlineMap(const std::shared_ptr<SyntaxTree>& tree) {
86+
Compilation compilation;
87+
compilation.addSyntaxTree(tree);
88+
compilation.getAllDiagnostics();
89+
90+
ExternInlineMapper mapper;
91+
compilation.getRoot().visit(mapper);
92+
return std::move(mapper.candidates);
93+
}
94+
95+
class ExternInliner : public IncrementalRewriter<ExternInliner> {
96+
public:
97+
std::shared_ptr<SyntaxTree> transform(const std::shared_ptr<SyntaxTree> tree,
98+
AttemptStats& stats,
99+
int n = 1) {
100+
// transform() copies the syntax tree, so pointers found during earlier attempts are stale.
101+
// Rebuild the map on every attempt to point at nodes from the current tree.
102+
candidateByPrototype = makeExternInlineMap(tree);
103+
currentClassNode = nullptr;
104+
return IncrementalRewriter<ExternInliner>::transform(tree, stats, n);
105+
}
106+
107+
ShouldVisitChildren handle(const ClassDeclarationSyntax& node, bool isNodeRemovable) {
108+
currentClassNode = &node;
109+
visitDefault(node);
110+
currentClassNode = nullptr;
111+
return DONT_VISIT_CHILDREN;
112+
}
113+
114+
ShouldVisitChildren handle(const ClassMethodPrototypeSyntax& node, bool isNodeRemovable) {
115+
if (!isNodeRemovable) {
116+
return VISIT_CHILDREN;
117+
}
118+
119+
if (!currentClassNode) {
120+
return VISIT_CHILDREN;
121+
}
122+
123+
auto it = candidateByPrototype.find(&node);
124+
if (it == candidateByPrototype.end()) {
125+
return VISIT_CHILDREN;
126+
}
127+
128+
bool removedExtern = false;
129+
auto& replacement =
130+
createInlinedMember(node, *it->second.implementationDecl, removedExtern);
131+
if (!removedExtern) {
132+
return VISIT_CHILDREN;
133+
}
134+
135+
if (!shouldReplace(node)) {
136+
return VISIT_CHILDREN;
137+
}
138+
139+
logType<ClassMethodPrototypeSyntax>();
140+
std::cerr << prefixLines(node.toString(), "-") << "\n";
141+
std::cerr << prefixLines(it->second.implementationRemovalNode->toString(), "-") << "\n";
142+
std::cerr << prefixLines(replacement.toString(), "+") << "\n";
143+
144+
if (!it->second.methodName.empty()) {
145+
rewrittenTypeInfo = it->second.methodName;
146+
}
147+
148+
// Remove the extern declaration and place the inlined method at the end of the class,
149+
// so member accesses remain valid even if fields are declared below the prototype.
150+
insertAtBack(currentClassNode->items, replacement);
151+
remove(node);
152+
remove(*it->second.implementationRemovalNode);
153+
154+
checkPoints.push_back({node.sourceRange()});
155+
state = REGISTER_CHILD;
156+
return DONT_VISIT_CHILDREN;
157+
}
158+
159+
private:
160+
ExternInlineMap candidateByPrototype;
161+
const ClassDeclarationSyntax* currentClassNode = nullptr;
162+
163+
TokenList& cloneQualifiersWithoutExtern(const TokenList& qualifiers,
164+
bool& removedExtern,
165+
std::vector<parsing::Trivia>& leadingExternTrivia) {
166+
std::vector<Token> out;
167+
out.reserve(qualifiers.size());
168+
for (const auto token : qualifiers) {
169+
if (token.kind == parsing::TokenKind::ExternKeyword) {
170+
removedExtern = true;
171+
if (out.empty()) {
172+
for (const auto trivia : token.trivia()) {
173+
leadingExternTrivia.push_back(trivia);
174+
}
175+
}
176+
continue;
177+
}
178+
179+
if (out.empty() && !leadingExternTrivia.empty()) {
180+
auto mergedTrivia = mergeExternLeadingTrivia(leadingExternTrivia, token.trivia());
181+
auto copiedTrivia = alloc.copyFrom(std::span<const parsing::Trivia>(mergedTrivia));
182+
out.push_back(token.withTrivia(alloc, copiedTrivia));
183+
leadingExternTrivia.clear();
184+
continue;
185+
}
186+
187+
out.push_back(token.deepClone(alloc));
188+
}
189+
190+
auto copied = alloc.copyFrom(std::span<const Token>(out));
191+
return *alloc.emplace<TokenList>(copied);
192+
}
193+
194+
ClassMethodDeclarationSyntax& createInlinedMember(
195+
const ClassMethodPrototypeSyntax& prototype,
196+
const FunctionDeclarationSyntax& implementation,
197+
bool& removedExtern) {
198+
std::vector<parsing::Trivia> leadingExternTrivia;
199+
auto& qualifiers =
200+
cloneQualifiersWithoutExtern(prototype.qualifiers, removedExtern, leadingExternTrivia);
201+
202+
auto& newFunctionDecl = factory.functionDeclaration(
203+
implementation.kind, *deepClone(implementation.attributes, alloc),
204+
*deepClone(*prototype.prototype, alloc), prototype.semi.deepClone(alloc),
205+
*deepClone(implementation.items, alloc), implementation.end.deepClone(alloc),
206+
implementation.endBlockName ? deepClone(*implementation.endBlockName, alloc) : nullptr);
207+
208+
auto& classMethod = factory.classMethodDeclaration(*deepClone(prototype.attributes, alloc),
209+
qualifiers, newFunctionDecl);
210+
211+
if (!leadingExternTrivia.empty()) {
212+
auto* firstTok = classMethod.getFirstTokenPtr();
213+
ASSERT(firstTok, "inlined class method must have a first token");
214+
215+
auto mergedTrivia = mergeExternLeadingTrivia(leadingExternTrivia, firstTok->trivia());
216+
auto copiedTrivia = alloc.copyFrom(std::span<const parsing::Trivia>(mergedTrivia));
217+
*firstTok = firstTok->withTrivia(alloc, copiedTrivia);
218+
}
219+
220+
return classMethod;
221+
}
222+
};
223+
224+
template bool rewriteLoop<ExternInliner>(std::shared_ptr<SyntaxTree>& tree,
225+
std::string stageName,
226+
std::string passIdx,
227+
SvBugpoint* svBugpoint);

source/IncrementalRewriter.hpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ enum class RewriteResult {
241241
};
242242

243243
template <typename TDerived>
244-
RewriteResult rewrite(IncrementalRewriter<TDerived>& rewriter,
244+
RewriteResult rewrite(TDerived& rewriter,
245245
std::shared_ptr<SyntaxTree>& tree,
246246
std::string stageName,
247247
std::string passIdx,
@@ -263,7 +263,7 @@ RewriteResult rewrite(IncrementalRewriter<TDerived>& rewriter,
263263
}
264264

265265
template <typename TDerived>
266-
size_t rewriteBisectFailed(IncrementalRewriter<TDerived>& rewriter,
266+
size_t rewriteBisectFailed(TDerived& rewriter,
267267
std::shared_ptr<SyntaxTree>& tree,
268268
std::string stageName,
269269
std::string passIdx,
@@ -288,7 +288,7 @@ size_t rewriteBisectFailed(IncrementalRewriter<TDerived>& rewriter,
288288
}
289289

290290
template <typename TDerived>
291-
size_t rewriteBisect(IncrementalRewriter<TDerived>& rewriter,
291+
size_t rewriteBisect(TDerived& rewriter,
292292
std::shared_ptr<SyntaxTree>& tree,
293293
std::string stageName,
294294
std::string passIdx,
@@ -326,7 +326,9 @@ bool rewriteLoop(std::shared_ptr<SyntaxTree>& tree,
326326
std::string passIdx,
327327
SvBugpoint* svBugpoint) {
328328
using enum RewriteResult;
329-
IncrementalRewriter<TDerived> rewriter;
329+
// Keep the concrete type here: derived rewriters can refresh state before delegating to the
330+
// base transform().
331+
TDerived rewriter;
330332
bool committed = false;
331333
size_t rewriteLimit = svBugpoint->n_at_once;
332334
while (!rewriter.traversalDone) {

source/IncrementalRewritersFwd.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class InstantationRemover;
1919
class BindRemover;
2020
class ModuleRemover;
2121
class TypeSimplifier;
22+
class ExternInliner;
2223

2324
template <typename T>
2425
bool rewriteLoop(std::shared_ptr<SyntaxTree>& tree,

source/SetRemovers.cpp

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -228,43 +228,6 @@ SetRemover makePortsRemover(std::shared_ptr<SyntaxTree> tree) {
228228
return makeSetRemover<PortMapper>(tree);
229229
}
230230

231-
class ExternMapper final : public ASTVisitor<ExternMapper, true, true, true> {
232-
// Builds vector that maps the declarations (prototypes) and definitions (implementations) of
233-
// extern methods
234-
public:
235-
std::vector<SetRemover::RemovalSet> removals;
236-
237-
void handle(const GenericClassDefSymbol& t) {
238-
ASTVisitor<ExternMapper, true, true, true>::visitDefault(t);
239-
if (t.numSpecializations() == 0) {
240-
// in order to visit members of not specialized class we create an artificial
241-
// specialization
242-
t.getInvalidSpecialization().visit(*this);
243-
}
244-
}
245-
246-
void handle(const MethodPrototypeSymbol& proto) {
247-
SourceRange protoLocation = SourceRange::NoLocation;
248-
SourceRange implLocation = SourceRange::NoLocation;
249-
ASSERT(proto.getSyntax(), "MethodPrototypeSymbol should have syntax node");
250-
protoLocation = proto.getSyntax()->sourceRange();
251-
252-
auto impl = proto.getSubroutine();
253-
if (impl && impl->getSyntax()) {
254-
implLocation = impl->getSyntax()->sourceRange();
255-
}
256-
257-
if (protoLocation != SourceRange::NoLocation || implLocation != SourceRange::NoLocation) {
258-
removals.push_back({protoLocation, implLocation});
259-
}
260-
visitDefault(proto);
261-
}
262-
};
263-
264-
SetRemover makeExternRemover(std::shared_ptr<SyntaxTree> tree) {
265-
return makeSetRemover<ExternMapper>(tree);
266-
}
267-
268231
class StructFieldMapper final : public ASTVisitor<StructFieldMapper, true, true, true> {
269232
// Builds vector that maps the definitions and initializations of struct fields
270233
public:

source/SetRemovers.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,5 +77,4 @@ class SetRemover : public SyntaxRewriter<SetRemover> {
7777

7878
SetRemover makeFunctionArgRemover(std::shared_ptr<SyntaxTree> tree);
7979
SetRemover makePortsRemover(std::shared_ptr<SyntaxTree> tree);
80-
SetRemover makeExternRemover(std::shared_ptr<SyntaxTree> tree);
8180
SetRemover makeStructFieldRemover(std::shared_ptr<SyntaxTree> tree);

source/SvBugpoint.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ bool SvBugpoint::pass(const std::string& passIdx) {
260260
commited |= rewriteLoop<InstantationRemover>(tree, "instantiationRemover", passIdx, this);
261261
commited |= rewriteLoop<BindRemover>(tree, "bindRemover", passIdx, this);
262262
commited |= rewriteLoop<BodyPartsRemover>(tree, "bodyPartsRemover", passIdx, this);
263-
commited |= rewriteLoop(makeExternRemover(tree), tree, "externRemover", passIdx, this);
263+
commited |= rewriteLoop<ExternInliner>(tree, "externInliner", passIdx, this);
264264
commited |= rewriteLoop<DeclRemover>(tree, "declRemover", passIdx, this);
265265
commited |= rewriteLoop<StatementsRemover>(tree, "statementsRemover", passIdx, this);
266266
commited |= rewriteLoop<ImportsRemover>(tree, "importsRemover", passIdx, this);

tests/Makefile

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,17 @@ test_dont_abort_after_err:
88
test: test_short test_caliptra test_comment_dir test_tricky_missing_newline test_irremovable_verilator_config test_comment_dir2
99

1010
.PHONY: test_short
11-
test_short: test_short_exit0 test_truncator test_short_exit1 test_short_grep test_short_verilator_errmsg test_short_multi_file_verilator_errmsg test_short_multi_file_flag_y_verilator_errmsg test_short_multi_file_flag_f_verilator_errmsg test_generate
11+
test_short: test_short_exit0 test_extern_inline test_truncator test_short_exit1 test_short_grep test_short_verilator_errmsg test_short_multi_file_verilator_errmsg test_short_multi_file_flag_y_verilator_errmsg test_short_multi_file_flag_f_verilator_errmsg test_generate
1212

1313
.PHONY: test_short_exit0
1414
test_short_exit0:
1515
@./run_test short_exit0 checkexit0.sh ${INPUT_DIR}/short_in.sv
1616

17+
.PHONY: test_extern_inline
18+
test_extern_inline:
19+
@./run_test extern_inline checkverilator_run_finish.sh ${INPUT_DIR}/extern_inline.sv --n-at-once 1 && \
20+
awk -F'\t' '/externInliner/ && $$1 > 1 {print "check on trace failed - externInliner should need only one pass"; exit(1)}' out/extern_inline/debug/trace
21+
1722
.PHONY: test_truncator
1823
test_truncator:
1924
@./run_test truncator checkexit0.sh ${INPUT_DIR}/short_in.sv
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
class c;
2+
static function int f();
3+
endfunction
4+
static virtual function int g(int a);
5+
return a + 1;
6+
endfunction
7+
endclass
8+
module t;
9+
initial if(c::f()==0 && c::g(1)==2) $finish;
10+
endmodule

tests/input_files/extern_inline.sv

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class c;
2+
extern static function int f(int a);
3+
extern static virtual function int g(int a);
4+
endclass
5+
6+
function int c::f(int a);
7+
endfunction
8+
9+
function int c::g(int a);
10+
return a + 1;
11+
endfunction
12+
13+
module t;
14+
initial if(c::f(1)==0 && c::g(1)==2) $finish;
15+
endmodule

0 commit comments

Comments
 (0)