Fix virtual interface method call inlining and IMPURE suppression (#7505)
This commit is contained in:
parent
cee174c4b6
commit
4437ae79e7
|
|
@ -61,6 +61,7 @@ class TaskFTaskVertex final : public TaskBaseVertex {
|
||||||
// Every task gets a vertex, and we link tasks together based on funcrefs.
|
// Every task gets a vertex, and we link tasks together based on funcrefs.
|
||||||
AstNodeFTask* const m_nodep;
|
AstNodeFTask* const m_nodep;
|
||||||
AstCFunc* m_cFuncp = nullptr;
|
AstCFunc* m_cFuncp = nullptr;
|
||||||
|
bool m_needsNonInlineCFunc = false; // Needs CFunc for selected non-inlined call sites
|
||||||
|
|
||||||
public:
|
public:
|
||||||
TaskFTaskVertex(V3Graph* graphp, AstNodeFTask* nodep)
|
TaskFTaskVertex(V3Graph* graphp, AstNodeFTask* nodep)
|
||||||
|
|
@ -72,6 +73,8 @@ public:
|
||||||
string dotColor() const override { return pure() ? "black" : "red"; }
|
string dotColor() const override { return pure() ? "black" : "red"; }
|
||||||
AstCFunc* cFuncp() const { return m_cFuncp; }
|
AstCFunc* cFuncp() const { return m_cFuncp; }
|
||||||
void cFuncp(AstCFunc* nodep) { m_cFuncp = nodep; }
|
void cFuncp(AstCFunc* nodep) { m_cFuncp = nodep; }
|
||||||
|
bool needsNonInlineCFunc() const { return m_needsNonInlineCFunc; }
|
||||||
|
void needsNonInlineCFunc(bool flag) { m_needsNonInlineCFunc = flag; }
|
||||||
};
|
};
|
||||||
|
|
||||||
class TaskCodeVertex final : public TaskBaseVertex {
|
class TaskCodeVertex final : public TaskBaseVertex {
|
||||||
|
|
@ -119,6 +122,17 @@ class TaskStateVisitor final : public VNVisitor {
|
||||||
|
|
||||||
public:
|
public:
|
||||||
// METHODS
|
// METHODS
|
||||||
|
static bool isVirtualIfaceMethodCall(const AstNodeFTaskRef* refp) {
|
||||||
|
const AstMethodCall* const mrefp = VN_CAST(refp, MethodCall);
|
||||||
|
if (!mrefp) return false;
|
||||||
|
const AstNodeExpr* const fromp = mrefp->fromp();
|
||||||
|
if (!fromp || !fromp->dtypep()) return false;
|
||||||
|
const AstIfaceRefDType* const ifrefp = VN_CAST(fromp->dtypep()->skipRefp(), IfaceRefDType);
|
||||||
|
return ifrefp && ifrefp->isVirtual();
|
||||||
|
}
|
||||||
|
static bool isIfaceFTaskScope(const AstScope* scopep) {
|
||||||
|
return scopep && VN_IS(scopep->modp(), Iface);
|
||||||
|
}
|
||||||
AstScope* getScope(AstNodeFTask* nodep) {
|
AstScope* getScope(AstNodeFTask* nodep) {
|
||||||
AstScope* const scopep = VN_AS(nodep->user3p(), Scope);
|
AstScope* const scopep = VN_AS(nodep->user3p(), Scope);
|
||||||
UASSERT_OBJ(scopep, nodep, "No scope for function");
|
UASSERT_OBJ(scopep, nodep, "No scope for function");
|
||||||
|
|
@ -138,6 +152,9 @@ public:
|
||||||
m_funcToClassMap[newp] = getClassp(nodep);
|
m_funcToClassMap[newp] = getClassp(nodep);
|
||||||
}
|
}
|
||||||
bool ftaskNoInline(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->noInline(); }
|
bool ftaskNoInline(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->noInline(); }
|
||||||
|
bool ftaskNeedsNonInlineCFunc(AstNodeFTask* nodep) {
|
||||||
|
return getFTaskVertex(nodep)->needsNonInlineCFunc();
|
||||||
|
}
|
||||||
AstCFunc* ftaskCFuncp(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->cFuncp(); }
|
AstCFunc* ftaskCFuncp(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->cFuncp(); }
|
||||||
void ftaskCFuncp(AstNodeFTask* nodep, AstCFunc* cfuncp) {
|
void ftaskCFuncp(AstNodeFTask* nodep, AstCFunc* cfuncp) {
|
||||||
getFTaskVertex(nodep)->cFuncp(cfuncp);
|
getFTaskVertex(nodep)->cFuncp(cfuncp);
|
||||||
|
|
@ -229,6 +246,9 @@ private:
|
||||||
UASSERT_OBJ(nodep->taskp(), nodep, "Unlinked task");
|
UASSERT_OBJ(nodep->taskp(), nodep, "Unlinked task");
|
||||||
TaskFTaskVertex* const taskVtxp = getFTaskVertex(nodep->taskp());
|
TaskFTaskVertex* const taskVtxp = getFTaskVertex(nodep->taskp());
|
||||||
new TaskEdge{&m_callGraph, m_curVxp, taskVtxp};
|
new TaskEdge{&m_callGraph, m_curVxp, taskVtxp};
|
||||||
|
if (isVirtualIfaceMethodCall(nodep) && isIfaceFTaskScope(getScope(nodep->taskp()))) {
|
||||||
|
taskVtxp->needsNonInlineCFunc(true);
|
||||||
|
}
|
||||||
// Do we have to disable inlining the function?
|
// Do we have to disable inlining the function?
|
||||||
const V3TaskConnects tconnects = V3Task::taskConnects(nodep, nodep->taskp()->stmtsp());
|
const V3TaskConnects tconnects = V3Task::taskConnects(nodep, nodep->taskp()->stmtsp());
|
||||||
if (!taskVtxp->noInline()) { // Else short-circuit below
|
if (!taskVtxp->noInline()) { // Else short-circuit below
|
||||||
|
|
@ -251,13 +271,6 @@ private:
|
||||||
if (nodep->dpiImport()) m_curVxp->noInline(true);
|
if (nodep->dpiImport()) m_curVxp->noInline(true);
|
||||||
if (nodep->classMethod()) m_curVxp->noInline(true); // Until V3Task supports it
|
if (nodep->classMethod()) m_curVxp->noInline(true); // Until V3Task supports it
|
||||||
if (nodep->recursive()) m_curVxp->noInline(true);
|
if (nodep->recursive()) m_curVxp->noInline(true);
|
||||||
// V3Scope resolves virtual-interface MethodCalls via user2p (last-wins),
|
|
||||||
// so inlining would bake in the wrong instance's VarScope refs.
|
|
||||||
if (v3Global.hasVirtIfaces()) {
|
|
||||||
if (const AstScope* const scopep = VN_CAST(nodep->user3p(), Scope)) {
|
|
||||||
if (VN_IS(scopep->modp(), Iface)) m_curVxp->noInline(true);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (nodep->isConstructor()) {
|
if (nodep->isConstructor()) {
|
||||||
m_curVxp->noInline(true);
|
m_curVxp->noInline(true);
|
||||||
m_ctorp = nodep;
|
m_ctorp = nodep;
|
||||||
|
|
@ -1495,8 +1508,10 @@ class TaskVisitor final : public VNVisitor {
|
||||||
// Iterate into the FTask we are calling. Note it may be under a different
|
// Iterate into the FTask we are calling. Note it may be under a different
|
||||||
// scope then the caller, so we need to restore state.
|
// scope then the caller, so we need to restore state.
|
||||||
VL_RESTORER(m_scopep);
|
VL_RESTORER(m_scopep);
|
||||||
|
VL_RESTORER(m_modp);
|
||||||
VL_RESTORER(m_insStmtp);
|
VL_RESTORER(m_insStmtp);
|
||||||
m_scopep = m_statep->getScope(nodep);
|
m_scopep = m_statep->getScope(nodep);
|
||||||
|
m_modp = m_scopep->modp();
|
||||||
iterate(nodep);
|
iterate(nodep);
|
||||||
}
|
}
|
||||||
void insertBeforeStmt(AstNode* nodep, AstNode* newp) {
|
void insertBeforeStmt(AstNode* nodep, AstNode* newp) {
|
||||||
|
|
@ -1528,6 +1543,16 @@ class TaskVisitor final : public VNVisitor {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool isIfaceLocalImpure(AstNodeFTask* nodep, AstNode* impurep) {
|
||||||
|
const AstScope* const scopep = m_statep->getScope(nodep);
|
||||||
|
if (!TaskStateVisitor::isIfaceFTaskScope(scopep)) return false;
|
||||||
|
|
||||||
|
const AstVarRef* const refp = VN_CAST(impurep, VarRef);
|
||||||
|
if (!refp || !refp->varScopep()) return false;
|
||||||
|
|
||||||
|
return refp->varScopep()->scopep() == scopep;
|
||||||
|
}
|
||||||
|
|
||||||
// VISITORS
|
// VISITORS
|
||||||
void visit(AstNodeModule* nodep) override {
|
void visit(AstNodeModule* nodep) override {
|
||||||
VL_RESTORER(m_modp);
|
VL_RESTORER(m_modp);
|
||||||
|
|
@ -1598,7 +1623,10 @@ class TaskVisitor final : public VNVisitor {
|
||||||
// Create cloned statements
|
// Create cloned statements
|
||||||
AstNode* beginp;
|
AstNode* beginp;
|
||||||
AstCNew* cnewp = nullptr;
|
AstCNew* cnewp = nullptr;
|
||||||
if (m_statep->ftaskNoInline(nodep->taskp())) {
|
const bool virtualIfaceCall
|
||||||
|
= TaskStateVisitor::isVirtualIfaceMethodCall(nodep)
|
||||||
|
&& TaskStateVisitor::isIfaceFTaskScope(m_statep->getScope(nodep->taskp()));
|
||||||
|
if (m_statep->ftaskNoInline(nodep->taskp()) || virtualIfaceCall) {
|
||||||
processArgs(nodep);
|
processArgs(nodep);
|
||||||
// This may share VarScope's with a public task, if any. Yuk.
|
// This may share VarScope's with a public task, if any. Yuk.
|
||||||
beginp = createNonInlinedFTask(nodep, namePrefix, outvscp, cnewp /*ref*/);
|
beginp = createNonInlinedFTask(nodep, namePrefix, outvscp, cnewp /*ref*/);
|
||||||
|
|
@ -1697,32 +1725,38 @@ class TaskVisitor final : public VNVisitor {
|
||||||
}
|
}
|
||||||
|
|
||||||
const bool noInline = m_statep->ftaskNoInline(nodep);
|
const bool noInline = m_statep->ftaskNoInline(nodep);
|
||||||
|
const bool needsNonInlineCFunc = m_statep->ftaskNeedsNonInlineCFunc(nodep);
|
||||||
// Warn if not inlining an impure ftask (unless method, recursive,
|
// Warn if not inlining an impure ftask (unless method, recursive,
|
||||||
// or interface function -- interface member access is not truly external).
|
// or interface-local member access, which is not truly external).
|
||||||
// Will likely not schedule correctly.
|
// Will likely not schedule correctly.
|
||||||
// TODO: Why not if recursive? It will not work ...
|
// TODO: Why not if recursive? It will not work ...
|
||||||
if (noInline && !nodep->classMethod() && !nodep->recursive()
|
if (noInline && !nodep->classMethod() && !nodep->recursive()) {
|
||||||
&& !VN_IS(m_modp, Iface)) {
|
|
||||||
if (AstNode* const impurep = m_statep->checkImpure(nodep)) {
|
if (AstNode* const impurep = m_statep->checkImpure(nodep)) {
|
||||||
nodep->v3warn(
|
if (!isIfaceLocalImpure(nodep, impurep)) {
|
||||||
IMPURE,
|
nodep->v3warn(
|
||||||
"Unsupported: External variable referenced by non-inlined function/task: "
|
IMPURE,
|
||||||
<< nodep->prettyNameQ() << '\n'
|
"Unsupported: External variable referenced by non-inlined function/task: "
|
||||||
<< nodep->warnContextPrimary() << '\n'
|
<< nodep->prettyNameQ() << '\n'
|
||||||
<< impurep->warnOther() << "... Location of the external reference: "
|
<< nodep->warnContextPrimary() << '\n'
|
||||||
<< impurep->prettyNameQ() << '\n'
|
<< impurep->warnOther() << "... Location of the external reference: "
|
||||||
<< impurep->warnContextSecondary());
|
<< impurep->prettyNameQ() << '\n'
|
||||||
|
<< impurep->warnContextSecondary());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (noInline || nodep->dpiImport() || nodep->dpiExport() || nodep->taskPublic()) {
|
if (noInline || needsNonInlineCFunc || nodep->dpiImport() || nodep->dpiExport()
|
||||||
|
|| nodep->taskPublic()) {
|
||||||
// Clone it first, because we may have later FTaskRef's that still need
|
// Clone it first, because we may have later FTaskRef's that still need
|
||||||
// the original version.
|
// the original version.
|
||||||
AstNodeFTask* const clonedFuncp = nodep->cloneTree(false);
|
AstNodeFTask* const clonedFuncp = nodep->cloneTree(false);
|
||||||
if (nodep->isConstructor()) m_statep->remapFuncClassp(nodep, clonedFuncp);
|
if (nodep->isConstructor()) m_statep->remapFuncClassp(nodep, clonedFuncp);
|
||||||
if (AstCFunc* const cfuncp = makeUserFunc(clonedFuncp, noInline)) {
|
const bool makeNonInlineFunc = noInline || needsNonInlineCFunc;
|
||||||
|
if (AstCFunc* const cfuncp = makeUserFunc(clonedFuncp, makeNonInlineFunc)) {
|
||||||
nodep->addNextHere(cfuncp);
|
nodep->addNextHere(cfuncp);
|
||||||
if (nodep->dpiImport() || noInline) m_statep->ftaskCFuncp(nodep, cfuncp);
|
if (nodep->dpiImport() || makeNonInlineFunc) {
|
||||||
|
m_statep->ftaskCFuncp(nodep, cfuncp);
|
||||||
|
}
|
||||||
iterateIntoFTask(clonedFuncp); // Do the clone too
|
iterateIntoFTask(clonedFuncp); // Do the clone too
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,9 @@
|
||||||
|
%Error-IMPURE: t/t_interface_func_impure_bad.v:21:8: Unsupported: External variable referenced by non-inlined function/task: 'external_write'
|
||||||
|
: ... note: In instance 't.i'
|
||||||
|
21 | task external_write;
|
||||||
|
| ^~~~~~~~~~~~~~
|
||||||
|
t/t_interface_func_impure_bad.v:23:5: ... Location of the external reference: 'external_sig'
|
||||||
|
23 | external_sig = 1'b1;
|
||||||
|
| ^~~~~~~~~~~~
|
||||||
|
... For error description see https://verilator.org/warn/IMPURE?v=latest
|
||||||
|
%Error: Exiting due to
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
#!/usr/bin/env python3
|
||||||
|
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
|
||||||
|
#
|
||||||
|
# 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-FileCopyrightText: 2026 Wilson Snyder
|
||||||
|
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
|
||||||
|
|
||||||
|
import vltest_bootstrap
|
||||||
|
|
||||||
|
test.scenarios('vlt')
|
||||||
|
|
||||||
|
test.lint(fails=True, expect_filename=test.golden_filename)
|
||||||
|
|
||||||
|
test.passes()
|
||||||
|
|
@ -0,0 +1,34 @@
|
||||||
|
// DESCRIPTION: Verilator: Verilog Test module
|
||||||
|
//
|
||||||
|
// This file ONLY is placed under the Creative Commons Public Domain.
|
||||||
|
// SPDX-FileCopyrightText: 2026 Wilson Snyder
|
||||||
|
// SPDX-License-Identifier: CC0-1.0
|
||||||
|
|
||||||
|
package pkg;
|
||||||
|
bit external_sig;
|
||||||
|
endpackage
|
||||||
|
|
||||||
|
interface iface;
|
||||||
|
import pkg::*;
|
||||||
|
|
||||||
|
bit local_sig;
|
||||||
|
|
||||||
|
task local_write;
|
||||||
|
// verilator no_inline_task
|
||||||
|
local_sig = 1'b1;
|
||||||
|
endtask
|
||||||
|
|
||||||
|
task external_write;
|
||||||
|
// verilator no_inline_task
|
||||||
|
external_sig = 1'b1;
|
||||||
|
endtask
|
||||||
|
endinterface
|
||||||
|
|
||||||
|
module t;
|
||||||
|
iface i();
|
||||||
|
|
||||||
|
initial begin
|
||||||
|
i.local_write();
|
||||||
|
i.external_write();
|
||||||
|
end
|
||||||
|
endmodule
|
||||||
|
|
@ -0,0 +1,18 @@
|
||||||
|
#!/usr/bin/env python3
|
||||||
|
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
|
||||||
|
#
|
||||||
|
# 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-FileCopyrightText: 2026 Wilson Snyder
|
||||||
|
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
|
||||||
|
|
||||||
|
import vltest_bootstrap
|
||||||
|
|
||||||
|
test.scenarios('simulator')
|
||||||
|
|
||||||
|
test.compile(verilator_flags2=["--binary"])
|
||||||
|
|
||||||
|
test.execute()
|
||||||
|
|
||||||
|
test.passes()
|
||||||
|
|
@ -0,0 +1,40 @@
|
||||||
|
// DESCRIPTION: Verilator: Verilog Test module
|
||||||
|
//
|
||||||
|
// This file ONLY is placed under the Creative Commons Public Domain.
|
||||||
|
// SPDX-FileCopyrightText: 2026 Wilson Snyder
|
||||||
|
// SPDX-License-Identifier: CC0-1.0
|
||||||
|
|
||||||
|
interface iface;
|
||||||
|
bit x;
|
||||||
|
|
||||||
|
function bit get_and_toggle();
|
||||||
|
return x++;
|
||||||
|
endfunction
|
||||||
|
endinterface
|
||||||
|
|
||||||
|
class Driver;
|
||||||
|
virtual iface vif;
|
||||||
|
|
||||||
|
function bit call();
|
||||||
|
return vif.get_and_toggle();
|
||||||
|
endfunction
|
||||||
|
endclass
|
||||||
|
|
||||||
|
module t;
|
||||||
|
iface a();
|
||||||
|
iface b();
|
||||||
|
|
||||||
|
initial begin
|
||||||
|
automatic Driver d = new;
|
||||||
|
a.x = 1'b0;
|
||||||
|
b.x = 1'b1;
|
||||||
|
d.vif = a;
|
||||||
|
if (d.call() !== 1'b0) $stop;
|
||||||
|
d.vif = b;
|
||||||
|
if (d.call() !== 1'b1) $stop;
|
||||||
|
if (a.x !== 1'b1) $stop;
|
||||||
|
if (b.x !== 1'b0) $stop;
|
||||||
|
$write("*-* All Finished *-*\n");
|
||||||
|
$finish;
|
||||||
|
end
|
||||||
|
endmodule
|
||||||
|
|
@ -0,0 +1,18 @@
|
||||||
|
#!/usr/bin/env python3
|
||||||
|
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
|
||||||
|
#
|
||||||
|
# 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-FileCopyrightText: 2026 Wilson Snyder
|
||||||
|
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
|
||||||
|
|
||||||
|
import vltest_bootstrap
|
||||||
|
|
||||||
|
test.scenarios('simulator')
|
||||||
|
|
||||||
|
test.compile(verilator_flags2=["--binary"])
|
||||||
|
|
||||||
|
test.execute()
|
||||||
|
|
||||||
|
test.passes()
|
||||||
|
|
@ -0,0 +1,28 @@
|
||||||
|
// DESCRIPTION: Verilator: Verilog Test module
|
||||||
|
//
|
||||||
|
// This file ONLY is placed under the Creative Commons Public Domain.
|
||||||
|
// SPDX-FileCopyrightText: 2026 Wilson Snyder
|
||||||
|
// SPDX-License-Identifier: CC0-1.0
|
||||||
|
|
||||||
|
interface iface;
|
||||||
|
function bit get();
|
||||||
|
static bit x;
|
||||||
|
return x++;
|
||||||
|
endfunction
|
||||||
|
endinterface
|
||||||
|
|
||||||
|
class Holder;
|
||||||
|
virtual iface vif;
|
||||||
|
endclass
|
||||||
|
|
||||||
|
module t;
|
||||||
|
iface i();
|
||||||
|
|
||||||
|
initial begin
|
||||||
|
automatic Holder h = new;
|
||||||
|
if (i.get() !== 1'b0) $stop;
|
||||||
|
if (i.get() !== 1'b1) $stop;
|
||||||
|
$write("*-* All Finished *-*\n");
|
||||||
|
$finish;
|
||||||
|
end
|
||||||
|
endmodule
|
||||||
Loading…
Reference in New Issue