diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e54602de4..43228b494 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -62,6 +62,7 @@ set(HEADERS V3Cast.h V3Cfg.h V3Class.h + V3ClassGraph.h V3Clean.h V3Clock.h V3Combine.h @@ -230,6 +231,7 @@ set(COMMON_SOURCES V3CfgBuilder.cpp V3CfgLiveVariables.cpp V3Class.cpp + V3ClassGraph.cpp V3Clean.cpp V3Clock.cpp V3Combine.cpp diff --git a/src/Makefile_obj.in b/src/Makefile_obj.in index 1697fcfe4..1e3e60b6c 100644 --- a/src/Makefile_obj.in +++ b/src/Makefile_obj.in @@ -247,6 +247,7 @@ RAW_OBJS_PCH_ASTNOMT = \ V3CfgBuilder.o \ V3CfgLiveVariables.o \ V3Class.o \ + V3ClassGraph.o \ V3Clean.o \ V3Clock.o \ V3Combine.o \ diff --git a/src/V3ClassGraph.cpp b/src/V3ClassGraph.cpp new file mode 100644 index 000000000..45d726741 --- /dev/null +++ b/src/V3ClassGraph.cpp @@ -0,0 +1,215 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// DESCRIPTION: Verilator: Virtual function calls - function finder +// +// Code available from: https://verilator.org +// +//************************************************************************* +// +// 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: 2003-2026 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* +// +// Builds a graph of inheritance (implements & extends) +// Allows for lookups of what AstCFunc may be called +// due to a virtual function call +// +// Current limitation: This only may be called after V3Task +// when AstNodeFTasks are substituted with AstCFuncs +// +//************************************************************************* +// TODO: +// - Probably in future this phase shall have a check for overriding +// virtual functions with static functions - currently +// there is no such check - it shall happen before V3Task +// since it moves static functions outside classes +// - This shall check for error that is currently emitted in V3LinkDot +// `Class 'Derived' implements 'Base' but is missing implementation +// for 'function' (IEEE 1800-2023 8.26)` +// Sometimes this error is emitted and it is a false positive +// since the following snipped is correct but Verilator emits an error: +// +// class Base; +// virtual task foo(); +// endtask +// endclass +// +// interface class Iface; +// pure virtual task foo(); +// endclass +// +// class Derived extends Base implements Iface; +// // This is legal as it is but Verilator fails +// endclass +// +// Mentioned `IEEE 1800-2023 8.26` says: +// `When an interface class is implemented by a class, the required +// implementations of interface class methods may be +// provided by inherited virtual method implementations.` +// issue: #6908 +// +// Therefore, this shall probably become a full phase +// and check for those errors somewhere before V3Task +//************************************************************************* + +#include "V3PchAstNoMT.h" // VL_MT_DISABLED_CODE_UNIT + +#include "V3ClassGraph.h" + +VL_DEFINE_DEBUG_FUNCTIONS; + +class V3ClassGraphVertex final : public V3GraphVertex { + VL_RTTI_IMPL(V3ClassGraphVertex, V3GraphVertex) + friend class ClassGraphBuilderVisitor; + enum Virtualization : uint8_t { + NON_VIRTUAL = 0, // Not a virtual member function + VIRTUAL_IMPLICIT, // virtual keyword is not next to a function definition the ancestor + // class has such virtual function therefore, it is treated as an + // virtual + VIRTUAL, // function explicitly defined as virtual with a keyword + VIRTUAL_INHERITED, // Function not declared in a current class however, it is inherited + // and it is virtual + // Note: non-virtual inherited function may not be described with this enum - no need for + // now as well as static functions + }; + struct FuncInfo final { + AstCFunc* const m_cfuncp; // pointer to a described function + std::unordered_set + m_cache; // set of functions that may be called due to a virtual call + Virtualization m_virtualization; // Whether function is virtual (see Virtualization) + bool m_cacheInitialized; // true when m_cache stores a valid value + FuncInfo(AstCFunc* const cfuncp, Virtualization virtualization) + : m_cfuncp{cfuncp} + , m_virtualization{virtualization} + , m_cacheInitialized{false} {} + }; + std::unordered_map + m_funcsVirtualization; // Map from function names to their description + const AstClass* const m_classp; // A class of which the function is a member of + +public: + explicit V3ClassGraphVertex(V3Graph* const graphp, const AstClass* const classp) + : V3GraphVertex{graphp} + , m_classp{classp} {} + ~V3ClassGraphVertex() override = default; + + std::string name() const override { return m_classp->name(); } + + void addFunc(AstCFunc* const cfuncp) { + m_funcsVirtualization.emplace( + std::piecewise_construct, std::forward_as_tuple(cfuncp->name()), + std::forward_as_tuple(cfuncp, cfuncp->isVirtual() ? V3ClassGraphVertex::VIRTUAL + : V3ClassGraphVertex::NON_VIRTUAL)); + } + void propagateVirt(const V3ClassGraphVertex& from) { + for (auto funcs : from.m_funcsVirtualization) { + if (funcs.second.m_virtualization == NON_VIRTUAL) continue; + auto& vertex + = m_funcsVirtualization + .emplace( + std::piecewise_construct, std::forward_as_tuple(funcs.first), + std::forward_as_tuple(nullptr, V3ClassGraphVertex::VIRTUAL_INHERITED)) + .first->second; + if (vertex.m_virtualization == NON_VIRTUAL) vertex.m_virtualization = VIRTUAL_IMPLICIT; + } + } + const std::unordered_set& getCallPossibleCFuncs(const std::string& name) & { + auto it = m_funcsVirtualization.find(name); + UASSERT_OBJ(it != m_funcsVirtualization.end(), m_classp, + "Unexpected call - CFunc with name '" << name << "' not found"); + FuncInfo& info = it->second; + if (!info.m_cacheInitialized) { + if (info.m_cfuncp) info.m_cache.insert(info.m_cfuncp); + if (info.m_virtualization != V3ClassGraphVertex::NON_VIRTUAL) { + for (V3GraphEdge& edge : outEdges()) { + V3ClassGraphVertex* const other = edge.top()->as(); + const std::unordered_set otherCalls + = other->getCallPossibleCFuncs(name); + info.m_cache.insert(otherCalls.begin(), otherCalls.end()); + } + } + info.m_cacheInitialized = true; + } + return info.m_cache; + } +}; + +const std::unordered_set& +V3ClassGraph::getCallPossibleCFuncs(const AstNodeCCall* const callp) const& { + // super references shall have AstNodeCCall::funcp() pointing to the only possible + // AstCFunc that may be called + if (callp->superReference()) return m_emptySet; + const auto it = m_memberFuncToClassVertex.find(callp->funcp()); + if (it == m_memberFuncToClassVertex.end()) { + return m_emptySet; // Not under class return empty set + } + return it->second->getCallPossibleCFuncs(callp->funcp()->name()); +} + +// Build a Class graph and returns it in a V3ClassGraph +class ClassGraphBuilderVisitor final : VNVisitorConst { + std::unique_ptr m_graphp; // Graph of classes + std::unordered_map + m_vertexMap; // Maps classes to their vertices + AstClass* m_classp = nullptr; // Current class + V3ClassGraphVertex* m_classVertexp = nullptr; // vertex corresponding to a current class - + // getClassVertex(m_classp) == m_classVertexp + + V3ClassGraphVertex* getClassVertex(const AstClass* const classp) { + auto it = m_vertexMap.find(classp); + if (it != m_vertexMap.end()) return it->second; + return m_vertexMap.emplace(classp, new V3ClassGraphVertex{m_graphp.get(), classp}) + .first->second; + } + + void visit(AstCFunc* const nodep) override { + // Constructors have explicitly added super call (added by V3LinkDot if not added by user), + // therefore assume that no virtualization happens - only explicit calls + if (!m_classp || nodep->isConstructor()) return; + UASSERT_OBJ(m_classVertexp, nodep, + "If function is under class m_classVertexp shall be set"); + m_graphp->m_memberFuncToClassVertex.emplace(nodep, m_classVertexp); + m_classVertexp->addFunc(nodep); + } + void visit(AstClassExtends* const nodep) override { + UASSERT_OBJ(m_classVertexp, nodep, + "m_classVertexp shall be set while visiting AstClassExtends"); + new V3GraphEdge{m_graphp.get(), + getClassVertex(VN_AS(nodep->childDTypep(), ClassRefDType)->classp()), + m_classVertexp, 1}; + } + void visit(AstNodeModule* const nodep) override { + VL_RESTORER(m_classp); + VL_RESTORER(m_classVertexp); + m_classp = VN_CAST(nodep, Class); + if (m_classp) m_classVertexp = getClassVertex(m_classp); + iterateChildrenConst(nodep); + } + void visit(AstNode* const nodep) override { iterateChildrenConst(nodep); } + +public: + explicit ClassGraphBuilderVisitor(AstNetlist* const nodep) + : m_graphp{new V3ClassGraph} { + iterateConst(nodep); + + // Propagate virtualization + m_graphp->order(); + for (V3GraphVertex& graphVertex : m_graphp->vertices()) { + V3ClassGraphVertex* const vertexp = graphVertex.as(); + for (V3GraphEdge& outEdge : vertexp->outEdges()) { + V3ClassGraphVertex* const successorp = outEdge.top()->as(); + successorp->propagateVirt(*vertexp); + } + } + } + ~ClassGraphBuilderVisitor() override = default; + std::unique_ptr takeGraph() && { return std::move(m_graphp); } +}; + +std::unique_ptr V3ClassGraph::build(AstNetlist* netlistp) { + return ClassGraphBuilderVisitor{netlistp}.takeGraph(); +} diff --git a/src/V3ClassGraph.h b/src/V3ClassGraph.h new file mode 100644 index 000000000..528a8d552 --- /dev/null +++ b/src/V3ClassGraph.h @@ -0,0 +1,57 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// DESCRIPTION: Verilator: Virtual function calls - function finder +// +// Code available from: https://verilator.org +// +//************************************************************************* +// +// 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: 2003-2026 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* + +#ifndef VERILATOR_V3CLASS_GRAPH_H_ +#define VERILATOR_V3CLASS_GRAPH_H_ + +#include "config_build.h" +#include "verilatedos.h" + +#include "V3Ast.h" +#include "V3Graph.h" + +#include +#include +#include + +//============================================================================ + +class V3ClassGraphVertex; + +// Graph of classes - build by V3ClassGraph::build() and used to get CFuncs that may be +// called by a AstNodeCCall (due to virtualization) +class V3ClassGraph final : public V3Graph { + friend class ClassGraphBuilderVisitor; + std::unordered_map + m_memberFuncToClassVertex; // Map from member function to its class + const std::unordered_set + m_emptySet; // Always empty set - used to return a reference to an empty set + + V3ClassGraph() = default; + +public: + static std::unique_ptr build(AstNetlist*); + ~V3ClassGraph() override = default; + + // Returns possible CFuncs that may be called due to a NodeCCall passed as a callp argument + // Warning: returned set may be empty if NodeCCall is a New call or a super call - in such + // cases only `callp->funcp()` may be called. Also, if callp is to a non-member function the + // set will be empty - post V3Task static functions are not member functions anymore. + const std::unordered_set& + getCallPossibleCFuncs(const AstNodeCCall* const callp) const&; +}; + +#endif // Guard diff --git a/src/V3SchedTiming.cpp b/src/V3SchedTiming.cpp index 4734007bd..f432a9a5c 100644 --- a/src/V3SchedTiming.cpp +++ b/src/V3SchedTiming.cpp @@ -27,6 +27,7 @@ #include "V3PchAstNoMT.h" // VL_MT_DISABLED_CODE_UNIT #include "V3AstUserAllocator.h" +#include "V3ClassGraph.h" #include "V3EmitCBase.h" #include "V3Sched.h" @@ -193,6 +194,8 @@ class AwaitVisitor final : public VNVisitor { AstNodeStmt*& m_postUpdatesr; // Post updates for the trigger eval function // Additional var sensitivities std::map>& m_externalDomains; + std::unique_ptr + m_classGraphp; // class graph to get possibly called functions from a virtual call std::set m_processDomains; // Sentrees from the current process // Variables written by suspendable processes std::set m_writtenBySuspendable; @@ -352,7 +355,12 @@ class AwaitVisitor final : public VNVisitor { void visit(AstNodeCCall* const nodep) override { iterateChildren(nodep); // We need to visit bodies of non-inlined functions - visitCalledCFunc(nodep->funcp()); + const auto& cfuncps = m_classGraphp->getCallPossibleCFuncs(nodep); + if (cfuncps.empty()) { + visitCalledCFunc(nodep->funcp()); + } else { + for (AstCFunc* const cfuncp : cfuncps) visitCalledCFunc(cfuncp); + } } void visit(AstCFunc* const nodep) override { const auto& value = m_cfuncsCache(nodep); @@ -375,7 +383,8 @@ public: : m_scopeTopp{nodep->topScopep()->scopep()} , m_lbs{lbs} , m_postUpdatesr{postUpdatesr} - , m_externalDomains{externalDomains} { + , m_externalDomains{externalDomains} + , m_classGraphp{V3ClassGraph::build(nodep)} { iterate(nodep); } ~AwaitVisitor() override { diff --git a/test_regress/t/t_sched_noninlined_virt_func_suspendable.py b/test_regress/t/t_sched_noninlined_virt_func_suspendable.py new file mode 100755 index 000000000..7a669948d --- /dev/null +++ b/test_regress/t/t_sched_noninlined_virt_func_suspendable.py @@ -0,0 +1,21 @@ +#!/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', '--stats', '-fno-dfg']) + +test.execute() + +test.file_grep(test.stats, + r'Scheduling, count of non-inlined signal writes in suspendables\s+(\d+)', 6) + +test.passes() diff --git a/test_regress/t/t_sched_noninlined_virt_func_suspendable.v b/test_regress/t/t_sched_noninlined_virt_func_suspendable.v new file mode 100644 index 000000000..7cb4d469a --- /dev/null +++ b/test_regress/t/t_sched_noninlined_virt_func_suspendable.v @@ -0,0 +1,117 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0) +// verilog_format: on + +class TestBase; + virtual function int bar(); + endfunction +endclass + +interface class D1; + pure virtual task run(); +endclass + +interface class D2 implements D1; + pure virtual task a(bit x = 0); +endclass + +interface class D3 implements D1; + pure virtual task b(bit x = 0); +endclass + +interface class D4 implements D2, D3; +endclass + +package pkg; + bit [2:0] y3; +endpackage + +module t; + bit [2:0] y; + bit [2:0] z; + assign z[0] = 1'b1; + assign z[1] = !(y[0]); + assign z[2] = !(|y[1:0]); + + bit [2:0] y2; + bit [2:0] z2; + assign z2[0] = 1'b1; + assign z2[1] = !(y2[0]); + assign z2[2] = !(|y2[1:0]); + + import pkg::y3; + bit [2:0] z3; + assign z3[0] = 1'b1; + assign z3[1] = !(y3[0]); + assign z3[2] = !(|y3[1:0]); + + bit [2:0] y4; + bit [2:0] z4; + assign z4[0] = 1'b1; + assign z4[1] = !(y4[0]); + assign z4[2] = !(|y4[1:0]); + class Foo extends TestBase implements D4; + function automatic int bar(); + // verilator no_inline_task + y2 = 3'b111; + y3 = 3'b111; + return 1; + endfunction + task run(); + y = 3'b111; + #1; + `checkh(z, 3'b001); + `checkh(z2, 3'b001); + `checkh(z3, 3'b001); + `checkh(z4, 3'b111); + endtask + task a(bit x = 0); + // verilator no_inline_task + y4 = ~y4; + #1; + if (!x) b(!x); + endtask + task b(bit x = 0); + // verilator no_inline_task + if (!x) a(!x); + endtask + endclass + initial begin + static Foo inst = new; + static TestBase foo = inst; + static D3 d3 = inst; + static D1 d1 = inst; + static D4 d4 = inst; + #1; + `checkh(z, 3'b111); + `checkh(z2, 3'b111); + `checkh(z3, 3'b111); + `checkh(z4, 3'b111); + void'(foo.bar()); + #1; + `checkh(z, 3'b111); + `checkh(z2, 3'b001); + `checkh(z3, 3'b001); + `checkh(z4, 3'b111); + d1.run(); + d4.a(); + `checkh(z, 3'b001); + `checkh(z2, 3'b001); + `checkh(z3, 3'b001); + `checkh(z4, 3'b001); + d3.b(); + `checkh(z, 3'b001); + `checkh(z2, 3'b001); + `checkh(z3, 3'b001); + `checkh(z4, 3'b111); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_sched_noninlined_virt_func_suspendable2.py b/test_regress/t/t_sched_noninlined_virt_func_suspendable2.py new file mode 100755 index 000000000..eb0644215 --- /dev/null +++ b/test_regress/t/t_sched_noninlined_virt_func_suspendable2.py @@ -0,0 +1,21 @@ +#!/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', '--stats', '-fno-dfg']) + +test.execute() + +test.file_grep(test.stats, + r'Scheduling, count of non-inlined signal writes in suspendables\s+(\d+)', 13) + +test.passes() diff --git a/test_regress/t/t_sched_noninlined_virt_func_suspendable2.v b/test_regress/t/t_sched_noninlined_virt_func_suspendable2.v new file mode 100644 index 000000000..8d87dd9b5 --- /dev/null +++ b/test_regress/t/t_sched_noninlined_virt_func_suspendable2.v @@ -0,0 +1,148 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0) +// verilog_format: on + +package pkg; + bit [2:0] y3; +endpackage + +module t; + bit [2:0] y; + bit [2:0] z; + assign z[0] = 1'b1; + assign z[1] = !(y[0]); + assign z[2] = !(|y[1:0]); + + bit [2:0] y2; + bit [2:0] z2; + assign z2[0] = 1'b1; + assign z2[1] = !(y2[0]); + assign z2[2] = !(|y2[1:0]); + + import pkg::y3; + bit [2:0] z3; + assign z3[0] = 1'b1; + assign z3[1] = !(y3[0]); + assign z3[2] = !(|y3[1:0]); + + bit [2:0] y4; + bit [2:0] z4; + assign z4[0] = 1'b1; + assign z4[1] = !(y4[0]); + assign z4[2] = !(|y4[1:0]); + + bit [2:0] y5; + bit [2:0] z5; + assign z5[0] = 1'b1; + assign z5[1] = !(y5[0]); + assign z5[2] = !(|y5[1:0]); + + static bit [2:0] expected[5] = {3'b111, 3'b111, 3'b111, 3'b111, 3'b111}; + + `define check \ + do begin #1; \ + `checkh(z, expected[0]); \ + `checkh(z2, expected[1]); \ + `checkh(z3, expected[2]); \ + `checkh(z4, expected[3]); \ + `checkh(z5, expected[4]); \ + end while(0) + + class A; + virtual function int bar(); + // verilator no_inline_task + y2 = 3'b111; + expected[1] = 3'b001; + return 1; + endfunction + virtual task foo(); + y3 = 3'b111; + expected[2] = 3'b001; + endtask + virtual task a(bit x = 0); + x = ~x; // unused variable usage + #1; + endtask + endclass + + class B extends A; + task b(bit x = 0); + // verilator no_inline_task + if (!x) a(!x); + endtask + endclass + + // Commented out code defining/using BarIface is disabled due to issue: #6908 + // interface class BarIface; + // pure virtual function int bar(); + // endclass + + class C extends B /* implements BarIface */; + task foo(); + y = 3'b111; + expected[0] = 3'b001; + endtask + task a(bit x = 0); + // verilator no_inline_task + y4 = ~y4; + expected[3] = {~expected[3][2:1], 1'b1}; + #1; + if (!x) b(!x); + endtask + task b(bit x = 0); + x = ~x; // unused variable usage + #1; + endtask + endclass + + class Base; + virtual task foo(); + y5 = 3'b111; + expected[4] = 3'b001; + endtask + endclass + + class Derived extends Base; + endclass + + initial begin + static A aa = new; + static B bb = new; + static A ab = bb; + static C cc = new; + static A ac = cc; + static B bc = cc; + static Derived derived = new; + // static BarIface bar = cc; + `check; + aa.a(); + `check; + ab.a(); + `check; + bb.b(); + `check; + cc.b(); + `check; + bc.b(); + `check; + bc.a(); + `check; + bc.foo(); + `check; + bb.foo(); + `check; + // void'(bar.bar()); + `check; + derived.foo(); + `check; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule