From 09e856d2f3721c77f537e5f98bdacd524349d2f9 Mon Sep 17 00:00:00 2001 From: Ryszard Rozak Date: Fri, 28 Apr 2023 13:20:25 +0200 Subject: [PATCH] Fix deleting unused parameterized classes (#4150) --- src/V3AstNodeOther.h | 3 ++ src/V3LinkDot.cpp | 15 ---------- src/V3Param.cpp | 30 +++++++++++++++++-- src/verilog.y | 1 + .../t/t_class_extends_int_param_bad.out | 2 +- .../t/t_class_extends_int_param_bad.v | 3 +- .../t/t_class_extends_param_unused.pl | 21 +++++++++++++ test_regress/t/t_class_extends_param_unused.v | 15 ++++++++++ test_regress/t/t_class_param_type.v | 14 +++++++++ .../t/t_class_param_unused_default.pl | 21 +++++++++++++ test_regress/t/t_class_param_unused_default.v | 26 ++++++++++++++++ 11 files changed, 131 insertions(+), 20 deletions(-) create mode 100755 test_regress/t/t_class_extends_param_unused.pl create mode 100644 test_regress/t/t_class_extends_param_unused.v create mode 100755 test_regress/t/t_class_param_unused_default.pl create mode 100644 test_regress/t/t_class_param_unused_default.v diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index cff2cfdbe..fc998872b 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -2118,6 +2118,7 @@ class AstClass final : public AstNodeModule { bool m_interfaceClass = false; // Interface class bool m_needRNG = false; // Need RNG, uses srandom/randomize bool m_virtual = false; // Virtual class + bool m_parameterized = false; // Parameterized class void insertCache(AstNode* nodep); public: @@ -2151,6 +2152,8 @@ public: void isVirtual(bool flag) { m_virtual = flag; } bool needRNG() const { return m_needRNG; } void needRNG(bool flag) { m_needRNG = flag; } + bool isParameterized() const { return m_parameterized; } + void isParameterized(bool flag) { m_parameterized = flag; } // Return true if this class is an extension of base class (SLOW) // Accepts nullptrs static bool isClassExtendedFrom(const AstClass* refClassp, const AstClass* baseClassp); diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index e86b2df33..498763fac 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -161,7 +161,6 @@ private: bool m_forPrimary; // First link bool m_forPrearray; // Compress cell__[array] refs bool m_forScopeCreation; // Remove VarXRefs for V3Scope - bool m_removeVoidParamedClasses; // Remove classes with void params public: // METHODS @@ -208,7 +207,6 @@ public: m_forPrimary = (step == LDS_PRIMARY); m_forPrearray = (step == LDS_PARAMED || step == LDS_PRIMARY); m_forScopeCreation = (step == LDS_SCOPED); - m_removeVoidParamedClasses = (step == LDS_PARAMED); s_errorThisp = this; V3Error::errorExitCb(preErrorDumpHandler); // If get error, dump self } @@ -222,7 +220,6 @@ public: bool forPrimary() const { return m_forPrimary; } bool forPrearray() const { return m_forPrearray; } bool forScopeCreation() const { return m_forScopeCreation; } - bool removeVoidParamedClasses() const { return m_removeVoidParamedClasses; } // METHODS static string nodeTextType(AstNode* nodep) { @@ -916,18 +913,6 @@ class LinkDotFindVisitor final : public VNVisitor { void visit(AstClass* nodep) override { UASSERT_OBJ(m_curSymp, nodep, "Class not under module/package/$unit"); UINFO(8, " " << nodep << endl); - // Remove classes that have void params, as they were only used for the parameterization - // step and will not be instantiated - if (m_statep->removeVoidParamedClasses()) { - for (auto* stmtp = nodep->stmtsp(); stmtp; stmtp = stmtp->nextp()) { - if (auto* dtypep = VN_CAST(stmtp, ParamTypeDType)) { - if (VN_IS(dtypep->subDTypep(), VoidDType)) { - VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); - return; - } - } - } - } VL_RESTORER(m_scope); VL_RESTORER(m_classOrPackagep); VL_RESTORER(m_modSymp); diff --git a/src/V3Param.cpp b/src/V3Param.cpp index b1324d5a8..e15d38834 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -567,8 +567,9 @@ class ParamProcessor final { // Note all module internal variables will be re-linked to the new modules by clone // However links outside the module (like on the upper cells) will not. AstNodeModule* const newmodp = srcModp->cloneTree(false); - if (VN_IS(srcModp, Class)) { - replaceRefsRecurse(newmodp->stmtsp(), VN_AS(newmodp, Class), VN_AS(srcModp, Class)); + if (AstClass* const newClassp = VN_CAST(newmodp, Class)) { + newClassp->isParameterized(false); + replaceRefsRecurse(newmodp->stmtsp(), newClassp, VN_AS(srcModp, Class)); } newmodp->name(newname); @@ -809,6 +810,9 @@ class ParamProcessor final { if (!any_overrides) { UINFO(8, "Cell parameters all match original values, skipping expansion.\n"); + // Mark that the defeult instance is used. + // It will be checked only if srcModpr is a class. + srcModpr->user2(true); } else if (AstNodeModule* const paramedModp = m_hierBlocks.findByParams(srcModpr->name(), paramsp, m_modp)) { paramedModp->dead(false); @@ -916,7 +920,7 @@ public: class ParamVisitor final : public VNVisitor { // NODE STATE // AstNodeModule::user1 -> bool: already fixed level - + // AstClass::user2 -> bool: Referenced (value read only in parameterized classes) // STATE ParamProcessor m_processor; // De-parameterize a cell, build modules UnrollStateful m_unroller; // Loop unroller @@ -928,6 +932,7 @@ class ParamVisitor final : public VNVisitor { std::vector m_dots; // Dot references to process std::multimap m_cellps; // Cells left to process (in current module) std::multimap m_workQueue; // Modules left to process + std::vector m_paramClasses; // Parameterized classes // Map from AstNodeModule to set of all AstNodeModules that instantiates it. std::unordered_map> m_parentps; @@ -1047,6 +1052,14 @@ class ParamVisitor final : public VNVisitor { void visit(AstNodeModule* nodep) override { if (nodep->recursiveClone()) nodep->dead(true); // Fake, made for recursive elimination if (nodep->dead()) return; // Marked by LinkDot (and above) + if (AstClass* const classp = VN_CAST(nodep, Class)) { + if (classp->isParameterized()) { + // Don't enter into a definition. + // If a class is used, it will be visited through a reference + m_paramClasses.push_back(classp); + return; + } + } if (m_iterateModule) { // Iterating body UINFO(4, " MOD-under-MOD. " << nodep << endl); @@ -1363,6 +1376,17 @@ public: // Re-insert modules for (AstNodeModule* const modp : modps) netlistp->addModulesp(modp); + + for (AstClass* const classp : m_paramClasses) { + if (!classp->user2()) { + // Unreferenced, so it can be removed + VL_DO_DANGLING(pushDeletep(classp->unlinkFrBack()), classp); + } else { + // Referenced. classp became a specialized class with the default + // values of parameters and is not a parameterized class anymore + classp->isParameterized(false); + } + } } } ~ParamVisitor() override = default; diff --git a/src/verilog.y b/src/verilog.y index 844853f10..c7d17f3fd 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -6775,6 +6775,7 @@ class_declaration: // ==IEEE: part of class_declaration } /*cont*/ class_itemListE yENDCLASS endLabelE { $$ = $1; $1->addMembersp($2); + if ($2) $1->isParameterized(true); $1->addExtendsp($3); $1->addExtendsp($4); $1->addMembersp($7); diff --git a/test_regress/t/t_class_extends_int_param_bad.out b/test_regress/t/t_class_extends_int_param_bad.out index 8f75bf3af..e16b75da5 100644 --- a/test_regress/t/t_class_extends_int_param_bad.out +++ b/test_regress/t/t_class_extends_int_param_bad.out @@ -1,5 +1,5 @@ %Error: t/t_class_extends_int_param_bad.v:9:23: Attempting to extend using non-class : ... In instance t - 9 | class bar #(type T=int) extends T; + 9 | class Bar #(type T=int) extends T; | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_class_extends_int_param_bad.v b/test_regress/t/t_class_extends_int_param_bad.v index f5ee566eb..062bb014c 100644 --- a/test_regress/t/t_class_extends_int_param_bad.v +++ b/test_regress/t/t_class_extends_int_param_bad.v @@ -6,10 +6,11 @@ module t (/*AUTOARG*/); - class bar #(type T=int) extends T; + class Bar #(type T=int) extends T; endclass initial begin + Bar#() bar; $stop; end endmodule diff --git a/test_regress/t/t_class_extends_param_unused.pl b/test_regress/t/t_class_extends_param_unused.pl new file mode 100755 index 000000000..aabcde63e --- /dev/null +++ b/test_regress/t/t_class_extends_param_unused.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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 + +scenarios(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_class_extends_param_unused.v b/test_regress/t/t_class_extends_param_unused.v new file mode 100644 index 000000000..fcc46a865 --- /dev/null +++ b/test_regress/t/t_class_extends_param_unused.v @@ -0,0 +1,15 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +class Foo#(type T = logic) extends T; +endclass + +module t (/*AUTOARG*/); + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_class_param_type.v b/test_regress/t/t_class_param_type.v index e9f17ef14..24b7da7a7 100644 --- a/test_regress/t/t_class_param_type.v +++ b/test_regress/t/t_class_param_type.v @@ -90,6 +90,16 @@ class ExtendsMyInt #(type T=MyInt1) extends T; endfunction endclass +class StaticX; + static int val = 1; +endclass + +class GetStaticXVal #(type T = int); + static function int get; + return T::val; + endfunction +endclass + module t (/*AUTOARG*/); initial begin @@ -112,6 +122,8 @@ module t (/*AUTOARG*/); automatic ExtendsMyInt#() ext1 = new; automatic ExtendsMyInt#(MyInt2) ext2 = new; + automatic GetStaticXVal#(StaticX) get_statix_x_val = new; + typedef bit my_bit_t; Bar#(.A(my_bit_t)) bar_a_bit = new; Bar#(.B(my_bit_t)) bar_b_bit = new; @@ -145,6 +157,8 @@ module t (/*AUTOARG*/); if (ext1.get_this_type_x() != 1) $stop; if (ext2.get_this_type_x() != 2) $stop; + if (get_statix_x_val.get() != 1) $stop; + $write("*-* All Finished *-*\n"); $finish; end diff --git a/test_regress/t/t_class_param_unused_default.pl b/test_regress/t/t_class_param_unused_default.pl new file mode 100755 index 000000000..aabcde63e --- /dev/null +++ b/test_regress/t/t_class_param_unused_default.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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 + +scenarios(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_class_param_unused_default.v b/test_regress/t/t_class_param_unused_default.v new file mode 100644 index 000000000..e031fa36f --- /dev/null +++ b/test_regress/t/t_class_param_unused_default.v @@ -0,0 +1,26 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +class Bar#(type T = int); + T t; + function new; + t = new; + endfunction +endclass + +class Baz; + int x = 1; +endclass + +module t (/*AUTOARG*/); + initial begin + Bar#(Baz) bar_baz = new; + if (bar_baz.t.x != 1) $stop; + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule