From d5f9385e9c47d844205ae601cd3f82819d5a9413 Mon Sep 17 00:00:00 2001 From: em2machine <92717390+em2machine@users.noreply.github.com> Date: Wed, 6 May 2026 11:46:33 -0400 Subject: [PATCH] Fix class::localparam during elaboration (#7524) (#7534) --- src/V3AstNodeOther.h | 6 ++ src/V3AstNodes.cpp | 7 ++ src/V3Param.cpp | 55 ++++++++++- src/V3Param.h | 1 + src/Verilator.cpp | 1 + test_regress/t/t_class_localparam_dotref.py | 18 ++++ test_regress/t/t_class_localparam_dotref.v | 96 +++++++++++++++++++ .../t/t_class_localparam_dotref_bad.out | 6 ++ .../t/t_class_localparam_dotref_bad.py | 16 ++++ .../t/t_class_localparam_dotref_bad.v | 18 ++++ ...lass_localparam_dotref_funcref_arg_bad.out | 12 +++ ...class_localparam_dotref_funcref_arg_bad.py | 16 ++++ ..._class_localparam_dotref_funcref_arg_bad.v | 24 +++++ 13 files changed, 273 insertions(+), 3 deletions(-) create mode 100755 test_regress/t/t_class_localparam_dotref.py create mode 100644 test_regress/t/t_class_localparam_dotref.v create mode 100644 test_regress/t/t_class_localparam_dotref_bad.out create mode 100755 test_regress/t/t_class_localparam_dotref_bad.py create mode 100644 test_regress/t/t_class_localparam_dotref_bad.v create mode 100644 test_regress/t/t_class_localparam_dotref_funcref_arg_bad.out create mode 100755 test_regress/t/t_class_localparam_dotref_funcref_arg_bad.py create mode 100644 test_regress/t/t_class_localparam_dotref_funcref_arg_bad.v diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 691bb1e28..62caea77a 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1278,6 +1278,8 @@ class AstNetlist final : public AstNode { VTimescale m_timeprecision; // Global time precision bool m_timescaleSpecified = false; // Input HDL specified timescale uint32_t m_nTraceCodes = 0; // Number of trace codes used by design + // V3Param-deferred params awaiting V3LinkDot::linkDotParamed scope-resolution. + std::vector m_deferredParamVarps; // Sparse metadata for constants produced from named parameters/localparams. Keep this off // AstConst itself, as AstConst is a very common node and only a small fraction carry this // name. @@ -1286,6 +1288,10 @@ class AstNetlist final : public AstNode { public: AstNetlist(); ASTGEN_MEMBERS_AstNetlist; + const char* broken() const override; + void pushDeferredParamVarp(AstVar* varp) { m_deferredParamVarps.push_back(varp); } + const std::vector& deferredParamVarps() const { return m_deferredParamVarps; } + void clearDeferredParamVarps() { m_deferredParamVarps.clear(); } void deleteContents(); void cloneRelink() override { V3ERROR_NA; } // Not cloneable string name() const override VL_MT_STABLE { return "$root"; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 1d02f8ebc..fbdc2d26e 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -543,6 +543,13 @@ AstNetlist::AstNetlist() addMiscsp(m_constPoolp); } +const char* AstNetlist::broken() const { + for (const AstVar* const varp : m_deferredParamVarps) { + BROKEN_RTN(!varp || !varp->brokeExists()); + } + return nullptr; +} + string AstNetlist::astConstOrigParamName(const AstConst* nodep) const { if (!nodep->num().hasOrigParamName()) return ""; const auto it = m_constOrigParamNames.find(nodep); diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 51790b74d..0c6464e6e 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -72,6 +72,13 @@ VL_DEFINE_DEBUG_FUNCTIONS; +// Walk a class-typed node through typedef/RefDType chains to its ClassRefDType, or null. +static AstClassRefDType* classRefDTypeOfNode(AstNode* nodep) { + while (AstTypedef* const tdp = VN_CAST(nodep, Typedef)) nodep = tdp->subDTypep(); + AstNodeDType* const dtp = VN_CAST(nodep, NodeDType); + return dtp ? VN_CAST(dtp->skipRefOrNullp(), ClassRefDType) : nullptr; +} + //###################################################################### // Hierarchical block and parameter db (modules without parameters are also handled) @@ -1260,15 +1267,33 @@ class ParamProcessor final { } } } + // Typedef-aliased paramed class (e.g., typedef C#(7) my_c; my_c::b). + if (AstClassRefDType* const crp = classRefDTypeOfNode(classRefp->classOrPackageNodep())) { + if (AstClass* const srcClassp = crp->classp()) { + if (srcClassp->hasGParam() && crp->paramsp()) { + classRefDeparam(crp, srcClassp); + if (AstClass* const newClassp = crp->classp()) lhsClassp = newClassp; + } + } + } if (!lhsClassp) return; - AstTypedef* const tdefp - = VN_CAST(m_memberMap.findMember(lhsClassp, parseRefp->name()), Typedef); - if (tdefp) { + AstNode* const memberp = m_memberMap.findMember(lhsClassp, parseRefp->name()); + if (AstTypedef* const tdefp = VN_CAST(memberp, Typedef)) { AstRefDType* const refp = new AstRefDType{dotp->fileline(), tdefp->name()}; refp->typedefp(tdefp); dotp->replaceWith(refp); VL_DO_DANGLING(dotp->deleteTree(), dotp); + } else if (AstVar* const varp = VN_CAST(memberp, Var)) { + // Param/lparam member: substitute its constant value so the caller's constify can + // succeed. + if (varp->isParam() && varp->valuep()) { + if (!VN_IS(varp->valuep(), Const)) V3Const::constifyParamsEdit(varp); + if (AstConst* const constp = VN_CAST(varp->valuep(), Const)) { + dotp->replaceWith(constp->cloneTree(false)); + VL_DO_DANGLING(dotp->deleteTree(), dotp); + } + } } } @@ -2167,6 +2192,14 @@ public: } // Create new module name with _'s between the constants UINFOTREE(10, nodep, "", "cell"); + // Resolve `class::member` Dots in pin values so constify sees Consts. + // Single-pass: filter-collect class-scoped Dots, then reverse-iterate so inner + // Dots resolve before outer (vector stays empty for cells with no class Dots). + std::vector dotps; + nodep->foreach([&](AstDot* dotp) { + if (VN_IS(dotp->lhsp(), ClassOrPackageRef)) dotps.push_back(dotp); + }); + for (auto it = dotps.rbegin(); it != dotps.rend(); ++it) resolveDotToTypedef(*it); // Evaluate all module constants V3Const::constifyParamsEdit(nodep); // Set name for warnings for when we param propagate the module @@ -2756,6 +2789,13 @@ class ParamVisitor final : public VNVisitor { } }); if (hasUnresolvedLparamXRef) return; + // Defer if value contains a class::member Dot. By V3Param time the only + // surviving such Dots are paramed-class refs awaiting linkDotParamed. + if (nodep->valuep()->exists( + [](AstDot* dotp) { return VN_IS(dotp->lhsp(), ClassOrPackageRef); })) { + v3Global.rootp()->pushDeferredParamVarp(nodep); + return; + } V3Const::constifyParamsEdit(nodep); } } @@ -3254,6 +3294,7 @@ public: void V3Param::param(AstNetlist* rootp) { UINFO(2, __FUNCTION__ << ":"); + rootp->clearDeferredParamVarps(); // Defensive: drop any stragglers from a prior invocation if (dumpTreeEitherLevel() >= 9) V3LinkDotIfaceCapture::dumpEntries("before V3Param"); { ParamTop{rootp}; } @@ -3262,3 +3303,11 @@ void V3Param::param(AstNetlist* rootp) { V3Global::dumpCheckGlobalTree("param", 0, dumpTreeEitherLevel() >= 3); } + +void V3Param::finalizeDeferredParams(AstNetlist* rootp) { + // Constify params whose Dot RHS was resolved by V3LinkDot::linkDotParamed. + for (AstVar* const varp : rootp->deferredParamVarps()) { + if (varp->valuep() && !VN_IS(varp->valuep(), Const)) V3Const::constifyParamsEdit(varp); + } + rootp->clearDeferredParamVarps(); +} diff --git a/src/V3Param.h b/src/V3Param.h index 37db5d3e0..e4248dcd2 100644 --- a/src/V3Param.h +++ b/src/V3Param.h @@ -27,6 +27,7 @@ class AstNetlist; class V3Param final { public: static void param(AstNetlist* rootp) VL_MT_DISABLED; + static void finalizeDeferredParams(AstNetlist* rootp) VL_MT_DISABLED; }; #endif // Guard diff --git a/src/Verilator.cpp b/src/Verilator.cpp index 4eef35cf7..683f71ad7 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -183,6 +183,7 @@ static void process() { V3Param::param(v3Global.rootp()); V3LinkDot::linkDotParamed(v3Global.rootp()); // Cleanup as made new modules + V3Param::finalizeDeferredParams(v3Global.rootp()); V3LinkLValue::linkLValue(v3Global.rootp()); // Resolve new VarRefs // Link cleanup of 'with' as final link phase before V3Width diff --git a/test_regress/t/t_class_localparam_dotref.py b/test_regress/t/t_class_localparam_dotref.py new file mode 100755 index 000000000..8a938befd --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref.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() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_class_localparam_dotref.v b/test_regress/t/t_class_localparam_dotref.v new file mode 100644 index 000000000..5fd241c1a --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref.v @@ -0,0 +1,96 @@ +// 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 + +// Tests reference into a parameterized class via :: in a parameter expression. + +virtual class C #(parameter int a = 0); + localparam int b = a; + static function int get_a(); + return a; + endfunction + typedef enum {E0 = 100, E1 = 101} e_t; +endclass + +class D #(parameter int v = C#(7)::b); + static function int get_v(); + return v; + endfunction +endclass + +typedef C#(0) inst0; +typedef C#(1) inst1; +typedef C#(4) chain_a; +typedef chain_a chain_b; + +module t (/*AUTOARG*/); + + // Wilson's exact case: typedef-aliased paramed class lparam. + localparam int LP_TYPEDEF_LPARAM = inst0::b; + + // Direct (no typedef) paramed class lparam. + localparam int LP_DIRECT_LPARAM = C#(2)::b; + + // Static method RHS via typedef. + localparam int LP_STATIC_FUNC = inst1::get_a(); + + // Enum value RHS, direct and typedef-aliased. + localparam int LP_ENUM_DIRECT = int'(C#(3)::E0); + localparam int LP_ENUM_TYPEDEF = int'(inst0::E1); + + // Override propagation: explicit value should reach the lparam. + localparam int LP_OVERRIDE = C#(7)::a; + + // Compile-time math inside the class parameterization. + localparam int LP_MATH = C#(2 + 3)::a; + + // Module localparam used as the class parameter. + localparam int P = 5; + localparam int LP_PARAM_DRIVES_PARAM = C#(P)::a; + + // Chained typedef alias: typedef A B; B::b should resolve through both. + localparam int LP_CHAIN_TYPEDEF = chain_b::b; + + // Multiple Dots in one expression with different specializations. + localparam int LP_MULTI = C#(0)::a + C#(1)::a + C#(2)::a; + + // Class param default itself uses a paramed-class Dot, then we read v. + localparam int LP_CLASS_DEFAULT = D#()::get_v(); + + // Nested: paramed-class Dot used as the explicit parameter to another paramed class. + localparam int LP_NESTED_ARG = D#(C#(9)::b)::get_v(); + + // Typedef-aliased paramed class as inner of nested arg (silent-miscompile risk). + typedef C#(13) c13; + localparam int LP_NESTED_TYPEDEF_INNER = D#(c13::b)::get_v(); + + // 3-level nesting via lparam access: D's v gets D#(C#(15)::b), then read v. + localparam int LP_THREE_LEVEL = D#(D#(C#(15)::b)::v)::get_v(); + + // Chained typedef alias of a typedef of a paramed class. + typedef c13 c13_alias; + localparam int LP_CHAINED_TYPEDEF_INNER = D#(c13_alias::b)::get_v(); + + initial begin + if (LP_TYPEDEF_LPARAM !== 0) begin $write("%%Error: TYPEDEF_LPARAM=%0d\n", LP_TYPEDEF_LPARAM); $stop; end + if (LP_DIRECT_LPARAM !== 2) begin $write("%%Error: DIRECT_LPARAM=%0d\n", LP_DIRECT_LPARAM); $stop; end + if (LP_STATIC_FUNC !== 1) begin $write("%%Error: STATIC_FUNC=%0d\n", LP_STATIC_FUNC); $stop; end + if (LP_ENUM_DIRECT !== 100) begin $write("%%Error: ENUM_DIRECT=%0d\n", LP_ENUM_DIRECT); $stop; end + if (LP_ENUM_TYPEDEF !== 101) begin $write("%%Error: ENUM_TYPEDEF=%0d\n", LP_ENUM_TYPEDEF); $stop; end + if (LP_OVERRIDE !== 7) begin $write("%%Error: OVERRIDE=%0d\n", LP_OVERRIDE); $stop; end + if (LP_MATH !== 5) begin $write("%%Error: MATH=%0d\n", LP_MATH); $stop; end + if (LP_PARAM_DRIVES_PARAM !== 5) begin $write("%%Error: PARAM_DRIVES_PARAM=%0d\n", LP_PARAM_DRIVES_PARAM); $stop; end + if (LP_CHAIN_TYPEDEF !== 4) begin $write("%%Error: CHAIN_TYPEDEF=%0d\n", LP_CHAIN_TYPEDEF); $stop; end + if (LP_MULTI !== 3) begin $write("%%Error: MULTI=%0d\n", LP_MULTI); $stop; end + if (LP_CLASS_DEFAULT !== 7) begin $write("%%Error: CLASS_DEFAULT=%0d\n", LP_CLASS_DEFAULT); $stop; end + if (LP_NESTED_ARG !== 9) begin $write("%%Error: NESTED_ARG=%0d\n", LP_NESTED_ARG); $stop; end + if (LP_NESTED_TYPEDEF_INNER !== 13) begin $write("%%Error: NESTED_TYPEDEF_INNER=%0d\n", LP_NESTED_TYPEDEF_INNER); $stop; end + if (LP_THREE_LEVEL !== 15) begin $write("%%Error: THREE_LEVEL=%0d\n", LP_THREE_LEVEL); $stop; end + if (LP_CHAINED_TYPEDEF_INNER !== 13) begin $write("%%Error: CHAINED_TYPEDEF_INNER=%0d\n", LP_CHAINED_TYPEDEF_INNER); $stop; end + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule diff --git a/test_regress/t/t_class_localparam_dotref_bad.out b/test_regress/t/t_class_localparam_dotref_bad.out new file mode 100644 index 000000000..b164c6d02 --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref_bad.out @@ -0,0 +1,6 @@ +%Error: t/t_class_localparam_dotref_bad.v:17:33: Can't find definition of scope/variable/func: 'nonexistent' + : ... note: In instance 't' + 17 | localparam int LP_BAD = inst::nonexistent; + | ^~~~~~~~~~~ + ... 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_class_localparam_dotref_bad.py b/test_regress/t/t_class_localparam_dotref_bad.py new file mode 100755 index 000000000..38cf36b43 --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref_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('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_class_localparam_dotref_bad.v b/test_regress/t/t_class_localparam_dotref_bad.v new file mode 100644 index 000000000..cedf40460 --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref_bad.v @@ -0,0 +1,18 @@ +// 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 + +// Negative test: a parameterized class :: ref to a member that does not exist +// in a parameter expression should produce a clean error, not crash. + +class C #(parameter int a = 0); + localparam int b = a; +endclass + +typedef C#(0) inst; + +module t; + localparam int LP_BAD = inst::nonexistent; +endmodule diff --git a/test_regress/t/t_class_localparam_dotref_funcref_arg_bad.out b/test_regress/t/t_class_localparam_dotref_funcref_arg_bad.out new file mode 100644 index 000000000..59f225593 --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref_funcref_arg_bad.out @@ -0,0 +1,12 @@ +%Error-UNSUPPORTED: t/t_class_localparam_dotref_funcref_arg_bad.v:23:32: dotted expressions in parameters + : ... note: In instance 't' + : ... Suggest use a typedef + 23 | localparam int y = D#(C#(7)::get_a())::x; + | ^~~~~ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_class_localparam_dotref_funcref_arg_bad.v:23:32: Can't convert defparam value to constant: Param '__paramNumber1' of 'D' + : ... note: In instance 't' + 23 | localparam int y = D#(C#(7)::get_a())::x; + | ^~~~~ + ... 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_class_localparam_dotref_funcref_arg_bad.py b/test_regress/t/t_class_localparam_dotref_funcref_arg_bad.py new file mode 100755 index 000000000..38cf36b43 --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref_funcref_arg_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('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_class_localparam_dotref_funcref_arg_bad.v b/test_regress/t/t_class_localparam_dotref_funcref_arg_bad.v new file mode 100644 index 000000000..27d18c8b6 --- /dev/null +++ b/test_regress/t/t_class_localparam_dotref_funcref_arg_bad.v @@ -0,0 +1,24 @@ +// 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 + +// Negative test: a paramed-class static function call used directly as the +// parameter argument to another paramed class is not currently supported. +// Folding the function call to a constant during specialization requires +// constant-function evaluation that this fix does not provide. + +class C #(parameter int a = 0); + static function int get_a(); + return a; + endfunction +endclass + +class D #(parameter int v = 0); + localparam int x = v; +endclass + +module t; + localparam int y = D#(C#(7)::get_a())::x; +endmodule