From ab47fc66564022dce4f125c2010772f08b8dae49 Mon Sep 17 00:00:00 2001 From: Andrew Nolte Date: Thu, 11 Jan 2024 07:49:07 -0500 Subject: [PATCH] Fix localparam elaboration (#3858) (#4794) --- src/V3Param.cpp | 12 ++-- ...m_unsup.out => t_interface_localparam.out} | 4 +- ...ram_unsup.pl => t_interface_localparam.pl} | 2 - test_regress/t/t_interface_localparam.v | 60 +++++++++++++++++++ test_regress/t/t_interface_localparam_unsup.v | 51 ---------------- 5 files changed, 67 insertions(+), 62 deletions(-) rename test_regress/t/{t_interface_localparam_unsup.out => t_interface_localparam.out} (63%) rename test_regress/t/{t_interface_localparam_unsup.pl => t_interface_localparam.pl} (87%) create mode 100644 test_regress/t/t_interface_localparam.v delete mode 100644 test_regress/t/t_interface_localparam_unsup.v diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 806dedf9a..5169ae83b 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -825,7 +825,7 @@ class ParamProcessor final { } } - void storeOriginalParams(AstClass* const classp) { + void storeOriginalParams(AstNodeModule* const classp) { for (AstNode* stmtp = classp->stmtsp(); stmtp; stmtp = stmtp->nextp()) { AstNode* originalParamp = nullptr; if (AstVar* const varp = VN_CAST(stmtp, Var)) { @@ -870,13 +870,13 @@ class ParamProcessor final { UINFO(8, "Cell parameters all match original values, skipping expansion.\n"); // If it's the first use of the default instance, create a copy and store it in user3p. // user3p will also be used to check if the default instance is used. - if (!srcModpr->user3p() && VN_IS(srcModpr, Class)) { - AstClass* classCopyp = VN_AS(srcModpr, Class)->cloneTree(false); + if (!srcModpr->user3p() && (VN_IS(srcModpr, Class) || VN_IS(srcModpr, Iface))) { + AstNodeModule* nodeCopyp = srcModpr->cloneTree(false); // It is a temporary copy of the original class node, stored in order to create // another instances. It is needed only during class instantiation. - m_deleter.pushDeletep(classCopyp); - srcModpr->user3p(classCopyp); - storeOriginalParams(classCopyp); + m_deleter.pushDeletep(nodeCopyp); + srcModpr->user3p(nodeCopyp); + storeOriginalParams(nodeCopyp); } } else { const string newname diff --git a/test_regress/t/t_interface_localparam_unsup.out b/test_regress/t/t_interface_localparam.out similarity index 63% rename from test_regress/t/t_interface_localparam_unsup.out rename to test_regress/t/t_interface_localparam.out index 1ac204e0e..21c7ae04c 100644 --- a/test_regress/t/t_interface_localparam_unsup.out +++ b/test_regress/t/t_interface_localparam.out @@ -1,5 +1,3 @@ *-* All Finished *-* top.t.intf: symbolsPerBeat 16, symbolsPerBeatDivBy2 8, mismatch 0 -top.t.theCore.core_intf: symbolsPerBeat 64, symbolsPerBeatDivBy2 8, mismatch 1 -%Error: t/t_interface_localparam_unsup.v:23: Verilog $stop -Aborting... +top.t.theCore.core_intf: symbolsPerBeat 64, symbolsPerBeatDivBy2 32, mismatch 0 diff --git a/test_regress/t/t_interface_localparam_unsup.pl b/test_regress/t/t_interface_localparam.pl similarity index 87% rename from test_regress/t/t_interface_localparam_unsup.pl rename to test_regress/t/t_interface_localparam.pl index fa08b1e33..762975c8b 100755 --- a/test_regress/t/t_interface_localparam_unsup.pl +++ b/test_regress/t/t_interface_localparam.pl @@ -15,8 +15,6 @@ compile( execute( check_finished => 1, - fails => $Self->{vlt_all}, - expect_filename => $Self->{golden_filename}, ); ok(1); diff --git a/test_regress/t/t_interface_localparam.v b/test_regress/t/t_interface_localparam.v new file mode 100644 index 000000000..1adc2565b --- /dev/null +++ b/test_regress/t/t_interface_localparam.v @@ -0,0 +1,60 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2023 by Justin Thiel. +// SPDX-License-Identifier: CC0-1.0 + +interface SimpleIntf +#( + parameter int val = 28 +) +(); + + // This value is calculated incorrectly for other instances of + // this interface when it is accessed via the HDL for any other + // instance of this interface + localparam int valDiv2 = val/2; + localparam int valDiv4 = valDiv2/2; + + localparam bit mismatch2 = (val != (2*valDiv2) ); + localparam bit mismatch4 = (val != (4*valDiv4) ); + + initial begin + $write("%m: val %0d, valDiv2 %0d, mismatch2 %0d\n", + val, valDiv2, mismatch2); + $write("%m: val %0d, valDiv4 %0d, mismatch4 %0d\n", + val, valDiv4, mismatch2); + if (mismatch2) $stop; + if (mismatch4) $stop; + end + +endinterface + +module Core( + SimpleIntf intf +); + + // this will constify and valDiv2 will have the default value + localparam valDiv4Upper = intf.valDiv2; + + SimpleIntf #(.val(68)) core_intf (); + + initial begin + if (intf.valDiv2 != valDiv4Upper) begin + $display("%%Error: param = %0d", intf.valDiv2); + end + end +endmodule + +module t(); + + SimpleIntf intf(); + + Core theCore (.intf); + + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule diff --git a/test_regress/t/t_interface_localparam_unsup.v b/test_regress/t/t_interface_localparam_unsup.v deleted file mode 100644 index 6deb411ae..000000000 --- a/test_regress/t/t_interface_localparam_unsup.v +++ /dev/null @@ -1,51 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed into the Public Domain, for any use, -// without warranty, 2023 by Justin Thiel. -// SPDX-License-Identifier: CC0-1.0 - -interface SimpleIntf -#( - parameter int symbolsPerBeat = 16 -) -(); - - // This value is calculated incorrectly for other instances of - // this interface when it is accessed via the HDL for any other - // instance of this interface - localparam int symbolsPerBeatDivBy2 = symbolsPerBeat/2; - - localparam bit mismatch = (symbolsPerBeat != (2*symbolsPerBeatDivBy2) ); - - initial begin - $write("%m: symbolsPerBeat %0d, symbolsPerBeatDivBy2 %0d, mismatch %0d\n", - symbolsPerBeat, symbolsPerBeatDivBy2, mismatch); - if (mismatch) $stop; - end - -endinterface - -module Core( - SimpleIntf intf -); - - // NOTE: When this line is commented out the test will pass - localparam intf_symbolsPerBeatDivBy2 = intf.symbolsPerBeatDivBy2; - - localparam int core_intf_symbolsPerBeat = 64; - SimpleIntf #(.symbolsPerBeat(core_intf_symbolsPerBeat)) core_intf (); - -endmodule - -module t(); - - SimpleIntf intf(); - - Core theCore (.intf); - - initial begin - $write("*-* All Finished *-*\n"); - $finish; - end - -endmodule