From 60a637921c12d5f7c56af40072b1bb69fa157580 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 26 Apr 2012 18:43:12 -0400 Subject: [PATCH] Add PINMISSING and PINNOCONNECT lint checks. --- Changes | 2 ++ bin/verilator | 30 ++++++++++++++++----- src/V3Error.h | 5 ++++ src/V3LinkCells.cpp | 39 ++++++++++++++++------------ test_regress/t/t_gen_missing.v | 1 + test_regress/t/t_inst_missing_bad.pl | 21 +++++++++++++++ test_regress/t/t_inst_missing_bad.v | 13 ++++++++++ 7 files changed, 89 insertions(+), 22 deletions(-) create mode 100755 test_regress/t/t_inst_missing_bad.pl create mode 100644 test_regress/t/t_inst_missing_bad.v diff --git a/Changes b/Changes index a8d572d5a..30f13630e 100644 --- a/Changes +++ b/Changes @@ -13,6 +13,8 @@ indicates the contributor was also the author of the fix; Thanks! *** Support nmos and pmos, bug488. [Alex Solomatnikov] +*** Add PINMISSING and PINNOCONNECT lint checks. + *** Fix generate operators not short circuiting, bug413. [by Jeremy Bennett] **** Fix imports causing symbol table error, bug490. [Alex Solomatnikov] diff --git a/bin/verilator b/bin/verilator index 7a09db46c..ceb0210c8 100755 --- a/bin/verilator +++ b/bin/verilator @@ -931,8 +931,8 @@ Disable the specified warning message. Disable all lint related warning messages, and all style warnings. This is equivalent to "-Wno-CASEINCOMPLETE -Wno-CASEOVERLAP -Wno-CASEX -Wno-CASEWITHX -Wno-CMPCONST -Wno-ENDLABEL -Wno-IMPLICIT -Wno-LITENDIAN --Wno-SYNCASYNCNET -Wno-UNDRIVEN -Wno-UNSIGNED -Wno-UNUSED -Wno-WIDTH" plus -the list shown for Wno-style. +-Wno-PINMISSING -Wno-SYNCASYNCNET -Wno-UNDRIVEN -Wno-UNSIGNED -Wno-UNUSED +-Wno-WIDTH" plus the list shown for Wno-style. It is strongly recommended you cleanup your code rather than using this option, it is only intended to be use when running test-cases of code @@ -942,8 +942,8 @@ received from third parties. Disable all code style related warning messages (note by default they are already disabled). This is equivalent to "-Wno-DECLFILENAME -Wno-DEFPARAM --Wno-INCABSPATH -Wno-SYNCASYNCNET -Wno-UNDRIVEN -Wno-UNUSED --Wno-VARHIDDEN". +-Wno-INCABSPATH -Wno-PINNOCONNECT -Wno-SYNCASYNCNET -Wno-UNDRIVEN +-Wno-UNUSED -Wno-VARHIDDEN". =item -Wno-fatal @@ -963,13 +963,14 @@ Enable all lint related warning messages (note by default they are already enabled), but do not affect style messages. This is equivalent to "-Wwarn-CASEINCOMPLETE -Wwarn-CASEOVERLAP -Wwarn-CASEX -Wwarn-CASEWITHX -Wwarn-CMPCONST -Wwarn-ENDLABEL -Wwarn-IMPLICIT -Wwarn-LITENDIAN --Wwarn-REALCVT -Wwarn-UNSIGNED -Wwarn-WIDTH". +-Wwarn-PINMISSING -Wwarn-REALCVT -Wwarn-UNSIGNED -Wwarn-WIDTH". =item -Wwarn-style Enable all code style related warning messages. This is equivalent to "-Wwarn ASSIGNDLY -Wwarn-DECLFILENAME -Wwarn-DEFPARAM -Wwarn-INCABSPATH --Wwarn-SYNCASYNCNET -Wwarn-UNDRIVEN -Wwarn-UNUSED -Wwarn-VARHIDDEN". +-Wwarn-PINNOCONNECT -Wwarn-SYNCASYNCNET -Wwarn-UNDRIVEN -Wwarn-UNUSED +-Wwarn-VARHIDDEN". =item -x-assign 0 @@ -2754,6 +2755,23 @@ not really needed. The best solution is to insure that each module is in a unique file by the same name. Otherwise, make sure all library files are read in as libraries with -v, instead of automatically with -y. +=item PINMISSING + +Warns that a module has a pin which is not mentioned in a cell +instantiation. If a pin is not missing it should still be specified on the +cell declaration with a empty connection,using "(.pin_name())". + +Ignoring this warning will only suppress the lint check, it will simulate +correctly. + +=item PINNOCONNECT + +Warns that a cell instantiation has a pin which is not connected to another +signal. + +Disabled by default as this is a code style warning; it will simulate +correctly. + =item REALCVT Warns that a real number is being implicitly rounded to an integer, with diff --git a/src/V3Error.h b/src/V3Error.h index 61d614a83..b2dbac90e 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -79,6 +79,8 @@ public: LITENDIAN, // Little bit endian vector MODDUP, // Duplicate module MULTIDRIVEN, // Driven from multiple blocks + PINMISSING, // Cell pin not specified + PINNOCONNECT, // Cell pin not connected REALCVT, // Real conversion REDEFMACRO, // Redefining existing define macro SELRANGE, // Selection index out of range @@ -120,6 +122,7 @@ public: "IFDEPTH", "IMPERFECTSCH", "IMPLICIT", "IMPURE", "INCABSPATH", "LITENDIAN", "MODDUP", "MULTIDRIVEN", + "PINMISSING", "PINNOCONNECT", "REALCVT", "REDEFMACRO", "SELRANGE", "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET", "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNSIGNED", "UNUSED", @@ -146,6 +149,7 @@ public: || m_e==ENDLABEL || m_e==IMPLICIT || m_e==LITENDIAN + || m_e==PINMISSING || m_e==REALCVT || m_e==UNSIGNED || m_e==WIDTH); } @@ -155,6 +159,7 @@ public: || m_e==DEFPARAM || m_e==DECLFILENAME || m_e==INCABSPATH + || m_e==PINNOCONNECT || m_e==SYNCASYNCNET || m_e==UNDRIVEN || m_e==UNUSED diff --git a/src/V3LinkCells.cpp b/src/V3LinkCells.cpp index b87b1a17e..b41dce2b5 100644 --- a/src/V3LinkCells.cpp +++ b/src/V3LinkCells.cpp @@ -234,12 +234,19 @@ private: pinp->unlinkFrBack()->deleteTree(); pinp=NULL; } } - if (nodep->modp() && pinStar) { + // Convert unnamed pins to pin number based assignments + for (AstPin* pinp = nodep->pinsp(); pinp; pinp=pinp->nextp()->castPin()) { + if (pinp->name()=="") pinp->name("__pinNumber"+cvtToStr(pinp->pinNum())); + } + for (AstPin* pinp = nodep->paramsp(); pinp; pinp=pinp->nextp()->castPin()) { + if (pinp->name()=="") pinp->name("__paramNumber"+cvtToStr(pinp->pinNum())); + } + if (nodep->modp()) { // Note what pins exist - UINFO(9," CELL .* connect "<pinsp(); pinp; pinp=pinp->nextp()->castPin()) { if (pinp->name()=="") pinp->v3error("Connect by position is illegal in .* connected cells"); + if (!pinp->exprp()) pinp->v3warn(PINNOCONNECT,"Cell pin is not connected: "<prettyName()); if (!ports.findIdFlat(pinp->name())) { ports.insert(pinp->name(), pinp); } @@ -248,24 +255,24 @@ private: // and it's easier to do it now than in V3Link when we'd need to repeat steps. for (AstNode* portnodep = nodep->modp()->stmtsp(); portnodep; portnodep=portnodep->nextp()) { if (AstPort* portp = portnodep->castPort()) { - if (!ports.findIdFlat(portp->name())) { - UINFO(9," need PORT "<fileline(),0,portp->name(), - new AstVarRef(nodep->fileline(),portp->name(),false)); - newp->svImplicit(true); - nodep->addPinsp(newp); + if (!ports.findIdFlat(portp->name()) + && !ports.findIdFlat("__pinNumber"+cvtToStr(portp->pinNum()))) { + if (pinStar) { + UINFO(9," need .* PORT "<fileline(),0,portp->name(), + new AstVarRef(nodep->fileline(),portp->name(),false)); + newp->svImplicit(true); + nodep->addPinsp(newp); + } else { // warn on the CELL that needs it, not the port + nodep->v3warn(PINMISSING, "Cell has missing pin: "<prettyName()); + AstPin* newp = new AstPin(nodep->fileline(),0,portp->name(),NULL); + nodep->addPinsp(newp); + } } } } } - // Convert unnamed pins to pin number based assignments - for (AstPin* pinp = nodep->pinsp(); pinp; pinp=pinp->nextp()->castPin()) { - if (pinp->name()=="") pinp->name("__pinNumber"+cvtToStr(pinp->pinNum())); - } - for (AstPin* pinp = nodep->paramsp(); pinp; pinp=pinp->nextp()->castPin()) { - if (pinp->name()=="") pinp->name("__paramNumber"+cvtToStr(pinp->pinNum())); - } if (nodep->modp()) { nodep->iterateChildren(*this); } diff --git a/test_regress/t/t_gen_missing.v b/test_regress/t/t_gen_missing.v index f922da616..02ef3665f 100644 --- a/test_regress/t/t_gen_missing.v +++ b/test_regress/t/t_gen_missing.v @@ -4,6 +4,7 @@ // without warranty, 2011 by Wilson Snyder. module t; + // verilator lint_off PINMISSING `ifdef T_GEN_MISSING_BAD foobar #(.FOO_TYPE(1)) foobar; // This means we should instatiate missing module `elsif T_GEN_MISSING diff --git a/test_regress/t/t_inst_missing_bad.pl b/test_regress/t/t_inst_missing_bad.pl new file mode 100755 index 000000000..b78ef868b --- /dev/null +++ b/test_regress/t/t_inst_missing_bad.pl @@ -0,0 +1,21 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003-2009 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. + +compile ( + v_flags2 => ["--lint-only --Wall -Wno-DECLFILENAME"], + fails=>1, + expect=> +q{%Warning-PINNOCONNECT: t/t_inst_missing_bad.v:\d+: Cell pin is not connected: nc +%Warning-PINNOCONNECT: Use .* +%Warning-PINMISSING: t/t_inst_missing_bad.v:\d+: Cell has missing pin: missing +%Error: Exiting due to.*}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_inst_missing_bad.v b/test_regress/t/t_inst_missing_bad.v new file mode 100644 index 000000000..61a92b363 --- /dev/null +++ b/test_regress/t/t_inst_missing_bad.v @@ -0,0 +1,13 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2012 by Wilson Snyder. + +module t (/*AUTOARG*/); + wire ok = 1'b0; + sub sub (.ok(ok), .nc()); +endmodule + +module sub (input ok, input nc, input missing); + initial if (ok&&nc&&missing) begin end // No unused warning +endmodule