From 8cc4b588b2ba92e84087831763ac7f3b92e999ad Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 14 Nov 2017 19:50:31 -0500 Subject: [PATCH] Add error when driving input-only modport. --- Changes | 2 + src/V3LinkDot.cpp | 28 +++++++++++--- src/V3Undriven.cpp | 7 +++- test_regress/t/t_lint_modport_dir_bad.pl | 24 ++++++++++++ test_regress/t/t_lint_modport_dir_bad.v | 49 ++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 7 deletions(-) create mode 100755 test_regress/t/t_lint_modport_dir_bad.pl create mode 100644 test_regress/t/t_lint_modport_dir_bad.v diff --git a/Changes b/Changes index dbd46bcd8..df91dd47a 100644 --- a/Changes +++ b/Changes @@ -10,6 +10,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Add data types to --xml. [Rui Terra] +**** Add error when driving input-only modport. + * Verilator 3.914 2017-10-14 diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index f18cf9b91..52e2cf58d 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1394,9 +1394,10 @@ class LinkDotIfaceVisitor : public AstNVisitor { if (!symp) { nodep->v3error("Modport item not found: "<prettyName()); } else if (AstVar* varp = symp->nodep()->castVar()) { - // Make symbol under modport that points at the _interface_'s var, not the modport. + // Make symbol under modport that points at the _interface_'s var via the modport. + // (Need modport still to test input/output markings) nodep->varp(varp); - m_statep->insertSym(m_curSymp, nodep->name(), varp, NULL/*package*/); + m_statep->insertSym(m_curSymp, nodep->name(), nodep, NULL/*package*/); } else if (AstVarScope* vscp = symp->nodep()->castVarScope()) { // Make symbol under modport that points at the _interface_'s var, not the modport. nodep->varp(vscp->varp()); @@ -1514,6 +1515,23 @@ private: m_statep->insertSym(moduleSymp, newp->name(), newp, NULL/*packagep*/); } } + AstVar* foundToVarp(const VSymEnt* symp, AstNode* nodep, bool lvalue) { + // Return a variable if possible, auto converting a modport to variable + if (!symp) { + return NULL; + } else if (symp->nodep()->castVar()) { + return symp->nodep()->castVar(); + } else if (symp->nodep()->castModportVarRef()) { + AstModportVarRef* snodep = symp->nodep()->castModportVarRef(); + AstVar* varp = snodep->varp(); + if (lvalue && snodep->isInput()) { + nodep->v3error("Attempt to drive input-only modport: "<prettyName()); + } // else other simulators don't warn about reading, and IEEE doesn't say illegal + return varp; + } else { + return NULL; + } + } void taskFuncSwapCheck(AstNodeFTaskRef* nodep) { if (nodep->taskp() && nodep->taskp()->castTask() && nodep->castFuncRef()) nodep->v3error("Illegal call of a task as a function: "<prettyName()); @@ -1805,7 +1823,7 @@ private: } } } - else if (AstVar* varp = foundp->nodep()->castVar()) { + else if (AstVar* varp = foundToVarp(foundp, nodep, false)) { AstIfaceRefDType* ifacerefp = LinkDotState::ifaceRefFromArray(varp->subDTypep()); if (ifacerefp) { if (!ifacerefp->ifaceViaCellp()) ifacerefp->v3fatalSrc("Unlinked interface"); @@ -1926,7 +1944,7 @@ private: UINFO(9," linkVarRef se"<<(void*)m_curSymp<<" n="<v3fatalSrc("NULL lookup symbol table"); VSymEnt* foundp = m_curSymp->findIdFallback(nodep->name()); - if (AstVar* varp = foundp ? foundp->nodep()->castVar() : NULL) { + if (AstVar* varp = foundp ? foundToVarp(foundp, nodep, nodep->lvalue()) : NULL) { nodep->varp(varp); nodep->packagep(foundp->packagep()); // Generally set by parse, but might be an import } @@ -1960,7 +1978,7 @@ private: dotSymp = m_statep->findDotted(dotSymp, nodep->dotted(), baddot, okSymp); // Maybe NULL if (!m_statep->forScopeCreation()) { VSymEnt* foundp = m_statep->findSymPrefixed(dotSymp, nodep->name(), baddot); - AstVar* varp = foundp ? foundp->nodep()->castVar() : NULL; + AstVar* varp = foundp ? foundToVarp(foundp, nodep, nodep->lvalue()) : NULL; nodep->varp(varp); UINFO(7," Resolved "<varp()) { diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp index cd1ecd68a..da4072b4c 100644 --- a/src/V3Undriven.cpp +++ b/src/V3Undriven.cpp @@ -19,7 +19,7 @@ //************************************************************************* // V3Undriven's Transformations: // -// Each module: +// Netlist: // Make vector for all variables // SEL(VARREF(...))) mark only some bits as used/driven // else VARREF(...) mark all bits as used/driven @@ -223,8 +223,11 @@ public: class UndrivenVisitor : public AstNVisitor { private: // NODE STATE - // AstVar::user1p -> UndrivenVar* for usage var, 0=not set yet + // Netlist: + // AstVar::user1p -> UndrivenVar* for usage var, 0=not set yet AstUser1InUse m_inuser1; + // Each always: + // AstNode::user2p -> UndrivenVar* for usage var, 0=not set yet AstUser2InUse m_inuser2; // STATE diff --git a/test_regress/t/t_lint_modport_dir_bad.pl b/test_regress/t/t_lint_modport_dir_bad.pl new file mode 100755 index 000000000..cd0978837 --- /dev/null +++ b/test_regress/t/t_lint_modport_dir_bad.pl @@ -0,0 +1,24 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2008 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. + +#$Self->{vlt} or $Self->skip("Verilator only test"); + +compile ( + verilator_flags2 => ["--lint-only -Wno-DECLFILENAME"], + fails=>1, + verilator_make_gcc => 0, + make_top_shell => 0, + make_main => 0, + expect=> +'%Error: t/t_lint_modport_dir_bad.v:\d+: Attempt to drive input-only modport: signal +%Error: Exiting due to .*', + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_modport_dir_bad.v b/test_regress/t/t_lint_modport_dir_bad.v new file mode 100644 index 000000000..57ba33ffa --- /dev/null +++ b/test_regress/t/t_lint_modport_dir_bad.v @@ -0,0 +1,49 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2017 by Wilson Snyder. + +interface dummy_if (); + logic signal; + + modport slave + (output signal); + + modport master + (input signal); +endinterface: dummy_if + +module sub + ( + input wire signal_i, + output wire signal_o, + + dummy_if.master dummy_in, + dummy_if.slave dummy_out + ); + + assign dummy_in.signal = signal_i; + assign signal_o = dummy_out.signal; +endmodule + + +module t (/*AUTOARG*/ + // Outputs + signal_o, + // Inputs + signal_i + ); + input signal_i; + output signal_o; + + dummy_if dummy_if (); + + sub sub + ( + .signal_i(signal_i), + .signal_o(signal_o), + .dummy_in(dummy_if), + .dummy_out(dummy_if) + ); + +endmodule