Add `-fno-inline-funcs-eager` option to disable excessive inlining (#6682)

This commit is contained in:
Geza Lore 2025-11-11 21:46:19 +00:00 committed by GitHub
parent c5f8656aa0
commit 0dc9f779f8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 120 additions and 33 deletions

View File

@ -683,6 +683,8 @@ Summary:
.. option:: -fno-inline-funcs
.. option:: -fno-inline-funcs-eager
.. option:: -fno-life
.. option:: -fno-life-post

View File

@ -1439,6 +1439,7 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc,
DECL_OPTION("-fgate", FOnOff, &m_fGate);
DECL_OPTION("-finline", FOnOff, &m_fInline);
DECL_OPTION("-finline-funcs", FOnOff, &m_fInlineFuncs);
DECL_OPTION("-finline-funcs-eager", FOnOff, &m_fInlineFuncsEager);
DECL_OPTION("-flife", FOnOff, &m_fLife);
DECL_OPTION("-flife-post", FOnOff, &m_fLifePost);
DECL_OPTION("-flocalize", FOnOff, &m_fLocalize);

View File

@ -405,6 +405,7 @@ private:
bool m_fGate; // main switch: -fno-gate: gate wire elimination
bool m_fInline; // main switch: -fno-inline: module inlining
bool m_fInlineFuncs = true; // main switch: -fno-inline-funcs: function inlining
bool m_fInlineFuncsEager = true; // main switch: -fno-inline-funcs-eager: don't inline eagerly
bool m_fLife; // main switch: -fno-life: variable lifetime
bool m_fLifePost; // main switch: -fno-life-post: delayed assignment elimination
bool m_fLocalize; // main switch: -fno-localize: convert temps to local variables
@ -719,6 +720,7 @@ public:
bool fGate() const { return m_fGate; }
bool fInline() const { return m_fInline; }
bool fInlineFuncs() const { return m_fInlineFuncs; }
bool fInlineFuncsEager() const { return m_fInlineFuncsEager; }
bool fLife() const { return m_fLife; }
bool fLifePost() const { return m_fLifePost; }
bool fLocalize() const { return m_fLocalize; }

View File

@ -41,6 +41,7 @@ VL_DEFINE_DEBUG_FUNCTIONS;
// Graph subclasses
class TaskBaseVertex VL_NOT_FINAL : public V3GraphVertex {
VL_RTTI_IMPL(TaskBaseVertex, V3GraphVertex);
AstNode* m_impurep = nullptr; // Node causing impure function w/ outside references
bool m_noInline = false; // Marked with pragma
public:
@ -55,6 +56,7 @@ public:
};
class TaskFTaskVertex final : public TaskBaseVertex {
VL_RTTI_IMPL(TaskFTaskVertex, TaskBaseVertex);
// Every task gets a vertex, and we link tasks together based on funcrefs.
AstNodeFTask* const m_nodep;
AstCFunc* m_cFuncp = nullptr;
@ -72,6 +74,7 @@ public:
};
class TaskCodeVertex final : public TaskBaseVertex {
VL_RTTI_IMPL(TaskCodeVertex, TaskBaseVertex);
// Top vertex for all calls not under another task
public:
explicit TaskCodeVertex(V3Graph* graphp)
@ -133,38 +136,71 @@ public:
void remapFuncClassp(AstNodeFTask* nodep, AstNodeFTask* newp) {
m_funcToClassMap[newp] = getClassp(nodep);
}
bool ftaskNoInline(AstNodeFTask* nodep) {
return !v3Global.opt.fInlineFuncs() || getFTaskVertex(nodep)->noInline();
}
bool ftaskNoInline(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->noInline(); }
AstCFunc* ftaskCFuncp(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->cFuncp(); }
void ftaskCFuncp(AstNodeFTask* nodep, AstCFunc* cfuncp) {
getFTaskVertex(nodep)->cFuncp(cfuncp);
}
void checkPurity(AstNodeFTask* nodep) { checkPurity(nodep, getFTaskVertex(nodep)); }
// Returns pointer to node that makes the given FTask impure
AstNode* checkImpure(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->impureNode(); }
private:
void checkPurity(AstNodeFTask* nodep, TaskBaseVertex* vxp) {
if (nodep->recursive()) return; // Impure, but no warning
if (!vxp->pure()) {
nodep->v3warn(
IMPURE, "Unsupported: External variable referenced by non-inlined function/task: "
<< nodep->prettyNameQ() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< vxp->impureNode()->warnOther()
<< "... Location of the external reference: "
<< vxp->impureNode()->prettyNameQ() << '\n'
<< vxp->impureNode()->warnContextSecondary());
}
// And, we need to check all tasks this task calls
for (V3GraphEdge& edge : vxp->outEdges()) {
checkPurity(nodep, static_cast<TaskBaseVertex*>(edge.top()));
}
}
TaskFTaskVertex* getFTaskVertex(AstNodeFTask* nodep) {
if (!nodep->user4p()) nodep->user4p(new TaskFTaskVertex{&m_callGraph, nodep});
return static_cast<TaskFTaskVertex*>(nodep->user4u().toGraphVertex());
}
void propagateImpureReason() {
std::vector<TaskFTaskVertex*> workList;
// Gather all impure FTask vertices
for (V3GraphVertex& vtx : m_callGraph.vertices()) {
if (TaskFTaskVertex* const ftVtxp = vtx.cast<TaskFTaskVertex>()) {
if (!ftVtxp->pure()) workList.emplace_back(ftVtxp);
}
}
// Mark every pure caller as impure and enqueue them - this propagates impureness
while (!workList.empty()) {
TaskFTaskVertex* const calleep = workList.back();
workList.pop_back();
UASSERT_OBJ(!calleep->pure(), calleep->nodep(), "Should not enqueue pure vertex");
for (V3GraphEdge& edge : calleep->inEdges()) {
if (TaskFTaskVertex* const callerp = edge.fromp()->cast<TaskFTaskVertex>()) {
if (!callerp->pure()) continue; // Already processed or on workList
callerp->impure(calleep->impureNode());
workList.emplace_back(callerp);
}
}
}
}
void decideInlining() {
for (V3GraphVertex& vtx : m_callGraph.vertices()) {
TaskFTaskVertex* const ftVtxp = vtx.cast<TaskFTaskVertex>();
if (!ftVtxp) continue;
// Move on if already decided not to inline
if (ftVtxp->noInline()) continue;
// If inlining blanked disabled, don't do it. Note this usually
// results in large amounts of breakage so is mostly for testing.
if (!v3Global.opt.fInlineFuncs()) {
ftVtxp->noInline(true);
continue;
}
// If eagerly inlining (historic - and still default - behaviour) then nothing to do
if (v3Global.opt.fInlineFuncsEager()) continue;
AstNodeFTask* const ftaskp = ftVtxp->nodep();
// We are relying on inlining of some internal functions for
// functional corretness. Probably that's not a good thing?
if (!ftaskp->verilogFunction()) continue;
// Non-inlned public functions are a bit broken right now, so keep
// inlining them (internal calls are inlined + keep public version)
if (ftaskp->taskPublic()) continue;
// If the function is pure, it's OK not to inline, so don't do it
if (ftVtxp->pure()) ftVtxp->noInline(true);
}
}
// VISITORS
void visit(AstScope* nodep) override {
// Each FTask is unique per-scope, so AstNodeFTaskRefs do not need
@ -279,6 +315,8 @@ public:
iterate(nodep);
//
m_callGraph.removeRedundantEdgesSum(&TaskEdge::followAlwaysTrue);
propagateImpureReason();
decideInlining();
if (dumpGraphLevel()) m_callGraph.dumpDotFilePrefixed("task_call");
}
~TaskStateVisitor() override = default;
@ -1520,7 +1558,7 @@ class TaskVisitor final : public VNVisitor {
}
UINFO(4, " FTask REF Done.");
}
void visit(AstNodeFTask* nodep) override {
void visit(AstNodeFTask* const nodep) override {
UINFO(4, " visitFTask " << nodep);
VL_RESTORER(m_insStmtp);
m_insStmtp = nodep->stmtsp(); // Might be null if no statements, but we won't use it
@ -1543,22 +1581,31 @@ class TaskVisitor final : public VNVisitor {
<< nodep->prettyNameQ());
}
if (nodep->dpiImport() || nodep->dpiExport() || nodep->taskPublic()
|| m_statep->ftaskNoInline(nodep)) {
const bool noInline = m_statep->ftaskNoInline(nodep);
// Warn if not inlining an impure ftask (unless method or recursvie).
// Will likely not schedule correctly.
// TODO: Why not if recursive? It will not work ...
if (noInline && !nodep->classMethod() && !nodep->recursive()) {
if (AstNode* const impurep = m_statep->checkImpure(nodep)) {
nodep->v3warn(
IMPURE,
"Unsupported: External variable referenced by non-inlined function/task: "
<< nodep->prettyNameQ() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< impurep->warnOther() << "... Location of the external reference: "
<< impurep->prettyNameQ() << '\n'
<< impurep->warnContextSecondary());
}
}
if (noInline || nodep->dpiImport() || nodep->dpiExport() || nodep->taskPublic()) {
// Clone it first, because we may have later FTaskRef's that still need
// the original version.
if (m_statep->ftaskNoInline(nodep) && !nodep->classMethod()) {
m_statep->checkPurity(nodep);
}
AstNodeFTask* const clonedFuncp = nodep->cloneTree(false);
if (nodep->isConstructor()) m_statep->remapFuncClassp(nodep, clonedFuncp);
AstCFunc* const cfuncp = makeUserFunc(clonedFuncp, m_statep->ftaskNoInline(nodep));
if (cfuncp) {
if (AstCFunc* const cfuncp = makeUserFunc(clonedFuncp, noInline)) {
nodep->addNextHere(cfuncp);
if (nodep->dpiImport() || m_statep->ftaskNoInline(nodep)) {
m_statep->ftaskCFuncp(nodep, cfuncp);
}
if (nodep->dpiImport() || noInline) m_statep->ftaskCFuncp(nodep, cfuncp);
iterateIntoFTask(clonedFuncp); // Do the clone too
}
}

View File

@ -5,4 +5,10 @@
13 | sig = '1;
| ^~~
... For error description see https://verilator.org/warn/IMPURE?v=latest
%Error-IMPURE: t/t_func_impure_bad.v:20:9: Unsupported: External variable referenced by non-inlined function/task: 't.baz'
20 | task baz;
| ^~~
t/t_func_impure_bad.v:17:7: ... Location of the external reference: 't.sig'
17 | sig = '1;
| ^~~
%Error: Exiting due to

View File

@ -13,8 +13,18 @@ module t;
sig = '1;
endtask
task bar;
sig = '1;
endtask
task baz;
// verilator no_inline_task
bar();
endtask
initial begin
foo();
baz();
end
endmodule

View File

@ -0,0 +1,19 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2024 by Wilson Snyder. This program is free software; you
# can redistribute it and/or modify it under the terms of either the GNU
# Lesser General Public License Version 3 or the Perl Artistic License
# Version 2.0.
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('vlt')
test.top_filename = "t/t_opt_inline_funcs.v"
test.compile(verilator_flags2=['--fno-inline-funcs-eager', '--stats'], verilator_make_gmake=False)
test.file_grep(test.stats, r'Optimizations, Functions inlined\s+(\d+)', 1)
test.passes()