diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index a6b8aa63f..2908bbe95 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -234,6 +234,7 @@ private: bool m_inLibrary : 1; // From a library, no error if not used, never top level bool m_dead : 1; // LinkDot believes is dead; will remove in Dead visitors bool m_hasGParam : 1; // Has global parameter (for link) + bool m_hasParameterList : 1; // Has #() for parameter declaration bool m_hierBlock : 1; // Hierarchical Block marked by HIER_BLOCK pragma bool m_internal : 1; // Internally created bool m_recursive : 1; // Recursive module @@ -248,6 +249,7 @@ protected: , m_inLibrary{false} , m_dead{false} , m_hasGParam{false} + , m_hasParameterList{false} , m_hierBlock{false} , m_internal{false} , m_recursive{false} @@ -277,6 +279,8 @@ public: bool dead() const { return m_dead; } void hasGParam(bool flag) { m_hasGParam = flag; } bool hasGParam() const { return m_hasGParam; } + void hasParameterList(bool flag) { m_hasParameterList = flag; } + bool hasParameterList() const { return m_hasParameterList; } void hierBlock(bool flag) { m_hierBlock = flag; } bool hierBlock() const { return m_hierBlock; } void internal(bool flag) { m_internal = flag; } @@ -1660,7 +1664,7 @@ class AstVar final : public AstNode { VVarAttrClocker m_attrClocker; MTaskIdSet m_mtaskIds; // MTaskID's that read or write this var int m_pinNum = 0; // For XML, if non-zero the connection pin number - bool m_ansi : 1; // ANSI port list variable (for dedup check) + bool m_ansi : 1; // Params or pins declared in the module header, rather than the body bool m_declTyped : 1; // Declared as type (for dedup check) bool m_tristate : 1; // Inout or triwire or trireg bool m_primaryIO : 1; // In/out to top level (or directly assigned from same) diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp index 48a5cf0e4..5b863df9c 100644 --- a/src/V3LinkParse.cpp +++ b/src/V3LinkParse.cpp @@ -66,6 +66,7 @@ private: AstNodeExpr* m_defaultOutSkewp = nullptr; // Current default output skew int m_genblkAbove = 0; // Begin block number of if/case/for above int m_genblkNum = 0; // Begin block number, 0=none seen + int m_beginDepth = 0; // How many begin blocks above current node within current AstNodeModule VLifetime m_lifetime = VLifetime::STATIC; // Propagating lifetime bool m_insideLoop = false; // True if the node is inside a loop @@ -312,7 +313,17 @@ private: if (nodep->lifetime().isNone() && nodep->varType() != VVarType::PORT) { nodep->lifetime(m_lifetime); } + + if (nodep->isGParam() && !nodep->isAnsi()) { // shadow some parameters into localparams + if (m_beginDepth > 0 + || (m_beginDepth == 0 + && (m_modp->hasParameterList() || VN_IS(m_modp, Class) + || VN_IS(m_modp, Package)))) { + nodep->varType(VVarType::LPARAM); + } + } if (nodep->isGParam() && m_modp) m_modp->hasGParam(true); + if (nodep->isParam() && !nodep->valuep() && nodep->fileline()->language() < V3LangCode::L1800_2009) { nodep->v3warn(NEWERSTD, @@ -619,6 +630,7 @@ private: VL_RESTORER(m_modp); VL_RESTORER(m_genblkAbove); VL_RESTORER(m_genblkNum); + VL_RESTORER(m_beginDepth); VL_RESTORER(m_lifetime); { // Module: Create sim table for entire module and iterate @@ -628,6 +640,7 @@ private: m_modp = nodep; m_genblkAbove = 0; m_genblkNum = 0; + m_beginDepth = 0; m_valueModp = nodep; m_lifetime = nodep->lifetime(); if (m_lifetime.isNone()) { @@ -658,6 +671,8 @@ private: void visit(AstBegin* nodep) override { V3Config::applyCoverageBlock(m_modp, nodep); cleanFileline(nodep); + VL_RESTORER(m_beginDepth); + m_beginDepth++; const AstNode* const backp = nodep->backp(); // IEEE says directly nested item is not a new block // The genblk name will get attached to the if true/false LOWER begin block(s) diff --git a/src/V3ParseSym.h b/src/V3ParseSym.h index c29db09d3..32b817e19 100644 --- a/src/V3ParseSym.h +++ b/src/V3ParseSym.h @@ -130,6 +130,13 @@ public: UASSERT_OBJ(!m_sympStack.empty(), nodep, "symbol stack underflow"); m_symCurrentp = m_sympStack.back(); } + AstNodeModule* findTopNodeModule(FileLine* fl, bool requireNoneNull = true) { + for (VSymEnt* const symp : vlstd::reverse_view(m_sympStack)) { + if (AstNodeModule* const modp = VN_CAST(symp->nodep(), NodeModule)) return modp; + } + if (requireNoneNull) fl->v3fatalSrc("fail to find current module"); + return nullptr; + } void showUpward() { // LCOV_EXCL_START UINFO(1, "ParseSym Stack:\n"); for (VSymEnt* const symp : vlstd::reverse_view(m_sympStack)) { diff --git a/src/V3SplitVar.cpp b/src/V3SplitVar.cpp index 34779a573..e940918b8 100644 --- a/src/V3SplitVar.cpp +++ b/src/V3SplitVar.cpp @@ -719,7 +719,6 @@ class SplitUnpackedVarVisitor final : public VNVisitor, public SplitVarImpl { adtypep = VN_AS(refp->dtypep()->skipRefp(), UnpackArrayDType); } else { AstSliceSel* const selp = VN_AS(ref.nodep(), SliceSel); - UASSERT_OBJ(selp, ref.nodep(), "Unexpected op is registered"); refp = VN_AS(selp->fromp(), VarRef); UASSERT_OBJ(refp, selp, "Unexpected op is registered"); adtypep = VN_AS(selp->dtypep()->skipRefp(), UnpackArrayDType); diff --git a/src/verilog.y b/src/verilog.y index 134494488..2b9120969 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -84,7 +84,6 @@ public: AstNodeDType* m_memDTypep = nullptr; // Pointer to data type for next member declaration AstDelay* m_netDelayp = nullptr; // Pointer to delay for next signal declaration AstStrengthSpec* m_netStrengthp = nullptr; // Pointer to strength for next net declaration - AstNodeModule* m_modp = nullptr; // Last module for timeunits FileLine* m_instModuleFl = nullptr; // Fileline of module referenced for instantiations AstPin* m_instParamp = nullptr; // Parameters for instantiations string m_instModule; // Name of module referenced for instantiations @@ -93,10 +92,9 @@ public: VLifetime m_varLifetime; // Static/Automatic for next signal bool m_impliedDecl = false; // Allow implied wire declarations bool m_varDeclTyped = false; // Var got reg/wire for dedup check - bool m_pinAnsi = false; // In ANSI port list + bool m_pinAnsi = false; // In ANSI parameter or port list bool m_tracingParse = true; // Tracing disable for parser bool m_inImplements = false; // Is inside class implements list - bool m_insideClass = false; // Is inside a class body bool m_insideProperty = false; // Is inside property declaration bool m_typedPropertyPort = false; // Typed property port occurred on port lists bool m_modportImpExpActive @@ -317,14 +315,12 @@ int V3ParseGrammar::s_modTypeImpNum = 0; #define VARRESET_LIST(decl) \ { \ GRAMMARP->m_pinNum = 1; \ - GRAMMARP->m_pinAnsi = false; \ VARRESET(); \ VARDECL(decl); \ } // Start of pinlist #define VARRESET_NONLIST(decl) \ { \ GRAMMARP->m_pinNum = 0; \ - GRAMMARP->m_pinAnsi = false; \ VARRESET(); \ VARDECL(decl); \ } // Not in a pinlist @@ -1209,11 +1205,14 @@ description: // ==IEEE: description timeunits_declaration: // ==IEEE: timeunits_declaration yTIMEUNIT yaTIMENUM ';' - { PARSEP->timescaleMod($2, GRAMMARP->m_modp, true, $2, false, 0); $$ = nullptr; } + { PARSEP->timescaleMod($2, SYMP->findTopNodeModule($1, false), true, $2, false, 0); + $$ = nullptr; } | yTIMEUNIT yaTIMENUM '/' yaTIMENUM ';' - { PARSEP->timescaleMod($2, GRAMMARP->m_modp, true, $2, true, $4); $$ = nullptr; } + { PARSEP->timescaleMod($2, SYMP->findTopNodeModule($1, false), true, $2, true, $4); + $$ = nullptr; } | yTIMEPRECISION yaTIMENUM ';' - { PARSEP->timescaleMod($2, GRAMMARP->m_modp, false, 0, true, $2); $$ = nullptr; } + { PARSEP->timescaleMod($2, SYMP->findTopNodeModule($1, false), false, 0, true, $2); + $$ = nullptr; } ; //********************************************************************** @@ -1223,7 +1222,6 @@ package_declaration: // ==IEEE: package_declaration packageFront package_itemListE yENDPACKAGE endLabelE { $1->modTrace(GRAMMARP->allTracingOn($1->fileline())); // Stash for implicit wires, etc if ($2) $1->addStmtsp($2); - GRAMMARP->m_modp = nullptr; SYMP->popScope($1); GRAMMARP->endLabel($4, $1, $4); } ; @@ -1243,8 +1241,7 @@ packageFront: $$->modTrace(GRAMMARP->allTracingOn($$->fileline())); $$->timeunit(PARSEP->timeLastUnit()); PARSEP->rootp()->addModulesp($$); - SYMP->pushNew($$); - GRAMMARP->m_modp = $$; } + SYMP->pushNew($$); } ; package_itemListE: // IEEE: [{ package_item }] @@ -1343,7 +1340,6 @@ module_declaration: // ==IEEE: module_declaration if ($2) $1->addStmtsp($2); if ($3) $1->addStmtsp($3); if ($5) $1->addStmtsp($5); - GRAMMARP->m_modp = nullptr; SYMP->popScope($1); GRAMMARP->endLabel($7, $1, $7); } | udpFront portsStarE ';' @@ -1352,7 +1348,6 @@ module_declaration: // ==IEEE: module_declaration if ($2) $1->addStmtsp($2); if ($4) $1->addStmtsp($4); GRAMMARP->m_tracingParse = true; - GRAMMARP->m_modp = nullptr; SYMP->popScope($1); GRAMMARP->endLabel($6, $1, $6); } // @@ -1371,8 +1366,7 @@ modFront: $$->timeunit(PARSEP->timeLastUnit()); $$->unconnectedDrive(PARSEP->unconnectedDrive()); PARSEP->rootp()->addModulesp($$); - SYMP->pushNew($$); - GRAMMARP->m_modp = $$; } + SYMP->pushNew($$); } | modFront sigAttrScope { $$ = $1; } ; @@ -1419,13 +1413,18 @@ parameter_value_assignmentClass: // IEEE: [ parameter_value_assignment ] parameter_port_listE: // IEEE: parameter_port_list + empty == parameter_value_assignment /* empty */ { $$ = nullptr; } - | '#' '(' ')' { $$ = nullptr; } + | '#' '(' ')' { $$ = nullptr; + SYMP->findTopNodeModule($1)->hasParameterList(true); } // // IEEE: '#' '(' list_of_param_assignments { ',' parameter_port_declaration } ')' // // IEEE: '#' '(' parameter_port_declaration { ',' parameter_port_declaration } ')' // // Can't just do that as "," conflicts with between vars and between stmts, so // // split into pre-comma and post-comma parts - | '#' '(' {VARRESET_LIST(GPARAM);} paramPortDeclOrArgList ')' - { $$ = $4; VARRESET_NONLIST(UNKNOWN); } + | '#' '(' { VARRESET_LIST(GPARAM); + SYMP->findTopNodeModule($1)->hasParameterList(true); + GRAMMARP->m_pinAnsi = true; } + /*cont*/ paramPortDeclOrArgList ')' { $$ = $4; + VARRESET_NONLIST(UNKNOWN); + GRAMMARP->m_pinAnsi = false; } // // Note legal to start with "a=b" with no parameter statement ; @@ -1451,7 +1450,10 @@ portsStarE: // IEEE: .* + list_of_ports + list_of_port_decla | '(' ')' { $$ = nullptr; } // // .* expanded from module_declaration //UNSUP '(' yP_DOTSTAR ')' { UNSUP } - | '(' {VARRESET_LIST(PORT);} list_of_ports ')' { $$ = $3; VARRESET_NONLIST(UNKNOWN); } + | '(' { VARRESET_LIST(PORT); + GRAMMARP->m_pinAnsi = true; } + /*cont*/ list_of_ports ')' { $$ = $3; VARRESET_NONLIST(UNKNOWN); + GRAMMARP->m_pinAnsi = false; } ; list_of_portsE: // IEEE: list_of_ports + list_of_port_declarations @@ -1596,7 +1598,7 @@ portDirNetE: // IEEE: part of port, optional net type and/or // // The higher level rule may override this VARDTYPE with one later in the parse. | port_direction { VARDECL(PORT); VARDTYPE_NDECL(nullptr); } | port_direction { VARDECL(PORT); } net_type { VARDTYPE_NDECL(nullptr); } // net_type calls VARDECL - | net_type { VARDTYPE_NDECL(nullptr); } // net_type calls VARDECL + | net_type { VARDTYPE_NDECL(nullptr); } // net_type calls VARDECL ; port_declNetE: // IEEE: part of port_declaration, optional net type @@ -1706,7 +1708,6 @@ program_declaration: // IEEE: program_declaration + program_nonansi_h if ($2) $1->addStmtsp($2); if ($3) $1->addStmtsp($3); if ($5) $1->addStmtsp($5); - GRAMMARP->m_modp = nullptr; SYMP->popScope($1); GRAMMARP->endLabel($7, $1, $7); } | yEXTERN pgmFront parameter_port_listE portsStarE ';' @@ -1722,8 +1723,7 @@ pgmFront: $$->modTrace(GRAMMARP->allTracingOn($$->fileline())); $$->timeunit(PARSEP->timeLastUnit()); PARSEP->rootp()->addModulesp($$); - SYMP->pushNew($$); - GRAMMARP->m_modp = $$; } + SYMP->pushNew($$); } ; program_itemListE: // ==IEEE: [{ program_item }] @@ -1958,23 +1958,19 @@ net_type: // ==IEEE: net_type ; varParamReset: - yPARAMETER - { if (GRAMMARP->m_insideClass) { - VARRESET_NONLIST(LPARAM); - } else { - VARRESET_NONLIST(GPARAM); - } } + yPARAMETER { VARRESET_NONLIST(GPARAM); } | yLOCALPARAM { VARRESET_NONLIST(LPARAM); } + // Note that the type of these params could be modified later according to context (see V3LinkParse) ; port_direction: // ==IEEE: port_direction + tf_port_direction // // IEEE 19.8 just "input" FIRST forces type to wire - we'll ignore that here // // Only used for ANSI declarations - yINPUT { GRAMMARP->m_pinAnsi = true; VARIO(INPUT); } - | yOUTPUT { GRAMMARP->m_pinAnsi = true; VARIO(OUTPUT); } - | yINOUT { GRAMMARP->m_pinAnsi = true; VARIO(INOUT); } - | yREF { GRAMMARP->m_pinAnsi = true; VARIO(REF); } - | yCONST__REF yREF { GRAMMARP->m_pinAnsi = true; VARIO(CONSTREF); } + yINPUT { VARIO(INPUT); } + | yOUTPUT { VARIO(OUTPUT); } + | yINOUT { VARIO(INOUT); } + | yREF { VARIO(REF); } + | yCONST__REF yREF { VARIO(CONSTREF); } ; port_directionReset: // IEEE: port_direction that starts a port_declaraiton @@ -5960,12 +5956,12 @@ property_port_itemAssignment: // IEEE: part of property_port_item/sequen property_port_itemDirE: /* empty */ - { GRAMMARP->m_pinAnsi = true; VARIO(INPUT); } + { VARIO(INPUT); } | yLOCAL__ETC - { GRAMMARP->m_pinAnsi = true; VARIO(INPUT); + { VARIO(INPUT); BBUNSUP($1, "Unsupported: property port 'local'"); } | yLOCAL__ETC yINPUT - { GRAMMARP->m_pinAnsi = true; VARIO(INPUT); + { VARIO(INPUT); BBUNSUP($1, "Unsupported: property port 'local'"); } ; @@ -6743,7 +6739,6 @@ checker_declaration: // ==IEEE: part of checker_declaration $1->modTrace(GRAMMARP->allTracingOn($1->fileline())); if ($2) $1->addStmtsp($2); if ($4) $1->addStmtsp($4); - GRAMMARP->m_modp = nullptr; SYMP->popScope($1); GRAMMARP->endLabel($6, $1, $6); } ; @@ -6756,8 +6751,7 @@ checkerFront: // IEEE: part of checker_declaration $$->modTrace(GRAMMARP->allTracingOn($$->fileline())); $$->timeunit(PARSEP->timeLastUnit()); $$->unconnectedDrive(PARSEP->unconnectedDrive()); - SYMP->pushNew($$); - GRAMMARP->m_modp = $$; } + SYMP->pushNew($$); } ; checker_port_listE: // IEEE: [ ( [ checker_port_list ] ) ] @@ -6833,7 +6827,6 @@ class_declaration: // ==IEEE: part of class_declaration classFront parameter_port_listE classExtendsE classImplementsE ';' /*mid*/ { // Allow resolving types declared in base extends class if ($3) SYMP->importExtends($3); - GRAMMARP->m_insideClass = true; } /*cont*/ class_itemListE yENDCLASS endLabelE { $$ = $1; $1->addMembersp($2); @@ -6842,7 +6835,6 @@ class_declaration: // ==IEEE: part of class_declaration $1->addExtendsp($4); $1->addMembersp($7); SYMP->popScope($$); - GRAMMARP->m_insideClass = false; GRAMMARP->endLabel($9, $1, $9); } ; diff --git a/test_regress/t/t_param_implicit_local_bad.out b/test_regress/t/t_param_implicit_local_bad.out new file mode 100644 index 000000000..ef6479300 --- /dev/null +++ b/test_regress/t/t_param_implicit_local_bad.out @@ -0,0 +1,23 @@ +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:15:20: Parameter pin not found: '__paramNumber2' + 15 | NestedCls #(1, 2) cls; + | ^ + ... For error description see https://verilator.org/warn/PINNOTFOUND?v=latest +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:17:21: Parameter pin not found: '__paramNumber3' + 17 | mod1 # ( 3, 4, 5 ) i_mod1 (); + | ^ +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:15: Parameter pin not found: '__paramNumber1' + 19 | mod3 # ( 7, 24, 25 ) i_mod3 (); + | ^ +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:18: Parameter pin not found: '__paramNumber2' + 19 | mod3 # ( 7, 24, 25 ) i_mod3 (); + | ^~ +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:22: Parameter pin not found: '__paramNumber3' + 19 | mod3 # ( 7, 24, 25 ) i_mod3 (); + | ^~ +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:20:22: Parameter pin not found: '__paramNumber3' + 20 | intf1 # ( 8, 15, 17 ) i_intf1 (); + | ^~ +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:21:22: Parameter pin not found: '__paramNumber3' + 21 | prgm1 # ( 9, 40, 41 ) i_prgm1 (); + | ^~ +%Error: Exiting due to diff --git a/test_regress/t/t_param_implicit_local_bad.pl b/test_regress/t/t_param_implicit_local_bad.pl new file mode 100755 index 000000000..a60503a1f --- /dev/null +++ b/test_regress/t/t_param_implicit_local_bad.pl @@ -0,0 +1,19 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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_implicit_local_bad.v b/test_regress/t/t_param_implicit_local_bad.v new file mode 100644 index 000000000..a6252f77d --- /dev/null +++ b/test_regress/t/t_param_implicit_local_bad.v @@ -0,0 +1,55 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2023 by Anthony Donlon. +// SPDX-License-Identifier: CC0-1.0 + + +module t; + class NestedCls #( + parameter A = 0 + ); + parameter B = 0; + endclass + + NestedCls #(1, 2) cls; + + mod1 # ( 3, 4, 5 ) i_mod1 (); + mod2 # ( 5, 12, 13 ) i_mod2 (); + mod3 # ( 7, 24, 25 ) i_mod3 (); + intf1 # ( 8, 15, 17 ) i_intf1 (); + prgm1 # ( 9, 40, 41 ) i_prgm1 (); +endmodule + +`define CHECK_PARAMS if (A**2 + B**2 != C**2) $error("A**2 + B**2 != C**2") + +module mod1 # ( + parameter A = 1, B = 1 +); + parameter C = 1; + `CHECK_PARAMS; +endmodule + +module mod2 (); + parameter A = 1, B = 1, C = 1; + `CHECK_PARAMS; +endmodule + +module mod3 #() (); + parameter A = 1, B = 1, C = 1; + `CHECK_PARAMS; +endmodule + +interface intf1 # ( + parameter A = 1, B = 1 +); + parameter C = 1; + `CHECK_PARAMS; +endinterface + +program prgm1 # ( + parameter A = 1, B = 1 +); + parameter C = 1; + `CHECK_PARAMS; +endprogram diff --git a/test_regress/t/t_param_named.v b/test_regress/t/t_param_named.v index 633b6a220..523b65138 100644 --- a/test_regress/t/t_param_named.v +++ b/test_regress/t/t_param_named.v @@ -11,10 +11,7 @@ module t (/*AUTOARG*/ parameter PAR = 3; input clk; -`ifdef verilator - // Else it becomes a localparam, per IEEE 4.10.1, but we don't check it defparam m3.FROMDEFP = 19; -`endif m3 #(.P3(PAR), .P2(2)) @@ -36,7 +33,8 @@ module m3 parameter UNCH = 99, parameter P1 = 10, parameter P2 = 20, - P3 = 30 + P3 = 30, + parameter FROMDEFP = 11 ) (/*AUTOARG*/ // Inputs @@ -45,8 +43,6 @@ module m3 input clk; localparam LOC = 13; - parameter FROMDEFP = 11; - initial begin $display("%x %x %x",P1,P2,P3); end @@ -55,8 +51,6 @@ module m3 if (P1 !== 10) $stop; if (P2 !== 2) $stop; if (P3 !== 3) $stop; -`ifdef verilator if (FROMDEFP !== 19) $stop; -`endif end endmodule