diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 6c9a5578e..fba7690f8 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -61,6 +61,7 @@ class TaskFTaskVertex final : public TaskBaseVertex { // Every task gets a vertex, and we link tasks together based on funcrefs. AstNodeFTask* const m_nodep; AstCFunc* m_cFuncp = nullptr; + bool m_needsNonInlineCFunc = false; // Needs CFunc for selected non-inlined call sites public: TaskFTaskVertex(V3Graph* graphp, AstNodeFTask* nodep) @@ -72,6 +73,8 @@ public: string dotColor() const override { return pure() ? "black" : "red"; } AstCFunc* cFuncp() const { return m_cFuncp; } 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 { @@ -119,6 +122,17 @@ class TaskStateVisitor final : public VNVisitor { public: // 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* const scopep = VN_AS(nodep->user3p(), Scope); UASSERT_OBJ(scopep, nodep, "No scope for function"); @@ -138,6 +152,9 @@ public: m_funcToClassMap[newp] = getClassp(nodep); } bool ftaskNoInline(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->noInline(); } + bool ftaskNeedsNonInlineCFunc(AstNodeFTask* nodep) { + return getFTaskVertex(nodep)->needsNonInlineCFunc(); + } AstCFunc* ftaskCFuncp(AstNodeFTask* nodep) { return getFTaskVertex(nodep)->cFuncp(); } void ftaskCFuncp(AstNodeFTask* nodep, AstCFunc* cfuncp) { getFTaskVertex(nodep)->cFuncp(cfuncp); @@ -229,6 +246,9 @@ private: UASSERT_OBJ(nodep->taskp(), nodep, "Unlinked task"); TaskFTaskVertex* const taskVtxp = getFTaskVertex(nodep->taskp()); 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? const V3TaskConnects tconnects = V3Task::taskConnects(nodep, nodep->taskp()->stmtsp()); if (!taskVtxp->noInline()) { // Else short-circuit below @@ -251,13 +271,6 @@ private: if (nodep->dpiImport()) m_curVxp->noInline(true); if (nodep->classMethod()) m_curVxp->noInline(true); // Until V3Task supports it 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()) { m_curVxp->noInline(true); 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 // scope then the caller, so we need to restore state. VL_RESTORER(m_scopep); + VL_RESTORER(m_modp); VL_RESTORER(m_insStmtp); m_scopep = m_statep->getScope(nodep); + m_modp = m_scopep->modp(); iterate(nodep); } 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 void visit(AstNodeModule* nodep) override { VL_RESTORER(m_modp); @@ -1598,7 +1623,10 @@ class TaskVisitor final : public VNVisitor { // Create cloned statements AstNode* beginp; 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); // This may share VarScope's with a public task, if any. Yuk. 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 needsNonInlineCFunc = m_statep->ftaskNeedsNonInlineCFunc(nodep); // 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. // TODO: Why not if recursive? It will not work ... - if (noInline && !nodep->classMethod() && !nodep->recursive() - && !VN_IS(m_modp, Iface)) { + 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 (!isIfaceLocalImpure(nodep, impurep)) { + 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()) { + if (noInline || needsNonInlineCFunc || nodep->dpiImport() || nodep->dpiExport() + || nodep->taskPublic()) { // Clone it first, because we may have later FTaskRef's that still need // the original version. AstNodeFTask* const clonedFuncp = nodep->cloneTree(false); 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); - if (nodep->dpiImport() || noInline) m_statep->ftaskCFuncp(nodep, cfuncp); + if (nodep->dpiImport() || makeNonInlineFunc) { + m_statep->ftaskCFuncp(nodep, cfuncp); + } iterateIntoFTask(clonedFuncp); // Do the clone too } } diff --git a/test_regress/t/t_interface_func_impure_bad.out b/test_regress/t/t_interface_func_impure_bad.out new file mode 100644 index 000000000..70f39be73 --- /dev/null +++ b/test_regress/t/t_interface_func_impure_bad.out @@ -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 diff --git a/test_regress/t/t_interface_func_impure_bad.py b/test_regress/t/t_interface_func_impure_bad.py new file mode 100644 index 000000000..344a4e20a --- /dev/null +++ b/test_regress/t/t_interface_func_impure_bad.py @@ -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() diff --git a/test_regress/t/t_interface_func_impure_bad.v b/test_regress/t/t_interface_func_impure_bad.v new file mode 100644 index 000000000..6ef962e15 --- /dev/null +++ b/test_regress/t/t_interface_func_impure_bad.v @@ -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 diff --git a/test_regress/t/t_interface_virtual_func_runtime_instance.py b/test_regress/t/t_interface_virtual_func_runtime_instance.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_interface_virtual_func_runtime_instance.py @@ -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() diff --git a/test_regress/t/t_interface_virtual_func_runtime_instance.v b/test_regress/t/t_interface_virtual_func_runtime_instance.v new file mode 100644 index 000000000..1ad4e60e0 --- /dev/null +++ b/test_regress/t/t_interface_virtual_func_runtime_instance.v @@ -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 diff --git a/test_regress/t/t_interface_virtual_func_static_direct.py b/test_regress/t/t_interface_virtual_func_static_direct.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_interface_virtual_func_static_direct.py @@ -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() diff --git a/test_regress/t/t_interface_virtual_func_static_direct.v b/test_regress/t/t_interface_virtual_func_static_direct.v new file mode 100644 index 000000000..6d8d6cd12 --- /dev/null +++ b/test_regress/t/t_interface_virtual_func_static_direct.v @@ -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