diff --git a/src/V3Ast.h b/src/V3Ast.h index 8fc42582e..d49d60f1e 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -816,6 +816,7 @@ public: "BLOCKTEMP", "MODULETEMP", "STMTTEMP", "XTEMP", "IFACEREF", "MEMBER"}; return names[m_e]; } + bool isParam() const { return m_e == GPARAM || m_e == LPARAM; } bool isSignal() const { return (m_e == WIRE || m_e == WREAL || m_e == IMPLICITWIRE || m_e == TRIWIRE || m_e == TRI0 || m_e == TRI1 || m_e == PORT || m_e == SUPPLY0 || m_e == SUPPLY1 || m_e == VAR); diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index fb3c31789..7e7199427 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1922,9 +1922,7 @@ public: bool isClassMember() const { return varType() == VVarType::MEMBER; } bool isStatementTemp() const { return (varType() == VVarType::STMTTEMP); } bool isXTemp() const { return (varType() == VVarType::XTEMP); } - bool isParam() const VL_MT_SAFE { - return (varType() == VVarType::LPARAM || varType() == VVarType::GPARAM); - } + bool isParam() const { return varType().isParam(); } bool isGParam() const { return (varType() == VVarType::GPARAM); } bool isGenVar() const { return (varType() == VVarType::GENVAR); } bool isBitLogic() const { diff --git a/src/V3EmitCImp.cpp b/src/V3EmitCImp.cpp index e11daab55..18f75acc4 100644 --- a/src/V3EmitCImp.cpp +++ b/src/V3EmitCImp.cpp @@ -677,7 +677,7 @@ class EmitCTrace final : EmitCFunc { string fstvt; // Doubles have special decoding properties, so must indicate if a double if (nodep->dtypep()->basicp()->isDouble()) { - if (vartype == VVarType::GPARAM || vartype == VVarType::LPARAM) { + if (vartype.isParam()) { fstvt = "FST_VT_VCD_REAL_PARAMETER"; } else { fstvt = "FST_VT_VCD_REAL"; diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 283e15206..8b81802aa 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1616,6 +1616,7 @@ private: AstPin* const pinp = new AstPin{nodep->fileline(), -1, // Pin# not relevant nodep->name(), exprp}; + pinp->param(true); cellp->addParamsp(pinp); } VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); @@ -2250,6 +2251,11 @@ private: if (classp->isInterfaceClass()) importImplementsClass(nodep, srcp, classp); if (!cextp->isImplements()) m_curSymp->importFromClass(m_statep->symsp(), srcp); } + bool checkPinRef(AstPin* pinp, VVarType refVarType) { + // In instantiations of modules/ifaces, we shouldn't connect port pins to submodule's + // parameters or vice versa + return pinp->param() == refVarType.isParam(); + } // VISITs void visit(AstNetlist* nodep) override { @@ -2344,7 +2350,32 @@ private: UASSERT_OBJ(m_pinSymp, nodep, "Pin not under instance?"); VSymEnt* const foundp = m_pinSymp->findIdFlat(nodep->name()); const char* const whatp = nodep->param() ? "parameter pin" : "pin"; - if (!foundp) { + bool pinCheckFail = false; + if (foundp) { + if (AstVar* const refp = VN_CAST(foundp->nodep(), Var)) { + if (!refp->isIO() && !refp->isParam() && !refp->isIfaceRef()) { + nodep->v3error(ucfirst(whatp) + << " is not an in/out/inout/param/interface: " + << nodep->prettyNameQ()); + } else if (!checkPinRef(nodep, refp->varType())) { + pinCheckFail = true; + } else { + nodep->modVarp(refp); + markAndCheckPinDup(nodep, refp, whatp); + } + } else if (AstParamTypeDType* const refp + = VN_CAST(foundp->nodep(), ParamTypeDType)) { + if (!checkPinRef(nodep, refp->varType())) { + pinCheckFail = true; + } else { + nodep->modPTypep(refp); + markAndCheckPinDup(nodep, refp, whatp); + } + } else { + nodep->v3error(ucfirst(whatp) << " not found: " << nodep->prettyNameQ()); + } + } + if (!foundp || pinCheckFail) { if (nodep->name() == "__paramNumber1" && m_cellp && VN_IS(m_cellp->modp(), Primitive)) { // Primitive parameter is really a delay we can just ignore @@ -2360,19 +2391,6 @@ private: ucfirst(whatp) << " not found: " << nodep->prettyNameQ() << '\n' << (suggest.empty() ? "" : nodep->warnMore() + suggest)); - } else if (AstVar* const refp = VN_CAST(foundp->nodep(), Var)) { - if (!refp->isIO() && !refp->isParam() && !refp->isIfaceRef()) { - nodep->v3error(ucfirst(whatp) << " is not an in/out/inout/param/interface: " - << nodep->prettyNameQ()); - } else { - nodep->modVarp(refp); - markAndCheckPinDup(nodep, refp, whatp); - } - } else if (AstParamTypeDType* const refp = VN_CAST(foundp->nodep(), ParamTypeDType)) { - nodep->modPTypep(refp); - markAndCheckPinDup(nodep, refp, whatp); - } else { - nodep->v3error(ucfirst(whatp) << " not found: " << nodep->prettyNameQ()); } } // Early return() above when deleted diff --git a/test_regress/t/t_inst_pin_place_bad.out b/test_regress/t/t_inst_pin_place_bad.out new file mode 100644 index 000000000..0d3f845a6 --- /dev/null +++ b/test_regress/t/t_inst_pin_place_bad.out @@ -0,0 +1,12 @@ +%Warning-PINMISSING: t/t_inst_pin_place_bad.v:21:7: Cell has missing pin: 'pin_1' + 21 | ) i_sub ( + | ^~~~~ + ... For warning description see https://verilator.org/warn/PINMISSING?v=latest + ... Use "/* verilator lint_off PINMISSING */" and lint_on around source to disable this message. +%Error-PINNOTFOUND: t/t_inst_pin_place_bad.v:22:10: Pin not found: 'PARAM_A' + 22 | .PARAM_A(1) + | ^~~~~~~ +%Error-PINNOTFOUND: t/t_inst_pin_place_bad.v:20:10: Parameter pin not found: 'pin_1' + 20 | .pin_1(1) + | ^~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_inst_pin_place_bad.pl b/test_regress/t/t_inst_pin_place_bad.pl new file mode 100755 index 000000000..376c2d2ee --- /dev/null +++ b/test_regress/t/t_inst_pin_place_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 2023 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_inst_pin_place_bad.v b/test_regress/t/t_inst_pin_place_bad.v new file mode 100644 index 000000000..cac3e4f4c --- /dev/null +++ b/test_regress/t/t_inst_pin_place_bad.v @@ -0,0 +1,24 @@ +// 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 sub # ( + parameter PARAM_A = 1, + parameter type PARAM_B = logic +) ( + input pin_1 +); +endmodule + +module t; + parameter type PARAM_B = string; + + sub #( + .PARAM_B(PARAM_B), + .pin_1(1) + ) i_sub ( + .PARAM_A(1) + ); +endmodule