From dc33e8bb18042ef515d9dea5c44880fd93d08b44 Mon Sep 17 00:00:00 2001 From: em2machine <92717390+em2machine@users.noreply.github.com> Date: Sat, 11 Apr 2026 07:49:45 -0400 Subject: [PATCH] Fix for Returning an object of the wrong type from a static function of a parameterized class (#5479) (#7387) --- src/V3LinkDot.cpp | 7 +- src/V3Param.cpp | 63 +++++++++++++--- .../t/t_class_param_typedef_extends.py | 18 +++++ .../t/t_class_param_typedef_extends.v | 72 ++++++++++++++++++ test_regress/t/t_param_width_loc_bad.out | 1 - test_regress/t/t_uvm_return_type.py | 18 +++++ test_regress/t/t_uvm_return_type.v | 26 +++++++ test_regress/t/t_uvm_return_type2.py | 18 +++++ test_regress/t/t_uvm_return_type2.v | 30 ++++++++ test_regress/t/t_uvm_return_type3.py | 18 +++++ test_regress/t/t_uvm_return_type3.v | 74 +++++++++++++++++++ 11 files changed, 334 insertions(+), 11 deletions(-) create mode 100755 test_regress/t/t_class_param_typedef_extends.py create mode 100644 test_regress/t/t_class_param_typedef_extends.v create mode 100755 test_regress/t/t_uvm_return_type.py create mode 100644 test_regress/t/t_uvm_return_type.v create mode 100755 test_regress/t/t_uvm_return_type2.py create mode 100644 test_regress/t/t_uvm_return_type2.v create mode 100755 test_regress/t/t_uvm_return_type3.py create mode 100644 test_regress/t/t_uvm_return_type3.v diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 5054de9dd..bb946b5af 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -5588,7 +5588,12 @@ class LinkDotResolveVisitor final : public VNVisitor { AstNodeDType* const unwrappedp = typedefp->subDTypep()->skipRefp(); if (AstClassRefDType* const classRefp = VN_CAST(unwrappedp, ClassRefDType)) { AstPin* paramsp = cpackagerefp->paramsp(); - if (paramsp) { + if (!paramsp && classRefp->paramsp()) { + // No explicit #(...) on extends clause; carry over the + // typedef's type-parameter pins so V3Param sees them. + paramsp = classRefp->paramsp()->cloneTree(true); + nodep->parameterized(true); + } else if (paramsp) { paramsp = paramsp->cloneTree(true); nodep->parameterized(true); } diff --git a/src/V3Param.cpp b/src/V3Param.cpp index d4b73eaf7..e099ea1b1 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -1295,8 +1295,8 @@ class ParamProcessor final { return isEq.isNeqZero(); } - void cellPinCleanup(AstNode* nodep, AstPin* pinp, AstNodeModule* srcModp, string& longnamer, - bool& any_overridesr) { + void cellPinCleanup(AstNode* nodep, AstPin* pinp, AstPin* paramsp, AstNodeModule* srcModp, + string& longnamer, bool& any_overridesr) { if (!pinp->exprp()) return; // No-connect if (AstVar* const modvarp = pinp->modVarp()) { if (!modvarp->isGParam()) { @@ -1336,6 +1336,48 @@ class ParamProcessor final { } AstConst* const exprp = VN_CAST(pinp->exprp(), Const); AstConst* const origp = VN_CAST(modvarp->valuep(), Const); + // Width the pin value to match the port type so that the same + // logical value always produces the same specialization name. + AstConst* normedNamep = nullptr; + if (exprp && !exprp->num().isDouble() && !exprp->num().isString()) { + AstVar* cloneVarp = modvarp->cloneTree(false); + if (AstNode* const oldValuep = cloneVarp->valuep()) { + oldValuep->unlinkFrBack(); + VL_DO_DANGLING(oldValuep->deleteTree(), oldValuep); + } + cloneVarp->valuep(exprp->cloneTree(false)); + // Clone the dtype and resolve VarRefs to other parameters + // with their already-constified pin values (e.g., N-1 in + // logic [N-1:0] becomes Const-1 which can be folded). + if (AstNodeDType* const origDTypep = modvarp->subDTypep()) { + AstNodeDType* const dtypeClonep = origDTypep->cloneTree(false); + dtypeClonep->foreach([&](AstVarRef* varrefp) { + for (AstPin* pp = paramsp; pp; pp = VN_AS(pp->nextp(), Pin)) { + if (pp->modVarp() == varrefp->varp()) { + if (AstConst* const constp = VN_CAST(pp->exprp(), Const)) { + varrefp->replaceWith(constp->cloneTree(false)); + VL_DO_DANGLING(varrefp->deleteTree(), varrefp); + } + break; + } + } + }); + if (cloneVarp->childDTypep()) + cloneVarp->childDTypep()->unlinkFrBack()->deleteTree(); + cloneVarp->childDTypep(dtypeClonep); + cloneVarp->dtypep(nullptr); + } + V3Const::constifyParamsEdit(cloneVarp); + if (AstConst* const widthedp = VN_CAST(cloneVarp->valuep(), Const)) { + // Set the constant's dtype to the port's widthed type + // so identical values hash the same in paramValueNumber. + if (cloneVarp->dtypep()) widthedp->dtypep(cloneVarp->dtypep()); + widthedp->unlinkFrBack(); + normedNamep = widthedp; + } + VL_DO_DANGLING(cloneVarp->deleteTree(), cloneVarp); + } + AstConst* const namingExprp = normedNamep ? normedNamep : exprp; if (!exprp) { // With DepGraph architecture, all expressions should be constants // by the time V3Param runs. If not, it's an error. @@ -1356,16 +1398,18 @@ class ParamProcessor final { // This prevents making additional modules, and makes coverage more // obvious as it won't show up under a unique module page name. UINFO(9, "cellPinCleanup: same as default " << pinp); - } else if (exprp->num().isDouble() || exprp->num().isString() - || exprp->num().isFourState() || exprp->num().width() != 32) { - longnamer - += ("_" + paramSmallName(srcModp, modvarp) + paramValueNumber(exprp)); + } else if (namingExprp->num().isDouble() || namingExprp->num().isString() + || namingExprp->num().isFourState() + || namingExprp->num().width() != 32) { + longnamer += ("_" + paramSmallName(srcModp, modvarp) + + paramValueNumber(namingExprp)); any_overridesr = true; } else { - longnamer - += ("_" + paramSmallName(srcModp, modvarp) + exprp->num().ascii(false)); + longnamer += ("_" + paramSmallName(srcModp, modvarp) + + namingExprp->num().ascii(false)); any_overridesr = true; } + if (normedNamep) VL_DO_DANGLING(normedNamep->deleteTree(), normedNamep); } } else if (AstParamTypeDType* const modvarp = pinp->modPTypep()) { // Handle DOT with ParseRef RHS (e.g., p_class#(8)::p_type) @@ -1681,7 +1725,8 @@ class ParamProcessor final { any_overrides = longname != srcModp->name(); } else { for (AstPin* pinp = paramsp; pinp; pinp = VN_AS(pinp->nextp(), Pin)) { - cellPinCleanup(nodep, pinp, srcModp, longname /*ref*/, any_overrides /*ref*/); + cellPinCleanup(nodep, pinp, paramsp, srcModp, longname /*ref*/, + any_overrides /*ref*/); } } IfaceRefRefs ifaceRefRefs; diff --git a/test_regress/t/t_class_param_typedef_extends.py b/test_regress/t/t_class_param_typedef_extends.py new file mode 100755 index 000000000..31b1f0e53 --- /dev/null +++ b/test_regress/t/t_class_param_typedef_extends.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: 2025 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_class_param_typedef_extends.v b/test_regress/t/t_class_param_typedef_extends.v new file mode 100644 index 000000000..8973054a7 --- /dev/null +++ b/test_regress/t/t_class_param_typedef_extends.v @@ -0,0 +1,72 @@ +// Reproduction: type parameter lost through typedef extends +// +// When a non-parameterized class extends a typedef of a parameterized class, +// the inner member's type parameter falls back to the default instead of +// inheriting from the typedef's specialization. +// +// 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: 2025 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +module t; + class my_item; + int x; + endclass + + // Inner parameterized classes + virtual class if_base #(type T1 = int, type T2 = T1); + endclass + + virtual class port_base #(type IF = if_base#()); + function void connect(port_base #(IF) other); + endfunction + endclass + + class pull_port #(type REQ = int, type RSP = REQ) + extends port_base #(if_base #(REQ, RSP)); + endclass + + class pull_imp #(type REQ = int, type RSP = REQ, type IMP = int) + extends port_base #(if_base #(REQ, RSP)); + endclass + + // Outer parameterized class with member using the type param + class driver #(type REQ = int, type RSP = REQ); + pull_port #(REQ, RSP) seq_port; + function new; + seq_port = new; + endfunction + endclass + + // Outer parameterized class on the other side + class sequencer #(type REQ = int, type RSP = REQ); + pull_imp #(REQ, RSP, sequencer #(REQ, RSP)) seq_export; + function new; + seq_export = new; + endfunction + endclass + + // Typedef specialization + extends + typedef driver #(my_item) my_driver; + + class low_driver extends my_driver; + function new; + super.new(); + endfunction + endclass + + initial begin + sequencer #(my_item) sqr; + low_driver drv; + + sqr = new; + drv = new; + + drv.seq_port.connect(sqr.seq_export); + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_param_width_loc_bad.out b/test_regress/t/t_param_width_loc_bad.out index ed7b2e12c..ac6f5d19b 100644 --- a/test_regress/t/t_param_width_loc_bad.out +++ b/test_regress/t/t_param_width_loc_bad.out @@ -1,5 +1,4 @@ %Warning-WIDTHTRUNC: t/t_param_width_loc_bad.v:19:21: Operator VAR 'PARAM' expects 1 bits on the Initial value, but Initial value's CONST '32'h1' generates 32 bits. - : ... note: In instance 't.test_i' 19 | parameter logic PARAM = 1'b0 | ^~~~~ ... For warning description see https://verilator.org/warn/WIDTHTRUNC?v=latest diff --git a/test_regress/t/t_uvm_return_type.py b/test_regress/t/t_uvm_return_type.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_uvm_return_type.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_uvm_return_type.v b/test_regress/t/t_uvm_return_type.v new file mode 100644 index 000000000..86d84c000 --- /dev/null +++ b/test_regress/t/t_uvm_return_type.v @@ -0,0 +1,26 @@ +// DESCRIPTION: Verilator: Verify that a parameterized class's static function +// return type resolves to the same specialization as the caller's context. +// +// 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 + +module t; + + class cls #(bit T = 1); + static function cls#(T) f(); + cls#(T) c = new(); + return c; + endfunction + endclass + + initial begin + static cls#(0) c = cls#(0)::f(); + if (c == null) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule diff --git a/test_regress/t/t_uvm_return_type2.py b/test_regress/t/t_uvm_return_type2.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_uvm_return_type2.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_uvm_return_type2.v b/test_regress/t/t_uvm_return_type2.v new file mode 100644 index 000000000..d13292c08 --- /dev/null +++ b/test_regress/t/t_uvm_return_type2.v @@ -0,0 +1,30 @@ +// DESCRIPTION: Verilator: Verify parameterized class self-referential return +// types work with explicit-range value parameters (e.g. logic [7:0]). +// +// This exposes a gap where the declared type has a Range child not yet folded +// by V3Width, so the targeted keyword-width normalization cannot determine the +// port width. +// +// 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 + +module t; + + class cls #(logic [7:0] T = 8'd1); + static function cls#(T) f(); + cls#(T) c = new(); + return c; + endfunction + endclass + + initial begin + static cls#(8'd0) c = cls#(8'd0)::f(); + if (c == null) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule diff --git a/test_regress/t/t_uvm_return_type3.py b/test_regress/t/t_uvm_return_type3.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_uvm_return_type3.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_uvm_return_type3.v b/test_regress/t/t_uvm_return_type3.v new file mode 100644 index 000000000..2b25d774e --- /dev/null +++ b/test_regress/t/t_uvm_return_type3.v @@ -0,0 +1,74 @@ +// DESCRIPTION: Verilator: Verify parameterized class self-referential return +// types across a variety of value-parameter types (keyword, ranged, signed). +// +// 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 + +module t; + + // Keyword type: bit (1-bit unsigned) + class cls_bit #(bit P = 1); + static function cls_bit#(P) f(); + cls_bit#(P) c = new(); + return c; + endfunction + endclass + + // Keyword type: byte (8-bit signed) + class cls_byte #(byte P = 8'd1); + static function cls_byte#(P) f(); + cls_byte#(P) c = new(); + return c; + endfunction + endclass + + // Keyword type: shortint (16-bit signed) + class cls_shortint #(shortint P = 16'd1); + static function cls_shortint#(P) f(); + cls_shortint#(P) c = new(); + return c; + endfunction + endclass + + // Keyword type: integer (32-bit signed) + class cls_integer #(integer P = 1); + static function cls_integer#(P) f(); + cls_integer#(P) c = new(); + return c; + endfunction + endclass + + // Explicit range: logic [15:0] (16-bit unsigned) + class cls_logic16 #(logic [15:0] P = 16'd1); + static function cls_logic16#(P) f(); + cls_logic16#(P) c = new(); + return c; + endfunction + endclass + + // Explicit range: logic [31:0] (32-bit unsigned) + class cls_logic32 #(logic [31:0] P = 1); + static function cls_logic32#(P) f(); + cls_logic32#(P) c = new(); + return c; + endfunction + endclass + + initial begin + static cls_bit#(0) c1 = cls_bit#(0)::f(); + static cls_byte#(8'd0) c2 = cls_byte#(8'd0)::f(); + static cls_shortint#(16'd0) c3 = cls_shortint#(16'd0)::f(); + static cls_integer#(0) c4 = cls_integer#(0)::f(); + static cls_logic16#(16'd0) c5 = cls_logic16#(16'd0)::f(); + static cls_logic32#(0) c6 = cls_logic32#(0)::f(); + + if (c1 == null || c2 == null || c3 == null + || c4 == null || c5 == null || c6 == null) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule