diff --git a/Changes b/Changes index 18d9f6ec3..e262b8264 100644 --- a/Changes +++ b/Changes @@ -18,6 +18,7 @@ Verilator 5.007 devel * Add WIDTHEXPAND and WIDTHTRUNC warnings to replace WIDTH (#3900). [Andrew Nolte] * Add SOURCE_DATE_EPOCH for docs/guide/conf.py (#3918). [Larry Doolittle] * Add /*verilator public[flat|flat_rd|flat_rw| ]*/ metacomments (#3894). [Joseph Nwabueze] +* Add error on mixing .name and by-port instantiations. * Support unpacked unions. * Support interface classes and class implements. * Support global clocking and $global_clock. diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 2d7b74596..fe0594267 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1233,7 +1233,8 @@ private: AstVar* m_modVarp = nullptr; // Input/output this pin connects to on submodule. AstParamTypeDType* m_modPTypep = nullptr; // Param type this pin connects to on submodule. bool m_param = false; // Pin connects to parameter - bool m_svImplicit = false; // Pin is SystemVerilog .name'ed + bool m_svDotName = false; // Pin is SystemVerilog .name'ed + bool m_svImplicit = false; // Pin is SystemVerilog .name'ed, allow implicit public: AstPin(FileLine* fl, int pinNum, const string& name, AstNode* exprp) : ASTGEN_SUPER_Pin(fl) @@ -1257,6 +1258,8 @@ public: void modPTypep(AstParamTypeDType* nodep) { m_modPTypep = nodep; } bool param() const { return m_param; } void param(bool flag) { m_param = flag; } + bool svDotName() const { return m_svDotName; } + void svDotName(bool flag) { m_svDotName = flag; } bool svImplicit() const { return m_svImplicit; } void svImplicit(bool flag) { m_svImplicit = flag; } }; diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index d44850903..135674029 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1724,6 +1724,7 @@ void AstPin::dump(std::ostream& str) const { } else { str << " ->UNLINKED"; } + if (svDotName()) str << " [.n]"; if (svImplicit()) str << " [.SV]"; } const char* AstPin::broken() const { diff --git a/src/V3LinkCells.cpp b/src/V3LinkCells.cpp index f6db23721..20f5e545c 100644 --- a/src/V3LinkCells.cpp +++ b/src/V3LinkCells.cpp @@ -352,8 +352,10 @@ private: } // Convert .* to list of pins bool pinStar = false; + bool pinDotName = false; for (AstPin *nextp, *pinp = nodep->pinsp(); pinp; pinp = nextp) { nextp = VN_AS(pinp->nextp(), Pin); + if (pinp->svDotName()) pinDotName = true; if (pinp->dotStar()) { if (pinStar) pinp->v3error("Duplicate .* in an instance (IEEE 1800-2017 23.3.2)"); pinStar = true; @@ -374,8 +376,8 @@ private: // Note what pins exist std::unordered_set ports; // Symbol table of all connected port names for (AstPin* pinp = nodep->pinsp(); pinp; pinp = VN_AS(pinp->nextp(), Pin)) { - if (pinStar && pinp->name().substr(0, 11) == "__pinNumber") { - pinp->v3error("Connect by position is illegal in .* connected instances" + if ((pinStar || pinDotName) && pinp->name().substr(0, 11) == "__pinNumber") { + pinp->v3error("Mixing positional and .*/named instantiation connection" " (IEEE 1800-2017 23.3.2)"); } if (!pinp->exprp()) { @@ -404,6 +406,7 @@ private: nodep->fileline(), 0, portp->name(), new AstParseRef{nodep->fileline(), VParseRefExp::PX_TEXT, portp->name(), nullptr, nullptr}}; + newp->svDotName(true); newp->svImplicit(true); nodep->addPinsp(newp); } else { // warn on the CELL that needs it, not the port diff --git a/src/verilog.y b/src/verilog.y index ce1ad406a..cfc1642a5 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -3194,18 +3194,22 @@ cellparamItemE: // IEEE: named_parameter_assignment + empty // // Note empty can match either () or (,); V3LinkCells cleans up () /* empty: ',,' is legal */ { $$ = new AstPin{CRELINE(), PINNUMINC(), "", nullptr}; } | yP_DOTSTAR { $$ = new AstPin{$1, PINNUMINC(), ".*", nullptr}; } - | '.' idAny '(' ')' { $$ = new AstPin{$2, PINNUMINC(), *$2, nullptr}; } + | '.' idAny '(' ')' + { $$ = new AstPin{$2, PINNUMINC(), *$2, nullptr}; + $$->svDotName(true); } | '.' idSVKwd { $$ = new AstPin{$2, PINNUMINC(), *$2, new AstParseRef{$2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}}; - $$->svImplicit(true); } + $$->svDotName(true); $$->svImplicit(true); } | '.' idAny { $$ = new AstPin{$2, PINNUMINC(), *$2, new AstParseRef{$2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}}; - $$->svImplicit(true); } + $$->svDotName(true); $$->svImplicit(true); } // // mintypmax is expanded here, as it might be a UDP or gate primitive // // data_type for 'parameter type' hookups - | '.' idAny '(' exprOrDataType ')' { $$ = new AstPin{$2, PINNUMINC(), *$2, $4}; } + | '.' idAny '(' exprOrDataType ')' + { $$ = new AstPin{$2, PINNUMINC(), *$2, $4}; + $$->svDotName(true); } //UNSUP '.' idAny '(' exprOrDataType/*expr*/ ':' expr ')' { } //UNSUP '.' idAny '(' exprOrDataType/*expr*/ ':' expr ':' expr ')' { } // // data_type for 'parameter type' hookups @@ -3218,18 +3222,22 @@ cellpinItemE: // IEEE: named_port_connection + empty // // Note empty can match either () or (,); V3LinkCells cleans up () /* empty: ',,' is legal */ { $$ = new AstPin{CRELINE(), PINNUMINC(), "", nullptr}; } | yP_DOTSTAR { $$ = new AstPin{$1, PINNUMINC(), ".*", nullptr}; } - | '.' idAny '(' ')' { $$ = new AstPin{$2, PINNUMINC(), *$2, nullptr}; } + | '.' idAny '(' ')' + { $$ = new AstPin{$2, PINNUMINC(), *$2, nullptr}; + $$->svDotName(true); } | '.' idSVKwd { $$ = new AstPin{$2, PINNUMINC(), *$2, new AstParseRef{$2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}}; - $$->svImplicit(true);} + $$->svDotName(true); $$->svImplicit(true); } | '.' idAny { $$ = new AstPin{$2, PINNUMINC(), *$2, new AstParseRef{$2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}}; - $$->svImplicit(true);} + $$->svDotName(true); $$->svImplicit(true); } // // mintypmax is expanded here, as it might be a UDP or gate primitive //UNSUP pev_expr below - | '.' idAny '(' expr ')' { $$ = new AstPin{$2, PINNUMINC(), *$2, $4}; } + | '.' idAny '(' expr ')' + { $$ = new AstPin{$2, PINNUMINC(), *$2, $4}; + $$->svDotName(true); } //UNSUP '.' idAny '(' pev_expr ':' expr ')' { } //UNSUP '.' idAny '(' pev_expr ':' expr ':' expr ')' { } // diff --git a/test_regress/t/t_fuzz_genintf_bad.out b/test_regress/t/t_fuzz_genintf_bad.out index 407d8e478..780ca81cb 100644 --- a/test_regress/t/t_fuzz_genintf_bad.out +++ b/test_regress/t/t_fuzz_genintf_bad.out @@ -1,10 +1,4 @@ -%Error-UNSUPPORTED: t/t_fuzz_genintf_bad.v:24:12: Unsupported: Member call on object 'VARREF 'j'' which is a 'BASICDTYPE 'integer'' - : ... In instance t - 24 | j.e(0), - | ^ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error: Internal Error: t/t_fuzz_genintf_bad.v:24:11: ../V3Width.cpp:#: Unlinked pin data type - : ... In instance t +%Error: t/t_fuzz_genintf_bad.v:24:11: Mixing positional and .*/named instantiation connection (IEEE 1800-2017 23.3.2) 24 | j.e(0), | ^ - ... See the manual at https://verilator.org/verilator_doc.html for more assistance. +%Error: Exiting due to diff --git a/test_regress/t/t_inst_2star_bad.out b/test_regress/t/t_inst_2star_bad.out index fc7cc9140..b4d6f2922 100644 --- a/test_regress/t/t_inst_2star_bad.out +++ b/test_regress/t/t_inst_2star_bad.out @@ -1,7 +1,10 @@ -%Error: t/t_inst_2star_bad.v:11:17: Duplicate .* in an instance (IEEE 1800-2017 23.3.2) - 11 | sub sub (.*, .*); +%Error: t/t_inst_2star_bad.v:12:17: Duplicate .* in an instance (IEEE 1800-2017 23.3.2) + 12 | sub sub (.*, .*); | ^~ -%Error: t/t_inst_2star_bad.v:13:13: Connect by position is illegal in .* connected instances (IEEE 1800-2017 23.3.2) - 13 | sub sub (foo, .*); +%Error: t/t_inst_2star_bad.v:14:13: Mixing positional and .*/named instantiation connection (IEEE 1800-2017 23.3.2) + 14 | sub sub (foo, .*); + | ^~~ +%Error: t/t_inst_2star_bad.v:16:13: Mixing positional and .*/named instantiation connection (IEEE 1800-2017 23.3.2) + 16 | sub sub (foo, .bar); | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_inst_2star_bad.v b/test_regress/t/t_inst_2star_bad.v index 5c91aaac5..74e527c9d 100644 --- a/test_regress/t/t_inst_2star_bad.v +++ b/test_regress/t/t_inst_2star_bad.v @@ -7,12 +7,15 @@ module t (/*AUTOARG*/); wire foo; + wire bar; sub sub (.*, .*); sub sub (foo, .*); + sub sub (foo, .bar); + endmodule -module sub (input foo); +module sub (input foo, input bar); endmodule diff --git a/test_regress/t/t_inst_missing.v b/test_regress/t/t_inst_missing.v index 69ecc51fe..081be8f5d 100644 --- a/test_regress/t/t_inst_missing.v +++ b/test_regress/t/t_inst_missing.v @@ -6,9 +6,13 @@ module t (/*AUTOARG*/); wire ok = 1'b0; + // verilator lint_off UNDRIVEN + wire nc; + // verilator lint_on UNDRIVEN + // verilator lint_off PINNOCONNECT // verilator lint_off PINCONNECTEMPTY - sub sub (.ok(ok), , .nc()); + sub sub (ok, , nc); // verilator lint_on PINCONNECTEMPTY // verilator lint_on PINNOCONNECT endmodule diff --git a/test_regress/t/t_inst_missing_bad.out b/test_regress/t/t_inst_missing_bad.out index 8f676d048..c716cf0a5 100644 --- a/test_regress/t/t_inst_missing_bad.out +++ b/test_regress/t/t_inst_missing_bad.out @@ -1,12 +1,9 @@ -%Warning-PINNOCONNECT: t/t_inst_missing_bad.v:9:22: Cell pin is not connected: '__pinNumber2' - 9 | sub sub (.ok(ok), , .nc()); - | ^ +%Warning-PINNOCONNECT: t/t_inst_missing_bad.v:13:17: Cell pin is not connected: '__pinNumber2' + 13 | sub sub (ok, , nc); + | ^ ... For warning description see https://verilator.org/warn/PINNOCONNECT?v=latest ... Use "/* verilator lint_off PINNOCONNECT */" and lint_on around source to disable this message. -%Warning-PINCONNECTEMPTY: t/t_inst_missing_bad.v:9:25: Cell pin connected by name with empty reference: 'nc' - 9 | sub sub (.ok(ok), , .nc()); - | ^~ -%Warning-PINMISSING: t/t_inst_missing_bad.v:9:8: Cell has missing pin: 'missing' - 9 | sub sub (.ok(ok), , .nc()); +%Warning-PINMISSING: t/t_inst_missing_bad.v:13:8: Cell has missing pin: 'missing' + 13 | sub sub (ok, , nc); | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_inst_missing_bad.v b/test_regress/t/t_inst_missing_bad.v index 73f782433..2d0c703fe 100644 --- a/test_regress/t/t_inst_missing_bad.v +++ b/test_regress/t/t_inst_missing_bad.v @@ -6,7 +6,11 @@ module t (/*AUTOARG*/); wire ok = 1'b0; - sub sub (.ok(ok), , .nc()); + // verilator lint_off UNDRIVEN + wire nc; + // verilator lint_on UNDRIVEN + + sub sub (ok, , nc); endmodule module sub (input ok, input none, input nc, input missing); diff --git a/test_regress/t/t_interface_modportlist.v b/test_regress/t/t_interface_modportlist.v index c72fa490f..7391bfcb0 100644 --- a/test_regress/t/t_interface_modportlist.v +++ b/test_regress/t/t_interface_modportlist.v @@ -8,7 +8,7 @@ module t(input clk); my_interface iface(); - my_module m(.clk(clk), iface); + my_module m(.clk(clk), .iface); endmodule module my_module(input clk, my_interface.my_port iface);