From d215149c7c8f2c544cb6782c1bdfcb357de3aeff Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 6 Dec 2017 21:29:10 -0500 Subject: [PATCH] Fix false ALWCOMBORDER on interface references, bug1247. --- Changes | 2 + src/V3Undriven.cpp | 5 +- test_regress/t/t_lint_always_comb_iface.pl | 21 +++++ test_regress/t/t_lint_always_comb_iface.v | 93 ++++++++++++++++++++++ 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100755 test_regress/t/t_lint_always_comb_iface.pl create mode 100644 test_regress/t/t_lint_always_comb_iface.v diff --git a/Changes b/Changes index 94b883a6d..0906db3d6 100644 --- a/Changes +++ b/Changes @@ -8,6 +8,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix modport outputs being treated as inputs, bug1246. [Jeff Bush] +**** Fix false ALWCOMBORDER on interface references, bug1247. [Josh Redford] + * Verilator 3.916 2017-11-25 diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp index d7ff25165..3fc32fa43 100644 --- a/src/V3Undriven.cpp +++ b/src/V3Undriven.cpp @@ -238,7 +238,7 @@ private: vector m_entryps[3]; // Nodes to delete when we are finished bool m_inBBox; // In black box; mark as driven+used AstNodeFTask* m_taskp; // Current task - AstAlways* m_alwaysp; // Current always + AstAlways* m_alwaysp; // Current always if combo, otherwise NULL // METHODS static int debug() { @@ -266,6 +266,7 @@ private: AstVar* varp = nodep->varp(); if (!varp->isParam() && !varp->isGenVar() && !varp->isUsedLoopIdx() && !m_inBBox // We may have falsely considered a SysIgnore as a driver + && !nodep->castVarXRef() // Xrefs might point at two different instances && !varp->fileline()->warnIsOff(V3ErrorCode::ALWCOMBORDER)) { // Warn only once per variable nodep->v3warn(ALWCOMBORDER, "Always_comb variable driven after use: "<prettyName()); varp->fileline()->modifyWarnOff(V3ErrorCode::ALWCOMBORDER, true); // Complain just once for any usage @@ -275,6 +276,8 @@ private: // VISITORS virtual void visit(AstVar* nodep) { for (int usr=1; usr<(m_alwaysp?3:2); ++usr) { + // For assigns and non-combo always, do just usr==1, to look for module-wide undriven etc + // For non-combo always, run both usr==1 for above, and also usr==2 for always-only checks UndrivenVarEntry* entryp = getEntryp (nodep, usr); if (nodep->isInput() || nodep->isSigPublic() || nodep->isSigUserRWPublic() diff --git a/test_regress/t/t_lint_always_comb_iface.pl b/test_regress/t/t_lint_always_comb_iface.pl new file mode 100755 index 000000000..fcc84f672 --- /dev/null +++ b/test_regress/t/t_lint_always_comb_iface.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 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 ( + v_flags2 => ["--lint-only"], + fails => 0, + verilator_make_gcc => 0, + make_top_shell => 0, + make_main => 0, + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_always_comb_iface.v b/test_regress/t/t_lint_always_comb_iface.v new file mode 100644 index 000000000..0017a5e96 --- /dev/null +++ b/test_regress/t/t_lint_always_comb_iface.v @@ -0,0 +1,93 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2017 by Josh Redford. + +interface my_if; + + logic valid; + logic [7:0] data ; + + modport slave_mp ( + input valid, + input data + ); + + modport master_mp ( + output valid, + output data + ); + +endinterface + +module t + ( + input wire in_valid, + input wire [7:0] in_data + ); + + my_if in_i (); + my_if out1_i (); + my_if out2_i (); + my_if out3_i (); + + assign in_i.valid = in_valid; + assign in_i.data = in_data ; + + my_module1 my_module1_i ( + .in_i (in_i ), + .out_i (out1_i) + ); + + my_module2 my_module2_i ( + .in_i (in_i ), + .out_i (out2_i) + ); + + my_module3 my_module3_i ( + .in_i (in_i ), + .out_i (out3_i) + ); + +endmodule + +module my_module1 ( + my_if.slave_mp in_i, + my_if.master_mp out_i + ); + + // Gives ALWCOMBORDER warning + always_comb + begin + out_i.valid = in_i.valid; + out_i.data = in_i.data ; + end + +endmodule + +module my_module2 ( + my_if.slave_mp in_i, + my_if.master_mp out_i + ); + + // Works if you initialise to non-interface signal first + always_comb + begin + out_i.valid = '0; + out_i.data = 'X; + out_i.valid = in_i.valid; + out_i.data = in_i.data ; + end + +endmodule + +module my_module3 ( + my_if.slave_mp in_i, + my_if.master_mp out_i + ); + + // Works if you use assign signal + assign out_i.valid = in_i.valid; + assign out_i.data = in_i.data ; + +endmodule