From 1de33b9fb763dd57fb62a2a7e50878d30dd2a654 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 22 Jul 2021 21:09:24 +0100 Subject: [PATCH] Support localparams in tasks/functions --- Changes | 1 + src/V3Param.cpp | 11 -- src/V3Task.cpp | 54 +++++---- test_regress/t/t_param_in_func.pl | 33 ++++++ test_regress/t/t_param_in_func.v | 130 +++++++++++++++++++++ test_regress/t/t_param_in_func_bad.out | 6 - test_regress/t/t_param_in_func_bad.pl | 19 --- test_regress/t/t_param_in_func_bad.v | 29 ----- test_regress/t/t_param_in_func_noinline.pl | 35 ++++++ 9 files changed, 233 insertions(+), 85 deletions(-) create mode 100755 test_regress/t/t_param_in_func.pl create mode 100644 test_regress/t/t_param_in_func.v delete mode 100644 test_regress/t/t_param_in_func_bad.out delete mode 100755 test_regress/t/t_param_in_func_bad.pl delete mode 100644 test_regress/t/t_param_in_func_bad.v create mode 100755 test_regress/t/t_param_in_func_noinline.pl diff --git a/Changes b/Changes index 4b4f51fbd..e2e7cf0da 100644 --- a/Changes +++ b/Changes @@ -13,6 +13,7 @@ Verilator 4.211 devel **Minor:** +* Support unpackes array localparams in tasks/functions (#3078). [Geza Lore] * Output files are split based on the set of headers required in order to aid incremental compilation via ccache (#3071). [Geza Lore] * Parameter values are now emitted as 'static constexpr' instead of enum. diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 3552d6a4a..733cb33d1 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -960,17 +960,6 @@ class ParamVisitor final : public AstNVisitor { << " (IEEE 1800-2017 6.20.1): " << nodep->prettyNameQ()); } else { V3Const::constifyParamsEdit(nodep); // The variable, not just the var->init() - if (!VN_IS(nodep->valuep(), Const) - && !VN_IS(nodep->valuep(), Unbounded)) { // Complex init, like an array - if (nodep->isFuncLocal()) { - // We should move the parameter out of the function and to the - // module, with appropriate dotting, but this confuses LinkDot - // (as then name isn't found later), so punt - probably can - // treat as static function variable when that is supported. - nodep->v3warn(E_UNSUPPORTED, - "Unsupported: Parameters in functions with complex assign"); - } - } } } } diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 9b2226468..3bf94dcb0 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -419,17 +419,22 @@ private: return newvscp; } AstVarScope* createVarScope(AstVar* invarp, const string& name) { - // We could create under either the ref's scope or the ftask's scope. - // It shouldn't matter, as they are only local variables. - // We choose to do it under whichever called this function, which results - // in more cache locality. - AstVar* newvarp = new AstVar(invarp->fileline(), AstVarType::BLOCKTEMP, name, invarp); - newvarp->funcLocal(false); - newvarp->propagateAttrFrom(invarp); - m_modp->addStmtp(newvarp); - AstVarScope* newvscp = new AstVarScope(newvarp->fileline(), m_scopep, newvarp); - m_scopep->addVarp(newvscp); - return newvscp; + if (invarp->isParam() && VN_IS(invarp->valuep(), InitArray)) { + // Move array params in functions into constant pool + return v3Global.rootp()->constPoolp()->findTable(VN_CAST(invarp->valuep(), InitArray)); + } else { + // We could create under either the ref's scope or the ftask's scope. + // It shouldn't matter, as they are only local variables. + // We choose to do it under whichever called this function, which results + // in more cache locality. + AstVar* newvarp = new AstVar{invarp->fileline(), AstVarType::BLOCKTEMP, name, invarp}; + newvarp->funcLocal(false); + newvarp->propagateAttrFrom(invarp); + m_modp->addStmtp(newvarp); + AstVarScope* newvscp = new AstVarScope{newvarp->fileline(), m_scopep, newvarp}; + m_scopep->addVarp(newvscp); + return newvscp; + } } AstNode* createInlinedFTask(AstNodeFTaskRef* refp, const string& namePrefix, @@ -1192,18 +1197,27 @@ private: for (AstNode *nextp, *stmtp = nodep->stmtsp(); stmtp; stmtp = nextp) { nextp = stmtp->nextp(); if (AstVar* portp = VN_CAST(stmtp, Var)) { - if (portp->isIO()) { - // Move it to new function + if (portp->isParam() && VN_IS(portp->valuep(), InitArray)) { + // Move array parameters in functions into constant pool portp->unlinkFrBack(); - portp->funcLocal(true); - cfuncp->addArgsp(portp); + pushDeletep(portp); + AstNode* const tablep = v3Global.rootp()->constPoolp()->findTable( + VN_CAST(portp->valuep(), InitArray)); + portp->user2p(tablep); } else { - // "Normal" variable, mark inside function - portp->funcLocal(true); + if (portp->isIO()) { + // Move it to new function + portp->unlinkFrBack(); + portp->funcLocal(true); + cfuncp->addArgsp(portp); + } else { + // "Normal" variable, mark inside function + portp->funcLocal(true); + } + AstVarScope* newvscp = new AstVarScope{portp->fileline(), m_scopep, portp}; + m_scopep->addVarp(newvscp); + portp->user2p(newvscp); } - AstVarScope* newvscp = new AstVarScope(portp->fileline(), m_scopep, portp); - m_scopep->addVarp(newvscp); - portp->user2p(newvscp); } } diff --git a/test_regress/t/t_param_in_func.pl b/test_regress/t/t_param_in_func.pl new file mode 100755 index 000000000..849ab9a69 --- /dev/null +++ b/test_regress/t/t_param_in_func.pl @@ -0,0 +1,33 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2019 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( + verilator_flags2 => ["--stats"], + ); + +execute(check_finished => 1); + +# The parameter array should have been put in the constant pool +if ($Self->{vlt_all}) { + file_grep($Self->{stats}, qr/ConstPool, Tables emitted\s+(\d+)/i, 3); +} + +# Shouldn't have any references to the parameter array +foreach my $file ( + glob_all("$Self->{obj_dir}/$Self->{VM_PREFIX}*.h"), + glob_all("$Self->{obj_dir}/$Self->{VM_PREFIX}*.cpp") + ) { + file_grep_not($file, qr/digits/i); +} + +ok(1); +1; diff --git a/test_regress/t/t_param_in_func.v b/test_regress/t/t_param_in_func.v new file mode 100644 index 000000000..62e86b395 --- /dev/null +++ b/test_regress/t/t_param_in_func.v @@ -0,0 +1,130 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2019 by Driss Hafdi. +// SPDX-License-Identifier: CC0-1.0 + +module t (/*AUTOARG*/); + initial begin + if (getUnpacked($c("0")) != "0") $stop; + if (getUnpacked($c("1")) != "1") $stop; + if (getUnpacked($c("2")) != "2") $stop; + if (getUnpacked($c("3")) != "3") $stop; + if (getUnpacked($c("4")) != "4") $stop; + if (getUnpacked($c("5")) != "5") $stop; + if (getUnpacked($c("6")) != "6") $stop; + if (getUnpacked($c("7")) != "7") $stop; + if (getUnpacked($c("8")) != "8") $stop; + if (getUnpacked($c("9")) != "9") $stop; + + if (getPacked($c("0")) != "0") $stop; + if (getPacked($c("1")) != "1") $stop; + if (getPacked($c("2")) != "2") $stop; + if (getPacked($c("3")) != "3") $stop; + if (getPacked($c("4")) != "4") $stop; + if (getPacked($c("5")) != "5") $stop; + if (getPacked($c("6")) != "6") $stop; + if (getPacked($c("7")) != "7") $stop; + if (getPacked($c("8")) != "8") $stop; + if (getPacked($c("9")) != "9") $stop; + + if (getString($c("0")) != "0") $stop; + if (getString($c("1")) != "1") $stop; + if (getString($c("2")) != "2") $stop; + if (getString($c("3")) != "3") $stop; + if (getString($c("4")) != "4") $stop; + if (getString($c("5")) != "5") $stop; + if (getString($c("6")) != "6") $stop; + if (getString($c("7")) != "7") $stop; + if (getString($c("8")) != "8") $stop; + if (getString($c("9")) != "9") $stop; + + if (getStruct($c("0")) != "0") $stop; + if (getStruct($c("1")) != "1") $stop; + if (getStruct($c("2")) != "2") $stop; + if (getStruct($c("3")) != "3") $stop; + if (getStruct($c("4")) != "4") $stop; + if (getStruct($c("5")) != "5") $stop; + if (getStruct($c("6")) != "6") $stop; + if (getStruct($c("7")) != "7") $stop; + if (getStruct($c("8")) != "8") $stop; + if (getStruct($c("9")) != "9") $stop; + + if (getType($c("0")) != "0") $stop; + if (getType($c("1")) != "1") $stop; + if (getType($c("2")) != "2") $stop; + if (getType($c("3")) != "3") $stop; + if (getType($c("4")) != "4") $stop; + if (getType($c("5")) != "5") $stop; + if (getType($c("6")) != "6") $stop; + if (getType($c("7")) != "7") $stop; + if (getType($c("8")) != "8") $stop; + if (getType($c("9")) != "9") $stop; + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule + +function automatic logic [7:0] getUnpacked(logic[3:0] d); +`ifdef NO_INLINE + /* verilator no_inline_task */ +`endif + localparam logic [7:0] digits [10] = + '{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9"}; + return digits[d]; +endfunction + +function automatic logic [7:0] getPacked(logic[3:0] d); +`ifdef NO_INLINE + /* verilator no_inline_task */ +`endif + localparam logic [9:0][7:0] digits = + {"9", "8", "7", "6", "5", "4", "3", "2", "1", "0"}; + return digits[d]; +endfunction + +function automatic string getString(logic[3:0] d); +`ifdef NO_INLINE + /* verilator no_inline_task */ +`endif + localparam string digits [10] = + '{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9"}; + return digits[d]; +endfunction + +function automatic logic [7:0] getStruct(logic[3:0] d); +`ifdef NO_INLINE + /* verilator no_inline_task */ +`endif + // Silly indirect lookup table because we want to use a struct + typedef struct packed { + logic [7:0] result; + longint index; + } lut_t; + localparam lut_t digits [10] = + '{ + '{result: "1", index: 9}, + '{result: "2", index: 0}, + '{result: "3", index: 1}, + '{result: "4", index: 2}, + '{result: "5", index: 3}, + '{result: "6", index: 4}, + '{result: "7", index: 5}, + '{result: "8", index: 6}, + '{result: "9", index: 7}, + '{result: "0", index: 8} + }; + return digits[4'(digits[d].index)].result; +endfunction + +function automatic logic [7:0] getType(logic[3:0] d); +`ifdef NO_INLINE + /* verilator no_inline_task */ +`endif + localparam type octet_t = logic [7:0]; + localparam octet_t [9:0] digits = + {"9", "8", "7", "6", "5", "4", "3", "2", "1", "0"}; + return digits[d]; +endfunction + diff --git a/test_regress/t/t_param_in_func_bad.out b/test_regress/t/t_param_in_func_bad.out deleted file mode 100644 index 73820ed23..000000000 --- a/test_regress/t/t_param_in_func_bad.out +++ /dev/null @@ -1,6 +0,0 @@ -%Error-UNSUPPORTED: t/t_param_in_func_bad.v:24:26: Unsupported: Parameters in functions with complex assign - : ... In instance t - 24 | localparam logic[7:0] digits[10] - | ^~~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error: Exiting due to diff --git a/test_regress/t/t_param_in_func_bad.pl b/test_regress/t/t_param_in_func_bad.pl deleted file mode 100755 index 27159da5b..000000000 --- a/test_regress/t/t_param_in_func_bad.pl +++ /dev/null @@ -1,19 +0,0 @@ -#!/usr/bin/env perl -if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } -# DESCRIPTION: Verilator: Verilog Test driver/expect definition -# -# Copyright 2019 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(linter => 1); - -lint( - fails => 1, - expect_filename => $Self->{golden_filename}, - ); - -ok(1); -1; diff --git a/test_regress/t/t_param_in_func_bad.v b/test_regress/t/t_param_in_func_bad.v deleted file mode 100644 index 02a451f42..000000000 --- a/test_regress/t/t_param_in_func_bad.v +++ /dev/null @@ -1,29 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed into the Public Domain, for any use, -// without warranty, 2019 by Driss Hafdi. -// SPDX-License-Identifier: CC0-1.0 - -module t (/*AUTOARG*/ - // Inputs - clk - ); - - input clk; - - logic [7:0] digit = getDigit(4'd1); - - initial begin - if (digit != "1") $stop; - $write("*-* All Finished *-*\n"); - $finish; - end -endmodule - -function automatic logic[7:0] getDigit(logic[3:0] d); - localparam logic[7:0] digits[10] - = '{ - "0", "1", "2", "3", "4", "5", "6", "7", "8", "9" - }; - return digits[d]; -endfunction diff --git a/test_regress/t/t_param_in_func_noinline.pl b/test_regress/t/t_param_in_func_noinline.pl new file mode 100755 index 000000000..c089b8b2e --- /dev/null +++ b/test_regress/t/t_param_in_func_noinline.pl @@ -0,0 +1,35 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2019 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); + +top_filename("t/t_param_in_func.v"); + +compile( + verilator_flags2 => ["--stats", "+define+NO_INLINE=1"], + ); + +execute(check_finished => 1); + +# The parameter array should have been put in the constant pool +if ($Self->{vlt_all}) { + file_grep($Self->{stats}, qr/ConstPool, Tables emitted\s+(\d+)/i, 3); +} + +# Shouldn't have any references to the parameter array +foreach my $file ( + glob_all("$Self->{obj_dir}/$Self->{VM_PREFIX}*.h"), + glob_all("$Self->{obj_dir}/$Self->{VM_PREFIX}*.cpp") + ) { + file_grep_not($file, qr/digits/i); +} + +ok(1); +1;