From 905c0c4f6d4d2dd0ae7f9be693330d01d2fbec67 Mon Sep 17 00:00:00 2001 From: Artur Bieniek Date: Fri, 5 Sep 2025 19:23:43 +0200 Subject: [PATCH] Optimize dead functions without references (#6380) --- src/V3Dead.cpp | 50 ++++++++++++++++---- src/V3Width.cpp | 5 -- src/V3WidthCommit.cpp | 33 +++++++++++++ test_regress/t/t_class_param_virtual_bad.out | 10 ++-- test_regress/t/t_func_virt_new.py | 18 +++++++ test_regress/t/t_func_virt_new.v | 19 ++++++++ test_regress/t/t_func_virt_new_bad.out | 6 +++ test_regress/t/t_func_virt_new_bad.py | 16 +++++++ test_regress/t/t_func_virt_new_bad.v | 19 ++++++++ test_regress/t/t_opt_dead_task.py | 21 ++++++++ test_regress/t/t_opt_dead_task.v | 28 +++++++++++ 11 files changed, 207 insertions(+), 18 deletions(-) create mode 100755 test_regress/t/t_func_virt_new.py create mode 100644 test_regress/t/t_func_virt_new.v create mode 100755 test_regress/t/t_func_virt_new_bad.out create mode 100755 test_regress/t/t_func_virt_new_bad.py create mode 100644 test_regress/t/t_func_virt_new_bad.v create mode 100755 test_regress/t/t_opt_dead_task.py create mode 100644 test_regress/t/t_opt_dead_task.v diff --git a/src/V3Dead.cpp b/src/V3Dead.cpp index 469c2e578..cf548f880 100644 --- a/src/V3Dead.cpp +++ b/src/V3Dead.cpp @@ -37,6 +37,8 @@ #include "V3Dead.h" +#include "V3Stats.h" + #include VL_DEFINE_DEBUG_FUNCTIONS; @@ -51,6 +53,7 @@ class DeadVisitor final : public VNVisitor { // AstVar::user1() -> int. Count of number of references // AstVarScope::user1() -> int. Count of number of references // AstNodeDType::user1() -> int. Count of number of references + // AstNodeFTask::user1() -> int. Non-zero if ever referenced (called) const VNUser1InUse m_inuser1; // TYPES @@ -69,6 +72,7 @@ class DeadVisitor final : public VNVisitor { std::vector m_cellsp; std::vector m_classesp; std::vector m_typedefsp; + std::vector m_tasksp; // All the tasks that could be removed if not called AssignMap m_assignMap; // List of all simple assignments for each variable bool m_sideEffect = false; // Side effects discovered in assign RHS @@ -78,6 +82,9 @@ class DeadVisitor final : public VNVisitor { AstNodeModule* m_modp = nullptr; // Current module AstSelLoopVars* m_selloopvarsp = nullptr; // Current loop vars + // STATE - Statistic tracking + VDouble0 m_statFTasksDeadified; + // METHODS void deleting(AstNode* nodep) { @@ -166,6 +173,7 @@ class DeadVisitor final : public VNVisitor { void visit(AstNodeFTaskRef* nodep) override { iterateChildren(nodep); checkAll(nodep); + if (nodep->taskp()) nodep->taskp()->user1(1); if (nodep->classOrPackagep()) { if (m_elimCells) { nodep->classOrPackagep(nullptr); @@ -174,9 +182,10 @@ class DeadVisitor final : public VNVisitor { } } } - void visit(AstMethodCall* nodep) override { + void visit(AstModportFTaskRef* nodep) override { iterateChildren(nodep); checkAll(nodep); + if (nodep->ftaskp()) nodep->ftaskp()->user1(1); } void visit(AstRefDType* nodep) override { iterateChildren(nodep); @@ -312,6 +321,19 @@ class DeadVisitor final : public VNVisitor { } if (assignInAssign) m_sideEffect = true; // Parent assign shouldn't optimize } + void visit(AstNodeFTask* nodep) override { + iterateChildren(nodep); + checkAll(nodep); + if (!nodep->taskPublic() && !nodep->dpiExport() && !nodep->dpiImport()) + m_tasksp.push_back(nodep); + if (nodep->classOrPackagep()) { + if (m_elimCells) { + nodep->classOrPackagep(nullptr); + } else { + nodep->classOrPackagep()->user1Inc(); + } + } + } //----- void visit(AstClockingItem* nodep) override { @@ -342,6 +364,15 @@ class DeadVisitor final : public VNVisitor { return m_elimCells && !typedefp->attrPublic(); } + void deadCheckTasks() { + for (AstNodeFTask* taskp : m_tasksp) { + if (!taskp->user1() && !taskp->classMethod()) { + deleting(taskp); + m_statFTasksDeadified++; + } + } + } + void deadCheckMod() { // Kill any unused modules // V3LinkCells has a graph that is capable of this too, but we need to do it @@ -515,7 +546,7 @@ class DeadVisitor final : public VNVisitor { public: // CONSTRUCTORS DeadVisitor(AstNetlist* nodep, bool elimUserVars, bool elimDTypes, bool elimScopes, - bool elimCells, bool elimTopIfaces) + bool elimCells, bool elimTopIfaces, bool elimTasks) : m_elimUserVars{elimUserVars} , m_elimDTypes{elimDTypes} , m_elimCells{elimCells} { @@ -534,6 +565,7 @@ public: if (itr.first->user1()) itr.second->user1Inc(); } + if (elimTasks) deadCheckTasks(); deadCheckTypedefs(); deadCheckVar(); // We only eliminate scopes when in a flattened structure @@ -549,7 +581,9 @@ public: nodep->typeTablep()->repairCache(); VIsCached::clearCacheTree(); // Removing assignments may affect isPure } - ~DeadVisitor() override = default; + ~DeadVisitor() override { + V3Stats::addStatSum("Optimizations, deadified FTasks", m_statFTasksDeadified); + }; }; //###################################################################### @@ -558,31 +592,31 @@ public: void V3Dead::deadifyModules(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":"); { // node, elimUserVars, elimDTypes, elimScopes, elimCells, elimTopIfaces - DeadVisitor{nodep, false, false, false, false, !v3Global.opt.topIfacesSupported()}; + DeadVisitor{nodep, false, false, false, false, !v3Global.opt.topIfacesSupported(), false}; } // Destruct before checking V3Global::dumpCheckGlobalTree("deadModules", 0, dumpTreeEitherLevel() >= 6); } void V3Dead::deadifyDTypes(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":"); - { DeadVisitor{nodep, false, true, false, false, false}; } // Destruct before checking + { DeadVisitor{nodep, false, true, false, false, false, true}; } // Destruct before checking V3Global::dumpCheckGlobalTree("deadDtypes", 0, dumpTreeEitherLevel() >= 3); } void V3Dead::deadifyDTypesScoped(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":"); - { DeadVisitor{nodep, false, true, true, false, false}; } // Destruct before checking + { DeadVisitor{nodep, false, true, true, false, false, false}; } // Destruct before checking V3Global::dumpCheckGlobalTree("deadDtypesScoped", 0, dumpTreeEitherLevel() >= 3); } void V3Dead::deadifyAll(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":"); - { DeadVisitor{nodep, true, true, false, true, false}; } // Destruct before checking + { DeadVisitor{nodep, true, true, false, true, false, false}; } // Destruct before checking V3Global::dumpCheckGlobalTree("deadAll", 0, dumpTreeEitherLevel() >= 3); } void V3Dead::deadifyAllScoped(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":"); - { DeadVisitor{nodep, true, true, true, true, false}; } // Destruct before checking + { DeadVisitor{nodep, true, true, true, true, false, false}; } // Destruct before checking V3Global::dumpCheckGlobalTree("deadAllScoped", 0, dumpTreeEitherLevel() >= 3); } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 867fbd324..0daa043cc 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -4582,11 +4582,6 @@ class WidthVisitor final : public VNVisitor { // Either made explicitly or V3LinkDot made implicitly classp->v3fatalSrc("Can't find class's new"); } - if (classp->isVirtual() || classp->isInterfaceClass()) { - nodep->v3error("Illegal to call 'new' using an abstract virtual class " - + AstNode::prettyNameQ(classp->origName()) - + " (IEEE 1800-2023 8.21)"); - } } else { // super.new case // in this case class and taskp() should be properly linked in V3LinkDot.cpp during // "super" reference resolution diff --git a/src/V3WidthCommit.cpp b/src/V3WidthCommit.cpp index ccf3d80cd..8d826e1a7 100644 --- a/src/V3WidthCommit.cpp +++ b/src/V3WidthCommit.cpp @@ -38,7 +38,11 @@ VL_DEFINE_DEBUG_FUNCTIONS; class WidthCommitVisitor final : public VNVisitor { // NODE STATE // AstVar::user1p -> bool, processed + // AstNodeFTask::user2() -> int. Non-zero if ever referenced (called) + // AstNew::user2() -> int. Count of number of references, minus references in + // functions never called const VNUser1InUse m_inuser1; + const VNUser2InUse m_inuser2; // STATE AstNodeModule* m_modp = nullptr; @@ -47,6 +51,8 @@ class WidthCommitVisitor final : public VNVisitor { = false; // Writing a dynamically-sized array element, not the array itself VMemberMap m_memberMap; // Member names cached for fast lookup bool m_underSel = false; // Whether is currently under AstMemberSel or AstSel + std::vector m_virtualNewsp; // Instantiations of virtual classes + std::vector m_tasksp; // All the tasks, we will check if they are ever called public: // METHODS @@ -151,6 +157,20 @@ private: } } + void deadCheckTasks() { + for (AstNodeFTask* taskp : m_tasksp) { + if (!taskp->user2()) { + taskp->foreach([](AstNew* newp) { newp->user2Inc(-1); }); + } + } + for (AstNew* newp : m_virtualNewsp) { + if (newp->user2() > 0) + newp->v3error("Illegal to call 'new' using an abstract virtual class " + + AstNode::prettyNameQ(newp->classOrPackagep()->origName()) + + " (IEEE 1800-2023 8.21)"); + } + } + // VISITORS void visit(AstNodeModule* nodep) override { VL_RESTORER(m_modp); @@ -291,6 +311,8 @@ private: nodep->virtRefDType2p(editOneDType(nodep->virtRefDType2p())); } void visit(AstNodeFTask* nodep) override { + if (!nodep->taskPublic() && !nodep->dpiExport() && !nodep->dpiImport()) + m_tasksp.push_back(nodep); iterateChildren(nodep); editDType(nodep); { @@ -371,6 +393,16 @@ private: iterateChildren(nodep); editDType(nodep); classEncapCheck(nodep, nodep->taskp(), VN_CAST(nodep->classOrPackagep(), Class)); + if (nodep->taskp()) nodep->taskp()->user2(1); + if (AstNew* const newp = VN_CAST(nodep, New)) { + if (!VN_IS(newp->backp(), Assign)) return; + if (AstClass* const classp = VN_CAST(newp->classOrPackagep(), Class)) { + if (classp->isVirtual() || classp->isInterfaceClass()) { + m_virtualNewsp.push_back(newp); + newp->user2Inc(); + } + } + } } void visit(AstMemberSel* nodep) override { { @@ -430,6 +462,7 @@ public: // Were changing widthMin's, so the table is now somewhat trashed nodep->typeTablep()->clearCache(); iterate(nodep); + deadCheckTasks(); // Don't want to AstTypeTable::repairCache, as all needed nodes // have been added back in; a repair would prevent dead nodes from // being detected diff --git a/test_regress/t/t_class_param_virtual_bad.out b/test_regress/t/t_class_param_virtual_bad.out index 31b5689f3..22322b74a 100644 --- a/test_regress/t/t_class_param_virtual_bad.out +++ b/test_regress/t/t_class_param_virtual_bad.out @@ -1,10 +1,10 @@ -%Error: t/t_class_param_virtual_bad.v:13:11: Illegal to call 'new' using an abstract virtual class 'VBase' (IEEE 1800-2023 8.21) - : ... note: In instance 't' - 13 | t = new; - | ^~~ - ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. %Error: t/t_class_param_virtual_bad.v:23:28: Illegal to call 'new' using an abstract virtual class 'ClsVirt' (IEEE 1800-2023 8.21) : ... note: In instance 't' 23 | ClsVirt#(VBase) cv = new; | ^~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_class_param_virtual_bad.v:13:11: Illegal to call 'new' using an abstract virtual class 'VBase' (IEEE 1800-2023 8.21) + : ... note: In instance 't' + 13 | t = new; + | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_func_virt_new.py b/test_regress/t/t_func_virt_new.py new file mode 100755 index 000000000..80ec03e15 --- /dev/null +++ b/test_regress/t/t_func_virt_new.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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.lint() + +test.compile() + +test.passes() diff --git a/test_regress/t/t_func_virt_new.v b/test_regress/t/t_func_virt_new.v new file mode 100644 index 000000000..149e8b286 --- /dev/null +++ b/test_regress/t/t_func_virt_new.v @@ -0,0 +1,19 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +class cl #(type T= int); + function void f(); + T obj = new; + endfunction +endclass +virtual class vcl; +endclass; +module t; + cl #(vcl) c = new; + initial begin + + end +endmodule diff --git a/test_regress/t/t_func_virt_new_bad.out b/test_regress/t/t_func_virt_new_bad.out new file mode 100755 index 000000000..8b1c621bf --- /dev/null +++ b/test_regress/t/t_func_virt_new_bad.out @@ -0,0 +1,6 @@ +%Error: t/t_func_virt_new_bad.v:9:13: Illegal to call 'new' using an abstract virtual class 'vcl' (IEEE 1800-2023 8.21) + : ... note: In instance 't' + 9 | T obj = new; + | ^~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: Exiting due to diff --git a/test_regress/t/t_func_virt_new_bad.py b/test_regress/t/t_func_virt_new_bad.py new file mode 100755 index 000000000..1bf1426f9 --- /dev/null +++ b/test_regress/t/t_func_virt_new_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_func_virt_new_bad.v b/test_regress/t/t_func_virt_new_bad.v new file mode 100644 index 000000000..8424235f4 --- /dev/null +++ b/test_regress/t/t_func_virt_new_bad.v @@ -0,0 +1,19 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +class cl #(type T= int); + function void f(); + T obj = new; + endfunction +endclass +virtual class vcl; +endclass; +module t; + cl #(vcl) c = new; + initial begin + c.f(); + end +endmodule diff --git a/test_regress/t/t_opt_dead_task.py b/test_regress/t/t_opt_dead_task.py new file mode 100755 index 000000000..95f680736 --- /dev/null +++ b/test_regress/t/t_opt_dead_task.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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('simulator') + +test.compile(verilator_flags2=["--stats"]) + +if test.vlt_all: + test.file_grep(test.stats, r'Optimizations, deadified FTasks\s+(\d+)', 2) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_opt_dead_task.v b/test_regress/t/t_opt_dead_task.v new file mode 100644 index 000000000..0be4aa58a --- /dev/null +++ b/test_regress/t/t_opt_dead_task.v @@ -0,0 +1,28 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +module t (input clk); + // AstLet and AstProperty are also NodeFTasks, but lets are substituted earlier and properties should be "used" by their asserts so also not deadified + let nclk = ~clk; + assert property(@(posedge clk)1); + + function void livefunc(); + endfunction + task livetask; + endtask + // Tasks/functions that are called somewhere will not be deadified + initial begin + livefunc(); + livetask(); + $finish; + end + + // These should be deadified + function deadfunc(); + endfunction + task deadtask; + endtask +endmodule