diff --git a/Changes b/Changes index e06146061..43c6977dc 100644 --- a/Changes +++ b/Changes @@ -13,13 +13,13 @@ Verilator 5.041 devel **Other:** +* Add error on parameter values from hierarchical paths (#1626) (#6456). [Luca Rufer] * Add error on zero/negative unpacked dimensions (#1642). [Stefan Wallentowitz] * Add verilator_gantt profiling of DPI imports (#3084). [Geza Lore] * Add error on non-packed struct randc (#5999). [Seth Pellegrino] * Add configure `--enable-asan` to compile verilator_bin with the address sanitizer (#6404). [Geza Lore] * Add $(LDFLAGS) and $(LIBS) to when building shared libraries (#6425) (#6426). [Ahmed El-Mahmoudy] * Add ASSIGNEQEXPR when use `=` inside expressions (#5567). [Ethan Sifferman] -* Add error on localparam value from hierarchical path (#6456). [Luca Rufer] * Add IMPLICITSTATIC also on procedure variables. * Deprecate sensitivity list on public_flat_rw attributes (#6443). [Geza Lore] * Support modports referencing clocking blocks (#4555) (#6436). [Ryszard Rozak, Antmicro Ltd.] diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 4a80b99f9..8151b2713 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -1357,6 +1357,21 @@ class ParamVisitor final : public VNVisitor { m_iterateModule = false; } + void checkParamNotHier(AstNode* valuep) { + if (!valuep) return; + valuep->foreachAndNext([&](const AstNodeExpr* exprp) { + if (const AstVarXRef* refp = VN_CAST(exprp, VarXRef)) { + refp->v3error("Parameter values cannot use hierarchical values" + " (IEEE 1800-2023 6.20.2)"); + } else if (const AstNodeFTaskRef* refp = VN_CAST(exprp, NodeFTaskRef)) { + if (refp->dotted() != "") { + refp->v3error("Parameter values cannot call hierarchical functions" + " (IEEE 1800-2023 6.20.2)"); + } + } + }); + } + // A generic visitor for cells and class refs void visitCellOrClassRef(AstNode* nodep, bool isIface) { // Must do ifaces first, so push to list and do in proper order @@ -1396,14 +1411,17 @@ class ParamVisitor final : public VNVisitor { processWorkQ(); } } - void visit(AstCell* nodep) override { + checkParamNotHier(nodep->paramsp()); visitCellOrClassRef(nodep, VN_IS(nodep->modp(), Iface)); } void visit(AstIfaceRefDType* nodep) override { if (nodep->ifacep()) visitCellOrClassRef(nodep, true); } - void visit(AstClassRefDType* nodep) override { visitCellOrClassRef(nodep, false); } + void visit(AstClassRefDType* nodep) override { + checkParamNotHier(nodep->paramsp()); + visitCellOrClassRef(nodep, false); + } void visit(AstClassOrPackageRef* nodep) override { // If it points to a typedef it is not really a class reference. That typedef will be // visited anyway (from its parent node), so even if it points to a parameterized class @@ -1416,13 +1434,7 @@ class ParamVisitor final : public VNVisitor { if (nodep->user2SetOnce()) return; // Process once iterateChildren(nodep); if (nodep->isParam()) { - // See if any Future before we process - if (nodep->valuep()) - nodep->valuep()->foreach([&](const AstVarXRef* refp) { - refp->v3error("Parameter values cannot be hierarchical" - " (IEEE 1800-2023 6.20.2): " - << nodep->prettyNameQ()); - }); + checkParamNotHier(nodep->valuep()); if (!nodep->valuep() && !VN_IS(m_modp, Class)) { nodep->v3error("Parameter without default value is never given value" << " (IEEE 1800-2023 6.20.1): " << nodep->prettyNameQ()); diff --git a/test_regress/t/t_interface_param_acc_bits.out b/test_regress/t/t_interface_param_acc_bits.out deleted file mode 100644 index 51998432b..000000000 --- a/test_regress/t/t_interface_param_acc_bits.out +++ /dev/null @@ -1,5 +0,0 @@ -%Error: t/t_interface_param_acc_bits.v:15:42: Parameter-resolved constants must not use dotted references: 'dummy' - : ... note: In instance 't' - 15 | simple_bus #(.PARAMETER($bits(sb_intf.dummy))) simple(); - | ^~~~~ -%Error: Exiting due to diff --git a/test_regress/t/t_interface_param_acc_bits.py b/test_regress/t/t_interface_param_acc_bits.py deleted file mode 100755 index dbdaf4551..000000000 --- a/test_regress/t/t_interface_param_acc_bits.py +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env python3 -# DESCRIPTION: Verilator: Verilog Test driver/expect definition -# -# Copyright 2024 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_st') - -test.compile() - -test.execute() - -test.passes() diff --git a/test_regress/t/t_interface_param_acc_bits.v b/test_regress/t/t_interface_param_acc_bits.v deleted file mode 100644 index 8dc51c0fd..000000000 --- a/test_regress/t/t_interface_param_acc_bits.v +++ /dev/null @@ -1,30 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed into the Public Domain, for any use, -// without warranty, 2017 by Johan Bjork. -// SPDX-License-Identifier: CC0-1.0 - -interface simple_bus #(PARAMETER = 0); - typedef struct packed { - logic [31:0] data; - logic [3:0] mask; - } payload_t; - - parameter [6:0] DUMMY = 22; - payload_t payload; - logic [1:0] x; -endinterface - -module t (); - simple_bus sb_intf(); - const int LP = $bits(sb_intf.payload.data); - simple_bus #(.PARAMETER($bits(sb_intf.DUMMY))) simple(); - simple_bus #(.PARAMETER($bits(sb_intf.x))) simple2(); - initial begin - if (LP != 32) $stop; - if (simple.PARAMETER != 7) $stop; - if (simple2.PARAMETER != 2) $stop; - $write("*-* All Finished *-*\n"); - $finish; - end -endmodule diff --git a/test_regress/t/t_interface_param_another_bad.out b/test_regress/t/t_interface_param_another_bad.out index 2c006f019..26ce6bf02 100644 --- a/test_regress/t/t_interface_param_another_bad.out +++ b/test_regress/t/t_interface_param_another_bad.out @@ -1,8 +1,12 @@ -%Error: t/t_interface_param_another_bad.v:9:36: Expecting expression to be constant, but variable isn't const: 'dummy' +%Error: t/t_interface_param_another_bad.v:9:36: Parameter values cannot use hierarchical values (IEEE 1800-2023 6.20.2) : ... note: In instance 't' 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple(); | ^~~~~ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_interface_param_another_bad.v:9:36: Expecting expression to be constant, but variable isn't const: 'dummy' + : ... note: In instance 't' + 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple(); + | ^~~~~ %Error: t/t_interface_param_another_bad.v:9:18: Can't convert defparam value to constant: Param 'PARAMETER' of 'simple' : ... note: In instance 't' 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple(); diff --git a/test_regress/t/t_interface_param_loop_bad.py b/test_regress/t/t_interface_param_loop_bad.py deleted file mode 100755 index 564f2a1d8..000000000 --- a/test_regress/t/t_interface_param_loop_bad.py +++ /dev/null @@ -1,19 +0,0 @@ -#!/usr/bin/env python3 -# DESCRIPTION: Verilator: Verilog Test driver/expect definition -# -# Copyright 2024 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('linter') - -test.lint( - # Should fail, Verilator unsupported, bug1626 - # expect_filename = test.golden_filename, - fails=not test.vlt) - -test.passes() diff --git a/test_regress/t/t_interface_param_loop_bad.v b/test_regress/t/t_interface_param_loop_bad.v deleted file mode 100644 index 8a8113aed..000000000 --- a/test_regress/t/t_interface_param_loop_bad.v +++ /dev/null @@ -1,23 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed into the Public Domain, for any use, -// without warranty, 2019 by Johan Bjork. -// SPDX-License-Identifier: CC0-1.0 - -module t (); - simple_bus #(.WIDTH(simple.get_width())) sb_intf(); - simple_bus #(.WIDTH(sb_intf.get_width())) simple(); - - initial begin - $write("*-* All Finished *-*\n"); - $finish; - end -endmodule - -interface simple_bus #(parameter int WIDTH = 8); - logic [WIDTH-1:0] data; - - function automatic int get_width(); - return WIDTH; - endfunction -endinterface diff --git a/test_regress/t/t_param_hier_bad.out b/test_regress/t/t_param_hier_bad.out index 27cb5777f..2eda267b2 100644 --- a/test_regress/t/t_param_hier_bad.out +++ b/test_regress/t/t_param_hier_bad.out @@ -1,6 +1,14 @@ -%Error: t/t_param_hier_bad.v:26:32: Parameter values cannot be hierarchical (IEEE 1800-2023 6.20.2): 'SUB_Y' +%Error: t/t_param_hier_bad.v:36:32: Parameter values cannot use hierarchical values (IEEE 1800-2023 6.20.2) : ... note: In instance 't' - 26 | localparam int SUB_Y = u_sub.Y; + 36 | localparam int SUB_Y = u_sub.Y; | ^ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_param_hier_bad.v:38:35: Parameter values cannot call hierarchical functions (IEEE 1800-2023 6.20.2) + : ... note: In instance 't' + 38 | localparam int SUB_FUNC = u_sub.sub_func(); + | ^~~~~~~~ +%Error: t/t_param_hier_bad.v:44:18: Parameter values cannot call hierarchical functions (IEEE 1800-2023 6.20.2) + : ... note: In instance 't' + 44 | sub #(.X(block.block_func())) u_sub2 (); + | ^~~~~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_param_hier_bad.v b/test_regress/t/t_param_hier_bad.v index e9a4d1e6c..d15aa44e1 100644 --- a/test_regress/t/t_param_hier_bad.v +++ b/test_regress/t/t_param_hier_bad.v @@ -9,13 +9,23 @@ `define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); // verilog_format: on +package pkg; + function int pkg_func(); + return 1; + endfunction +endpackage + module sub #( - parameter int X = 1 + parameter int X = 1 ) (); localparam int Y = X; localparam int Z = X; + function int sub_func(); + return 1; + endfunction + endmodule module t; @@ -25,6 +35,24 @@ module t; localparam int SUB_Y = u_sub.Y; // <--- BAD: IEEE 1800-2023 6.20.2 no hierarchical + localparam int SUB_FUNC = u_sub.sub_func(); // <--- BAD: IEEE 1800-2023 6.20.2 no hierarchical + + localparam int OK_FUNC = local_func(); // ok + + localparam int OK_PKG_FUNC = pkg::pkg_func(); // ok + + sub #(.X(block.block_func())) u_sub2 (); // <--- BAD + + begin : block + function int block_func(); + return 2; + endfunction + end + + function int local_func(); + return 2; + endfunction + initial begin `checkd(SUB_Y, 1); `checkd(u_sub.X, 2);