diff --git a/bin/verilator b/bin/verilator index 567301f27..e581f0867 100755 --- a/bin/verilator +++ b/bin/verilator @@ -4591,6 +4591,16 @@ exposed is acceptable. Ignoring this warning will only suppress the lint check, it will simulate correctly. +=item LATCH + +Warns that a signal is not assigned in all control paths of a combinational +always block, resulting in the inference of a latch. For intentional +latches, consider using the always_latch (SystemVerilog) keyword instead. +The warning may be disabled with a lint_off pragma around the always block. + +Ignoring this warning will only suppress the lint check, it will simulate +correctly. + =item LITENDIAN Warns that a packed vector is declared with little endian bit numbering @@ -4654,6 +4664,15 @@ based on the top module name followed by __02E (a Verilator-encoded ASCII ".'). This renaming is done even if the two modules' signals seem identical, e.g. multiple modules with a "clk" input. +=item NOLATCH + +Warns that no latch was detected in an always_latch block. The warning may +be disabled with a lint_off pragma around the always block, but recoding +using a regular always may be more appropriate. + +Ignoring this warning will only suppress the lint check, it will simulate +correctly. + =item PINCONNECTEMPTY Warns that an instance has a pin which is connected to .pin_name(), diff --git a/src/V3Active.cpp b/src/V3Active.cpp index baea23a7d..a0d3de8fd 100644 --- a/src/V3Active.cpp +++ b/src/V3Active.cpp @@ -35,11 +35,168 @@ #include "V3EmitCBase.h" #include "V3Const.h" #include "V3SenTree.h" // for SenTreeSet +#include "V3Graph.h" #include //***** See below for main transformation engine +//###################################################################### + +// Extend V3GraphVertex class for use in latch detection graph + +class LatchDetectGraphVertex final : public V3GraphVertex { +public: + enum VertexType : uint8_t { VT_BLOCK, VT_BRANCH, VT_OUTPUT }; + +private: + string m_name; // Only used for .dot file generation + VertexType m_type; // Vertex type (BLOCK/BRANCH/OUTPUT) + + string typestr() const { // " + switch (m_type) { + case VT_BLOCK: return "(||)"; // basic block node + case VT_BRANCH: return "(&&)"; // if/else branch mode + case VT_OUTPUT: return "(out)"; // var assignment + default: return "??"; // unknown + } + } + +public: + LatchDetectGraphVertex(V3Graph* graphp, const string& name, VertexType type = VT_BLOCK) + : V3GraphVertex(graphp) + , m_name(name) + , m_type(type) {} + virtual string name() const { return m_name + " " + typestr(); } + virtual string dotColor() const { return user() ? "green" : "black"; } + virtual int type() const { return m_type; } +}; + +//###################################################################### +// Extend V3Graph class for use as a latch detection graph + +class LatchDetectGraph final : public V3Graph { +protected: + typedef std::vector VarRefVec; + + LatchDetectGraphVertex* m_curVertexp; // Current latch detection graph vertex + VarRefVec m_outputs; // Vector of lvalues encountered on this pass + + VL_DEBUG_FUNC; // Declare debug() + + static LatchDetectGraphVertex* castVertexp(void* vertexp) { + return reinterpret_cast(vertexp); + } + + // Recursively traverse the graph to determine whether every control 'BLOCK' has an assignment + // to the output we are currently analysing (the output whose 'user() is set), if so return + // true. Where a BLOCK contains a BRANCH, both the if and else sides of the branch must return + // true for the BRANCH to evalute to true. A BLOCK however needs only a single one of its + // siblings to evaluate true in order to evaluate true itself. On output vertex only evaluates + // true if it is the vertex we are analyzing on this check + + bool latchCheckInternal(LatchDetectGraphVertex* vertexp) { + bool result = false; + switch (vertexp->type()) { + case LatchDetectGraphVertex::VT_OUTPUT: // Base case + result = vertexp->user(); + break; + case LatchDetectGraphVertex::VT_BLOCK: // (OR of potentially many siblings) + for (V3GraphEdge* edgep = vertexp->outBeginp(); edgep; edgep = edgep->outNextp()) { + if (latchCheckInternal(castVertexp(edgep->top()))) { + result = true; + break; + } + } + break; + case LatchDetectGraphVertex::VT_BRANCH: // (AND of both sibling) + // A BRANCH vertex always has exactly 2 siblings + LatchDetectGraphVertex* ifp = castVertexp(vertexp->outBeginp()->top()); + LatchDetectGraphVertex* elsp = castVertexp(vertexp->outBeginp()->outNextp()->top()); + result = latchCheckInternal(ifp) && latchCheckInternal(elsp); + break; + } + vertexp->user(result); + return result; + } + +public: + LatchDetectGraph() { clear(); } + ~LatchDetectGraph() { clear(); } + // ACCESSORS + LatchDetectGraphVertex* currentp() { return m_curVertexp; } + void currentp(LatchDetectGraphVertex* vertex) { m_curVertexp = vertex; } + // METHODS + void begin() { + // Start a new if/else tracking graph + // See NODE STATE comment in ActiveLatchCheckVisitor + AstNode::user1ClearTree(); + m_curVertexp = new LatchDetectGraphVertex(this, "ROOT"); + } + // Clear out userp field of referenced outputs on destruction + // (occurs at the end of each combinational always block) + void clear() { + m_outputs.clear(); + // Calling base class clear will unlink & delete all edges & vertices + V3Graph::clear(); + m_curVertexp = nullptr; + } + // Add a new control path and connect it to its parent + LatchDetectGraphVertex* addPathVertex(LatchDetectGraphVertex* parent, const string& name, + bool branch = false) { + m_curVertexp = new LatchDetectGraphVertex(this, name, + branch ? LatchDetectGraphVertex::VT_BRANCH + : LatchDetectGraphVertex::VT_BLOCK); + new V3GraphEdge(this, parent, m_curVertexp, 1); + return m_curVertexp; + } + // Add a new output variable vertex and store a pointer to it in the user1 field of the + // variables AstNode + LatchDetectGraphVertex* addOutputVertex(AstVarRef* nodep) { + LatchDetectGraphVertex* outVertexp + = new LatchDetectGraphVertex(this, nodep->name(), LatchDetectGraphVertex::VT_OUTPUT); + nodep->varp()->user1p(outVertexp); + m_outputs.push_back(nodep); + return outVertexp; + } + // Connect an output assignment to its parent control block + void addAssignment(AstVarRef* nodep) { + LatchDetectGraphVertex* outVertexp; + if (!nodep->varp()->user1p()) { // Not seen this output before + outVertexp = addOutputVertex(nodep); + } else + outVertexp = castVertexp(nodep->varp()->user1p()); + + new V3GraphEdge(this, m_curVertexp, outVertexp, 1); + } + // Run latchCheckInternal on each variable assigned by the always block to see if all control + // paths make an assignment. Detected latches are flagged in the variables AstVar + void latchCheck(AstNode* nodep, bool latch_expected) { + bool latch_detected = false; + for (const auto& vrp : m_outputs) { + LatchDetectGraphVertex* vertp = castVertexp(vrp->varp()->user1p()); + vertp->user(true); // Identify the output vertex we are checking paths _to_ + if (!latchCheckInternal(castVertexp(verticesBeginp()))) { latch_detected = true; } + if (latch_detected && !latch_expected) { + nodep->v3warn( + LATCH, + "Latch inferred for signal " + << vrp->prettyNameQ() + << " (not all control paths of combinational always assign a value)\n" + << nodep->warnMore() + << "... Suggest use of always_latch for intentional latches"); + if (debug() >= 9) { dumpDotFilePrefixed("latch_" + vrp->name()); } + } + vertp->user(false); // Clear again (see above) + vrp->varp()->isLatched(latch_detected); + } + // Should _all_ variables assigned in always_latch be latches? Probably, but this only + // warns if none of them are + if (latch_expected && !latch_detected) + nodep->v3warn(NOLATCH, "No latches detected in always_latch block"); + } +}; + //###################################################################### // Collect existing active names @@ -135,6 +292,48 @@ public: void main(AstScope* nodep) { iterate(nodep); } }; +//###################################################################### +// Latch checking visitor + +class ActiveLatchCheckVisitor final : public ActiveBaseVisitor { +private: + // NODE STATE + // Input: + // AstVar::user1p // V2LatchGraphVertex* The vertex handling this node + AstUser1InUse m_inuser1; + // STATE + LatchDetectGraph m_graph; // Graph used to detect latches in combo always + // VISITORS + virtual void visit(AstVarRef* nodep) { + AstVar* varp = nodep->varp(); + if (nodep->access().isWriteOrRW() && varp->isSignal() && !varp->isUsedLoopIdx()) { + m_graph.addAssignment(nodep); + } + } + virtual void visit(AstNodeIf* nodep) { + LatchDetectGraphVertex* parentp = m_graph.currentp(); + LatchDetectGraphVertex* branchp = m_graph.addPathVertex(parentp, "BRANCH", true); + m_graph.addPathVertex(branchp, "IF"); + iterateAndNextNull(nodep->ifsp()); + m_graph.addPathVertex(branchp, "ELSE"); + iterateAndNextNull(nodep->elsesp()); + m_graph.currentp(parentp); + } + // Empty visitors, speed things up + virtual void visit(AstNodeMath* nodep) {} + //-------------------- + virtual void visit(AstNode* nodep) { iterateChildren(nodep); } + +public: + // CONSTRUCTORS + ActiveLatchCheckVisitor(AstNode* nodep, VAlwaysKwd kwd) { + m_graph.begin(); + iterate(nodep); + m_graph.latchCheck(nodep, kwd == VAlwaysKwd::ALWAYS_LATCH); + } + virtual ~ActiveLatchCheckVisitor() {} +}; + //###################################################################### // Active AssignDly replacement functions @@ -157,11 +356,14 @@ private: << "... Suggest blocking assignments (=)"); } else if (m_check == CT_LATCH) { // Suppress. Shouldn't matter that the interior of the latch races - } else { + } else if (!(VN_IS(nodep->lhsp(), VarRef) + && VN_CAST(nodep->lhsp(), VarRef)->varp()->isLatched())) { nodep->v3warn(COMBDLY, "Delayed assignments (<=) in non-clocked" " (non flop or latch) block\n" << nodep->warnMore() << "... Suggest blocking assignments (=)"); + // Conversely, we could also suggest latches use delayed assignments, as + // recommended by Cliff Cummings? } AstNode* newp = new AstAssign(nodep->fileline(), nodep->lhsp()->unlinkFrBack(), nodep->rhsp()->unlinkFrBack()); @@ -341,6 +543,7 @@ private: // Warn and/or convert any delayed assignments if (combo && !sequent) { + ActiveLatchCheckVisitor latchvisitor(nodep, kwd); if (kwd == VAlwaysKwd::ALWAYS_LATCH) { ActiveDlyVisitor dlyvisitor(nodep, ActiveDlyVisitor::CT_LATCH); } else { diff --git a/src/V3Ast.h b/src/V3Ast.h index 8c591aa34..2295ff743 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -157,7 +157,7 @@ public: bool isReadOnly() const { return m_e == READ; } // False with READWRITE bool isReadOrRW() const { return m_e == READ || m_e == READWRITE; } bool isWriteOrRW() const { return m_e == WRITE || m_e == READWRITE; } - bool isRW() const { return m_e == READWRITE; } // False with READWRITE + bool isRW() const { return m_e == READWRITE; } }; inline bool operator==(const VAccess& lhs, const VAccess& rhs) { return lhs.m_e == rhs.m_e; } inline bool operator==(const VAccess& lhs, VAccess::en rhs) { return lhs.m_e == rhs; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index d2a448c85..262b0421b 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1580,6 +1580,7 @@ void AstVar::dump(std::ostream& str) const { if (isPulldown()) str << " [PULLDOWN]"; if (isUsedClock()) str << " [CLK]"; if (isSigPublic()) str << " [P]"; + if (isLatched()) str << " [LATCHED]"; if (isUsedLoopIdx()) str << " [LOOP]"; if (attrClockEn()) str << " [aCLKEN]"; if (attrIsolateAssign()) str << " [aISO]"; diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 6d52fb85e..117d28b50 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -1949,6 +1949,7 @@ private: bool m_noSubst : 1; // Do not substitute out references bool m_overridenParam : 1; // Overridden parameter by #(...) or defparam bool m_trace : 1; // Trace this variable + bool m_isLatched : 1; // Not assigned in all control paths of combo always VLifetime m_lifetime; // Lifetime VVarAttrClocker m_attrClocker; MTaskIdSet m_mtaskIds; // MTaskID's that read or write this var @@ -1989,6 +1990,7 @@ private: m_noSubst = false; m_overridenParam = false; m_trace = false; + m_isLatched = false; m_attrClocker = VVarAttrClocker::CLOCKER_UNKNOWN; } @@ -2146,6 +2148,7 @@ public: void overriddenParam(bool flag) { m_overridenParam = flag; } bool overriddenParam() const { return m_overridenParam; } void trace(bool flag) { m_trace = flag; } + void isLatched(bool flag) { m_isLatched = flag; } // METHODS virtual void name(const string& name) override { m_name = name; } virtual void tag(const string& text) override { m_tag = text; } @@ -2199,6 +2202,7 @@ public: bool isRand() const { return m_isRand; } bool isConst() const { return m_isConst; } bool isStatic() const { return m_isStatic; } + bool isLatched() const { return m_isLatched; } bool isFuncLocal() const { return m_funcLocal; } bool isFuncReturn() const { return m_funcReturn; } bool isPullup() const { return m_isPullup; } diff --git a/src/V3Case.cpp b/src/V3Case.cpp index 9147b40f0..d40c6eb0e 100644 --- a/src/V3Case.cpp +++ b/src/V3Case.cpp @@ -127,6 +127,7 @@ private: // STATE VDouble0 m_statCaseFast; // Statistic tracking VDouble0 m_statCaseSlow; // Statistic tracking + AstNode* m_alwaysp = nullptr; // Always in which case is located // Per-CASE int m_caseWidth = 0; // Width of valueItems @@ -475,12 +476,17 @@ private: ++m_statCaseFast; VL_DO_DANGLING(replaceCaseFast(nodep), nodep); } else { + // If a case statement is whole, presume signals involved aren't forming a latch + if (m_alwaysp) m_alwaysp->fileline()->warnOff(V3ErrorCode::LATCH, true); ++m_statCaseSlow; VL_DO_DANGLING(replaceCaseComplicated(nodep), nodep); } } //-------------------- - virtual void visit(AstNode* nodep) override { iterateChildren(nodep); } + virtual void visit(AstNode* nodep) override { + if (VN_IS(nodep, Always)) { m_alwaysp = nodep; } + iterateChildren(nodep); + } public: // CONSTRUCTORS diff --git a/src/V3EmitXml.cpp b/src/V3EmitXml.cpp index 030768097..27770caa5 100644 --- a/src/V3EmitXml.cpp +++ b/src/V3EmitXml.cpp @@ -151,6 +151,7 @@ class EmitXmlFileVisitor final : public AstNVisitor { } if (nodep->attrClockEn()) puts(" clock_enable=\"true\""); if (nodep->attrIsolateAssign()) puts(" isolate_assignments=\"true\""); + if (nodep->isLatched()) puts(" latched=\"true\""); if (nodep->isSigPublic()) puts(" public=\"true\""); if (nodep->isSigUserRdPublic()) puts(" public_flat_rd=\"true\""); if (nodep->isSigUserRWPublic()) puts(" public_flat_rw=\"true\""); diff --git a/src/V3Error.h b/src/V3Error.h index 56875cf3a..9f3b8daee 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -94,10 +94,12 @@ public: INFINITELOOP, // Infinite loop INITIALDLY, // Initial delayed statement INSECURE, // Insecure options + LATCH, // Latch detected outside of always_latch block LITENDIAN, // Little bit endian vector MODDUP, // Duplicate module MULTIDRIVEN, // Driven from multiple blocks MULTITOP, // Multiple top level modules + NOLATCH, // No latch detected in always_latch block PINMISSING, // Cell pin not specified PINNOCONNECT, // Cell pin not connected PINCONNECTEMPTY,// Cell pin connected by name with empty reference @@ -162,8 +164,8 @@ public: "IFDEPTH", "IGNOREDRETURN", "IMPERFECTSCH", "IMPLICIT", "IMPORTSTAR", "IMPURE", "INCABSPATH", "INFINITELOOP", "INITIALDLY", "INSECURE", - "LITENDIAN", "MODDUP", - "MULTIDRIVEN", "MULTITOP", + "LATCH", "LITENDIAN", "MODDUP", + "MULTIDRIVEN", "MULTITOP","NOLATCH", "PINMISSING", "PINNOCONNECT", "PINCONNECTEMPTY", "PKGNODECL", "PROCASSWIRE", "RANDC", "REALCVT", "REDEFMACRO", "SELRANGE", "SHORTREAL", "SPLITVAR", "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET", @@ -199,8 +201,8 @@ public: return (m_e == ALWCOMBORDER || m_e == BSSPACE || m_e == CASEINCOMPLETE || m_e == CASEOVERLAP || m_e == CASEWITHX || m_e == CASEX || m_e == CASTCONST || m_e == CMPCONST || m_e == COLONPLUS || m_e == ENDLABEL || m_e == IMPLICIT - || m_e == LITENDIAN || m_e == PINMISSING || m_e == REALCVT || m_e == UNSIGNED - || m_e == WIDTH); + || m_e == LATCH || m_e == LITENDIAN || m_e == PINMISSING || m_e == REALCVT + || m_e == UNSIGNED || m_e == WIDTH); } // Warnings that are style only bool styleError() const { diff --git a/test_regress/t/t_altera_lpm.v b/test_regress/t/t_altera_lpm.v index 16464a0ae..44676b6dc 100644 --- a/test_regress/t/t_altera_lpm.v +++ b/test_regress/t/t_altera_lpm.v @@ -51,6 +51,7 @@ // verilator lint_off MULTIDRIVEN // verilator lint_off UNSIGNED // verilator lint_off WIDTH +// verilator lint_off LATCH // BEGINNING OF MODULE `timescale 1 ps / 1 ps diff --git a/test_regress/t/t_case_huge_sub4.v b/test_regress/t/t_case_huge_sub4.v index b4eb77d28..221dafcc8 100644 --- a/test_regress/t/t_case_huge_sub4.v +++ b/test_regress/t/t_case_huge_sub4.v @@ -4,6 +4,8 @@ // any use, without warranty, 2005 by Wilson Snyder. // SPDX-License-Identifier: CC0-1.0 +// verilator lint_off LATCH + module t_case_huge_sub4 (/*AUTOARG*/ // Outputs outq, diff --git a/test_regress/t/t_clk_condflop.v b/test_regress/t/t_clk_condflop.v index 7cc4e1b69..98852229a 100644 --- a/test_regress/t/t_clk_condflop.v +++ b/test_regress/t/t_clk_condflop.v @@ -112,6 +112,7 @@ module clockgate (clk, sen, ena, gatedclk); wire gatedclk = clk & ena_b; // verilator lint_off COMBDLY + // verilator lint_off LATCH always @(clk or ena or sen) begin if (~clk) begin ena_b <= ena | sen; @@ -120,6 +121,7 @@ module clockgate (clk, sen, ena, gatedclk); if ((clk^sen)===1'bX) ena_b <= 1'bX; end end + // verilator lint_on LATCH // verilator lint_on COMBDLY endmodule diff --git a/test_regress/t/t_clk_condflop_nord.v b/test_regress/t/t_clk_condflop_nord.v index 7cc4e1b69..98852229a 100644 --- a/test_regress/t/t_clk_condflop_nord.v +++ b/test_regress/t/t_clk_condflop_nord.v @@ -112,6 +112,7 @@ module clockgate (clk, sen, ena, gatedclk); wire gatedclk = clk & ena_b; // verilator lint_off COMBDLY + // verilator lint_off LATCH always @(clk or ena or sen) begin if (~clk) begin ena_b <= ena | sen; @@ -120,6 +121,7 @@ module clockgate (clk, sen, ena, gatedclk); if ((clk^sen)===1'bX) ena_b <= 1'bX; end end + // verilator lint_on LATCH // verilator lint_on COMBDLY endmodule diff --git a/test_regress/t/t_clk_latch.v b/test_regress/t/t_clk_latch.v index 7fc4e96ea..caf25288c 100644 --- a/test_regress/t/t_clk_latch.v +++ b/test_regress/t/t_clk_latch.v @@ -53,6 +53,7 @@ module t (/*AUTOARG*/ end // verilator lint_off COMBDLY + // verilator lint_off LATCH always @ (`posstyle clk /*AS*/ or data) begin if (clk) begin data_a <= data + 8'd1; diff --git a/test_regress/t/t_clk_latchgate.v b/test_regress/t/t_clk_latchgate.v index bf43d7c06..120db757f 100644 --- a/test_regress/t/t_clk_latchgate.v +++ b/test_regress/t/t_clk_latchgate.v @@ -111,11 +111,13 @@ module llq (clk, d, q); reg [WIDTH-1:0] qr; /* verilator lint_off COMBDLY */ + /* verilator lint_off LATCH */ always @(clk or d) if (clk == 1'b0) qr <= d; + /* verilator lint_on LATCH */ /* verilator lint_on COMBDLY */ assign q = qr; diff --git a/test_regress/t/t_dedupe_clk_gate.pl b/test_regress/t/t_dedupe_clk_gate.pl index 229d1186d..631bd3480 100755 --- a/test_regress/t/t_dedupe_clk_gate.pl +++ b/test_regress/t/t_dedupe_clk_gate.pl @@ -17,7 +17,7 @@ compile( ); if ($Self->{vlt_all}) { - file_grep("$out_filename", qr/\/i); + file_grep("$out_filename", qr/\/i); file_grep($Self->{stats}, qr/Optimizations, Gate sigs deduped\s+(\d+)/i, 4); } diff --git a/test_regress/t/t_dedupe_clk_gate.v b/test_regress/t/t_dedupe_clk_gate.v index 1cfcec315..af6331196 100644 --- a/test_regress/t/t_dedupe_clk_gate.v +++ b/test_regress/t/t_dedupe_clk_gate.v @@ -45,7 +45,7 @@ module clock_gate_latch (gated_clk, clk, clken); assign gated_clk = clk & clken_latched ; wire clkb = ~clk; - always @(clkb or clken) + always_latch @(clkb or clken) if(clkb) clken_latched = clken; endmodule diff --git a/test_regress/t/t_lint_latch_1.pl b/test_regress/t/t_lint_latch_1.pl new file mode 100755 index 000000000..629a44bbb --- /dev/null +++ b/test_regress/t/t_lint_latch_1.pl @@ -0,0 +1,17 @@ +#!/usr/bin/env 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. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +lint( + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_latch_1.v b/test_regress/t/t_lint_latch_1.v new file mode 100644 index 000000000..95ae861c2 --- /dev/null +++ b/test_regress/t/t_lint_latch_1.v @@ -0,0 +1,16 @@ +// DESCRIPTION: Verilator: Verilog Test module for Issue#1609 +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Julien Margetts. + +module t (/*AUTOARG*/ a, b, o); + input a; + input b; + output reg o; + + // verilator lint_off LATCH + always @(a or b) + if (a) + o <= b; + +endmodule diff --git a/test_regress/t/t_lint_latch_2.pl b/test_regress/t/t_lint_latch_2.pl new file mode 100755 index 000000000..629a44bbb --- /dev/null +++ b/test_regress/t/t_lint_latch_2.pl @@ -0,0 +1,17 @@ +#!/usr/bin/env 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. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +lint( + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_latch_2.v b/test_regress/t/t_lint_latch_2.v new file mode 100644 index 000000000..d3c93d9d0 --- /dev/null +++ b/test_regress/t/t_lint_latch_2.v @@ -0,0 +1,20 @@ +// DESCRIPTION: Verilator: Verilog Test module for Issue#1609 +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Julien Margetts. + +module t (/*AUTOARG*/ i, o); + + input [1:0] i; + output reg [1:0] o; + + // This should not detect a latch as all options are covered + always @* begin + if (i==2'b00) o = 2'b11; + else if (i==2'b01) o = 2'b10; + else if (i==2'b10) o = 2'b01; + else if (i==2'b11) o = 2'b00; + else o = 2'b00; // Without this else a latch is (falsely) detected + end + +endmodule diff --git a/test_regress/t/t_lint_latch_3.pl b/test_regress/t/t_lint_latch_3.pl new file mode 100755 index 000000000..629a44bbb --- /dev/null +++ b/test_regress/t/t_lint_latch_3.pl @@ -0,0 +1,17 @@ +#!/usr/bin/env 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. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +lint( + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_latch_3.v b/test_regress/t/t_lint_latch_3.v new file mode 100644 index 000000000..6ba537f89 --- /dev/null +++ b/test_regress/t/t_lint_latch_3.v @@ -0,0 +1,50 @@ +// DESCRIPTION: Verilator: Verilog Test module for Issue#1609 +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Julien Margetts. + +module t (/*AUTOARG*/ out, out2, in ); + + input [9:0] in; + output reg [3:0] out; + output reg [3:0] out2; + + // Should be no latch here since the input space is fully covered + + always @* begin + casez (in) + 10'b0000000000 : out = 4'h0; + 10'b?????????1 : out = 4'h0; + 10'b????????10 : out = 4'h1; + 10'b???????100 : out = 4'h2; + 10'b??????1000 : out = 4'h3; + 10'b?????10000 : out = 4'h4; + 10'b????100000 : out = 4'h5; + 10'b???1000000 : out = 4'h6; + 10'b??10000000 : out = 4'h7; + 10'b?100000000 : out = 4'h8; + 10'b1000000000 : out = 4'h9; + endcase + end + + // Should detect a latch here since not all paths assign + // BUT we don't because warnOff(LATCH) is set for any always containing a + // complex case statement + + always @* begin + casez (in) + 10'b0000000000 : out2 = 4'h0; + 10'b?????????1 : out2 = 4'h0; + 10'b????????10 : out2 = 4'h1; + 10'b???????100 : out2 = 4'h2; + 10'b??????1000 : out2 = 4'h3; + 10'b?????10000 : /* No assignement */ ; + 10'b????100000 : out2 = 4'h5; + 10'b???1000000 : out2 = 4'h6; + 10'b??10000000 : out2 = 4'h7; + 10'b?100000000 : out2 = 4'h8; + 10'b1000000000 : out2 = 4'h9; + endcase + end + +endmodule diff --git a/test_regress/t/t_lint_latch_bad.out b/test_regress/t/t_lint_latch_bad.out index 751a7516f..eeeb70548 100644 --- a/test_regress/t/t_lint_latch_bad.out +++ b/test_regress/t/t_lint_latch_bad.out @@ -1,8 +1,11 @@ +%Warning-NOLATCH: t/t_lint_latch_bad.v:17:4: No latches detected in always_latch block + 17 | always_latch begin + | ^~~~~~~~~~~~ + ... Use "/* verilator lint_off NOLATCH */" and lint_on around source to disable this message. %Warning-COMBDLY: t/t_lint_latch_bad.v:25:10: Delayed assignments (<=) in non-clocked (non flop or latch) block : ... Suggest blocking assignments (=) 25 | bc <= a; | ^~ - ... Use "/* verilator lint_off COMBDLY */" and lint_on around source to disable this message. *** See the manual before disabling this, else you may end up with different sim results. %Error: Exiting due to diff --git a/test_regress/t/t_lint_latch_bad_2.out b/test_regress/t/t_lint_latch_bad_2.out new file mode 100644 index 000000000..ea37dddf6 --- /dev/null +++ b/test_regress/t/t_lint_latch_bad_2.out @@ -0,0 +1,6 @@ +%Warning-LATCH: t/t_lint_latch_bad_2.v:11:4: Latch inferred for signal 'o' (not all control paths of combinational always assign a value) + : ... Suggest use of always_latch for intentional latches + 11 | always @(a or b) + | ^~~~~~ + ... Use "/* verilator lint_off LATCH */" and lint_on around source to disable this message. +%Error: Exiting due to diff --git a/test_regress/t/t_lint_latch_bad_2.pl b/test_regress/t/t_lint_latch_bad_2.pl new file mode 100755 index 000000000..34ebfdc61 --- /dev/null +++ b/test_regress/t/t_lint_latch_bad_2.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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. + +scenarios(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_latch_bad_2.v b/test_regress/t/t_lint_latch_bad_2.v new file mode 100644 index 000000000..e4719d0e0 --- /dev/null +++ b/test_regress/t/t_lint_latch_bad_2.v @@ -0,0 +1,15 @@ +// DESCRIPTION: Verilator: Verilog Test module for Issue#1609 +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Julien Margetts. + +module t (/*AUTOARG*/ a, b, o); + input a; + input b; + output reg o; + + always @(a or b) + if (a) + o <= b; + +endmodule diff --git a/test_regress/t/t_lint_latch_bad_3.out b/test_regress/t/t_lint_latch_bad_3.out new file mode 100644 index 000000000..9b132b899 --- /dev/null +++ b/test_regress/t/t_lint_latch_bad_3.out @@ -0,0 +1,12 @@ +%Warning-LATCH: t/t_lint_latch_bad_3.v:18:1: Latch inferred for signal 'o5' (not all control paths of combinational always assign a value) + : ... Suggest use of always_latch for intentional latches + 18 | always @(reset or en or a or b) + | ^~~~~~ + ... Use "/* verilator lint_off LATCH */" and lint_on around source to disable this message. +%Warning-COMBDLY: t/t_lint_latch_bad_3.v:70:12: Delayed assignments (<=) in non-clocked (non flop or latch) block + : ... Suggest blocking assignments (=) + 70 | o4 <= 1'b0; + | ^~ + *** See the manual before disabling this, + else you may end up with different sim results. +%Error: Exiting due to diff --git a/test_regress/t/t_lint_latch_bad_3.pl b/test_regress/t/t_lint_latch_bad_3.pl new file mode 100755 index 000000000..34ebfdc61 --- /dev/null +++ b/test_regress/t/t_lint_latch_bad_3.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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. + +scenarios(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_latch_bad_3.v b/test_regress/t/t_lint_latch_bad_3.v new file mode 100644 index 000000000..90890e1c8 --- /dev/null +++ b/test_regress/t/t_lint_latch_bad_3.v @@ -0,0 +1,74 @@ +// DESCRIPTION: Verilator: Verilog Test module for Issue#1609 +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Julien Margetts. + +module t (/*AUTOARG*/ reset, a, b, c, en, o1, o2, o3, o4, o5); + input reset; + input a; + input b; + input c; + input en; + output reg o1; // Always assigned + output reg o2; // " + output reg o3; // " + output reg o4; // " + output reg o5; // Latch + +always @(reset or en or a or b) +if (reset) +begin + o1 = 1'b0; + o2 = 1'b0; + o3 = 1'b0; + o4 = 1'b0; + o5 <= 1'b0; // Do NOT expect Warning-COMBDLY +end +else +begin + o1 = 1'b1; + if (en) + begin + o2 = 1'b0; + + if (a) + begin + o3 = a; + o5 <= 1'b1; // Do NOT expect Warning-COMBDLY + end + else + begin + o3 = ~a; + o5 <= a; // Do NOT expect Warning-COMBDLY + end + + // o3 is not assigned in either path of this if/else + // but no latch because always assigned above + if (c) + begin + o2 = a ^ b; + o4 = 1'b1; + end + else + o4 = ~a ^ b; + + o2 = 1'b1; + end + else + begin + o2 = 1'b1; + if (b) + begin + o3 = ~a | b; + o5 <= ~b; // Do NOT expect Warning-COMBDLY + end + else + begin + o3 = a & ~b; + // No assignment to o5, expect Warning-LATCH + end + o4 <= 1'b0; // expect Warning-COMBDLY + end +end + +endmodule diff --git a/test_regress/t/t_lint_nolatch_bad.out b/test_regress/t/t_lint_nolatch_bad.out new file mode 100644 index 000000000..2cdc19da9 --- /dev/null +++ b/test_regress/t/t_lint_nolatch_bad.out @@ -0,0 +1,5 @@ +%Warning-NOLATCH: t/t_lint_nolatch_bad.v:11:4: No latches detected in always_latch block + 11 | always_latch @(a or b) + | ^~~~~~~~~~~~ + ... Use "/* verilator lint_off NOLATCH */" and lint_on around source to disable this message. +%Error: Exiting due to diff --git a/test_regress/t/t_lint_nolatch_bad.pl b/test_regress/t/t_lint_nolatch_bad.pl new file mode 100755 index 000000000..0b13642c9 --- /dev/null +++ b/test_regress/t/t_lint_nolatch_bad.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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. + +scenarios(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename} + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_nolatch_bad.v b/test_regress/t/t_lint_nolatch_bad.v new file mode 100644 index 000000000..517f57f94 --- /dev/null +++ b/test_regress/t/t_lint_nolatch_bad.v @@ -0,0 +1,17 @@ +// DESCRIPTION: Verilator: Verilog Test module for Issue#1609 +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2020 by Julien Margetts. + +module t (/*AUTOARG*/ a, b, o); + input a; + input b; + output reg o; + + always_latch @(a or b) + if (a) + o <= b; + else + o <= ~b; + +endmodule diff --git a/test_regress/t/t_lint_unsup_deassign.v b/test_regress/t/t_lint_unsup_deassign.v index 05454d438..77c1f2f1d 100644 --- a/test_regress/t/t_lint_unsup_deassign.v +++ b/test_regress/t/t_lint_unsup_deassign.v @@ -11,10 +11,12 @@ module t integer q; + // verilator lint_off LATCH always @(*) if (rst) assign q = 0; else deassign q; + // verilator lint_on LATCH endmodule diff --git a/test_regress/t/t_math_cmp.v b/test_regress/t/t_math_cmp.v index 6df08388a..bf5dc80e4 100644 --- a/test_regress/t/t_math_cmp.v +++ b/test_regress/t/t_math_cmp.v @@ -82,6 +82,7 @@ module prover ( reg signed [WIDTH-1:0] bs; wire [WIDTH-1:0] b = bs; + // verilator lint_off LATCH always @* begin casez (index_a) 3'd0: as = {(WIDTH){1'd0}}; // 0 @@ -100,6 +101,7 @@ module prover ( default: $stop; endcase end + // verilator lint_on LATCH reg [7:0] results[4:0][4:0]; diff --git a/test_regress/t/t_order_clkinst.v b/test_regress/t/t_order_clkinst.v index 4897d540a..859224e98 100644 --- a/test_regress/t/t_order_clkinst.v +++ b/test_regress/t/t_order_clkinst.v @@ -11,6 +11,7 @@ module t (/*AUTOARG*/ input clk; // verilator lint_off COMBDLY + // verilator lint_off LATCH // verilator lint_off UNOPT // verilator lint_off UNOPTFLAT // verilator lint_off BLKANDNBLK diff --git a/test_regress/t/t_order_clkinst_bad.out b/test_regress/t/t_order_clkinst_bad.out index bd4b51c51..2e369155a 100644 --- a/test_regress/t/t_order_clkinst_bad.out +++ b/test_regress/t/t_order_clkinst_bad.out @@ -1,23 +1,23 @@ -%Warning-IMPERFECTSCH: t/t_order_clkinst.v:18:16: Imperfect scheduling of variable: 't.c1_start' - 18 | reg c1_start; initial c1_start = 0; +%Warning-IMPERFECTSCH: t/t_order_clkinst.v:19:16: Imperfect scheduling of variable: 't.c1_start' + 19 | reg c1_start; initial c1_start = 0; | ^~~~~~~~ ... Use "/* verilator lint_off IMPERFECTSCH */" and lint_on around source to disable this message. -%Warning-IMPERFECTSCH: t/t_order_clkinst.v:19:16: Imperfect scheduling of variable: 't.c1_count' - 19 | wire [31:0] c1_count; +%Warning-IMPERFECTSCH: t/t_order_clkinst.v:20:16: Imperfect scheduling of variable: 't.c1_count' + 20 | wire [31:0] c1_count; | ^~~~~~~~ -%Warning-IMPERFECTSCH: t/t_order_clkinst.v:23:16: Imperfect scheduling of variable: 't.s2_count' - 23 | wire [31:0] s2_count; +%Warning-IMPERFECTSCH: t/t_order_clkinst.v:24:16: Imperfect scheduling of variable: 't.s2_count' + 24 | wire [31:0] s2_count; | ^~~~~~~~ -%Warning-IMPERFECTSCH: t/t_order_clkinst.v:27:16: Imperfect scheduling of variable: 't.c3_count' - 27 | wire [31:0] c3_count; +%Warning-IMPERFECTSCH: t/t_order_clkinst.v:28:16: Imperfect scheduling of variable: 't.c3_count' + 28 | wire [31:0] c3_count; | ^~~~~~~~ -%Warning-IMPERFECTSCH: t/t_order_clkinst.v:71:28: Imperfect scheduling of variable: 't.c1.runner' - 71 | reg [31:0] runnerm1, runner; initial runner = 0; +%Warning-IMPERFECTSCH: t/t_order_clkinst.v:72:28: Imperfect scheduling of variable: 't.c1.runner' + 72 | reg [31:0] runnerm1, runner; initial runner = 0; | ^~~~~~ -%Warning-IMPERFECTSCH: t/t_order_clkinst.v:100:28: Imperfect scheduling of variable: 't.s2.runner' - 100 | reg [31:0] runnerm1, runner; initial runner = 0; +%Warning-IMPERFECTSCH: t/t_order_clkinst.v:101:28: Imperfect scheduling of variable: 't.s2.runner' + 101 | reg [31:0] runnerm1, runner; initial runner = 0; | ^~~~~~ -%Warning-IMPERFECTSCH: t/t_order_clkinst.v:71:28: Imperfect scheduling of variable: 't.c3.runner' - 71 | reg [31:0] runnerm1, runner; initial runner = 0; +%Warning-IMPERFECTSCH: t/t_order_clkinst.v:72:28: Imperfect scheduling of variable: 't.c3.runner' + 72 | reg [31:0] runnerm1, runner; initial runner = 0; | ^~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_order_comboclkloop.v b/test_regress/t/t_order_comboclkloop.v index b6d6ac295..373c510bd 100644 --- a/test_regress/t/t_order_comboclkloop.v +++ b/test_regress/t/t_order_comboclkloop.v @@ -12,6 +12,7 @@ module t (/*AUTOARG*/ // verilator lint_off BLKANDNBLK // verilator lint_off COMBDLY + // verilator lint_off LATCH // verilator lint_off UNOPT // verilator lint_off UNOPTFLAT // verilator lint_off MULTIDRIVEN diff --git a/test_regress/t/t_order_comboloop.v b/test_regress/t/t_order_comboloop.v index 9056a08d8..b9b3d2fe3 100644 --- a/test_regress/t/t_order_comboloop.v +++ b/test_regress/t/t_order_comboloop.v @@ -11,6 +11,7 @@ module t (/*AUTOARG*/ input clk; integer cyc; initial cyc=1; + // verilator lint_off LATCH // verilator lint_off UNOPT // verilator lint_off UNOPTFLAT reg [31:0] runner; initial runner = 5; diff --git a/test_regress/t/t_order_doubleloop.v b/test_regress/t/t_order_doubleloop.v index 3b61fbedc..c0a32f4d9 100644 --- a/test_regress/t/t_order_doubleloop.v +++ b/test_regress/t/t_order_doubleloop.v @@ -11,6 +11,7 @@ module t (/*AUTOARG*/ input clk; integer cyc; initial cyc=1; + // verilator lint_off LATCH // verilator lint_off UNOPT // verilator lint_off UNOPTFLAT // verilator lint_off MULTIDRIVEN diff --git a/test_regress/t/t_prot_lib.v b/test_regress/t/t_prot_lib.v index c56a33433..b0195a940 100644 --- a/test_regress/t/t_prot_lib.v +++ b/test_regress/t/t_prot_lib.v @@ -155,7 +155,9 @@ module t #(parameter GATED_CLK = 0) (/*AUTOARG*/ if (GATED_CLK != 0) begin: yes_gated_clock logic clk_en_latch /*verilator clock_enable*/; /* verilator lint_off COMBDLY */ + /* verilator lint_off LATCH */ always_comb if (clk == '0) clk_en_latch <= clk_en; + /* verilator lint_on LATCH */ /* verilator lint_on COMBDLY */ assign possibly_gated_clk = clk & clk_en_latch; end else begin: no_gated_clock diff --git a/test_regress/t/t_prot_lib_secret.v b/test_regress/t/t_prot_lib_secret.v index fa39f529f..c01d0f48e 100644 --- a/test_regress/t/t_prot_lib_secret.v +++ b/test_regress/t/t_prot_lib_secret.v @@ -48,7 +48,9 @@ module secret #(parameter GATED_CLK = 0) if (GATED_CLK != 0) begin: yes_gated_clock logic clk_en_latch /*verilator clock_enable*/; /* verilator lint_off COMBDLY */ + /* verilator lint_off LATCH */ always_comb if (clk == '0) clk_en_latch <= clk_en; + /* verilator lint_on LATCH */ /* verilator lint_on COMBDLY */ assign the_clk = clk & clk_en_latch; end else begin: no_gated_clock