Add MULTIDRIVENPROC warning for signals driven by multiple plain always blocks (#7968)
This commit is contained in:
parent
bb0300f605
commit
8f8b5ade55
|
|
@ -14,6 +14,7 @@ Adrian Sampson
|
|||
Adrien Le Masle
|
||||
أحمد المحمودي (Ahmed El-Mahmoudy)
|
||||
Aidan McNay
|
||||
Aisha Salimgereyeva
|
||||
Aleksander Kiryk
|
||||
Alex Chadwick
|
||||
Alex Solomatnikov
|
||||
|
|
|
|||
|
|
@ -0,0 +1,6 @@
|
|||
.. comment: generated by t_lint_multidriven_proc_bad
|
||||
.. code-block:: sv
|
||||
:linenos:
|
||||
|
||||
always @(posedge clk) q <= d;
|
||||
always @(posedge clk) q <= ~d;
|
||||
|
|
@ -0,0 +1,8 @@
|
|||
.. comment: generated by t_lint_multidriven_proc_bad
|
||||
.. code-block::
|
||||
|
||||
%Warning-MULTIDRIVENPROC: example.v:1:25 Variable written to in always block also written by another always block: 'q'
|
||||
: ... note: In instance 't'
|
||||
example.v:1:25
|
||||
16 | always @(posedge clk) q <= ~d;
|
||||
| ^
|
||||
|
|
@ -1474,6 +1474,32 @@ List Of Warnings
|
|||
reduced optimizations.
|
||||
|
||||
|
||||
.. option:: MULTIDRIVENPROC
|
||||
|
||||
Warns that the whole of a variable is driven by more than one plain
|
||||
``always`` block. Unlike the :option:`MULTIDRIVEN` cases, plain
|
||||
``always`` blocks carry no ``always_comb``/``always_ff`` intent, so this
|
||||
is legal SystemVerilog rather than an IEEE 1800 violation. It is,
|
||||
however, typically a synthesis error: hardware cannot have a signal
|
||||
driven by two separate sequential blocks, so the design usually will not
|
||||
behave as the RTL simulation suggests.
|
||||
|
||||
Disabled by default as this is a code-style warning; it will simulate
|
||||
correctly.
|
||||
|
||||
Faulty example:
|
||||
|
||||
.. include:: ../../docs/gen/ex_MULTIDRIVENPROC_faulty.rst
|
||||
|
||||
Results in:
|
||||
|
||||
.. include:: ../../docs/gen/ex_MULTIDRIVENPROC_msg.rst
|
||||
|
||||
To fix, drive the signal from a single ``always`` block, or use
|
||||
``always_ff``/``always_comb`` if the intent is a single specialized
|
||||
process.
|
||||
|
||||
|
||||
.. option:: MULTITOP
|
||||
|
||||
.. TODO better example
|
||||
|
|
|
|||
|
|
@ -135,6 +135,7 @@ public:
|
|||
MODDUP, // Duplicate module
|
||||
MODMISSING, // Error: missing module
|
||||
MULTIDRIVEN, // Driven from multiple blocks
|
||||
MULTIDRIVENPROC, // Driven from multiple plain always blocks
|
||||
MULTITOP, // Multiple top level modules
|
||||
NEWERSTD, // Newer language standard required
|
||||
NOEFFECT, // Statement has no effect
|
||||
|
|
@ -233,17 +234,17 @@ public:
|
|||
"IEEEMAYDEPRECATE", "IFDEPTH", "IGNOREDRETURN", "IMPERFECTSCH", "IMPLICIT",
|
||||
"IMPLICITSTATIC", "IMPORTSTAR", "IMPURE", "INCABSPATH", "INFINITELOOP", "INITIALDLY",
|
||||
"INSECURE", "INSIDETRUE", "LATCH", "LITENDIAN", "MINTYPMAXDLY", "MISINDENT", "MODDUP",
|
||||
"MODMISSING", "MULTIDRIVEN", "MULTITOP", "NEWERSTD", "NOEFFECT", "NOLATCH", "NONSTD",
|
||||
"NORETURN", "NOTREDOP", "NULLPORT", "PARAMNODEFAULT", "PINCONNECTEMPTY", "PINMISSING",
|
||||
"PINNOCONNECT", "PINNOTFOUND", "PKGNODECL", "PREPROCZERO", "PROCASSINIT",
|
||||
"PROCASSWIRE", "PROFOUTOFDATE", "PROTECTED", "PROTOTYPEMIS", "RANDC", "REALCVT",
|
||||
"REDEFMACRO", "RISEFALLDLY", "SELRANGE", "SHORTREAL", "SIDEEFFECT", "SPECIFYIGN",
|
||||
"SPLITVAR", "STATICVAR", "STMTDLY", "SUPERNFIRST", "SYMRSVDWORD", "SYNCASYNCNET",
|
||||
"TICKCOUNT", "TIMESCALEMOD", "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNOPTTHREADS",
|
||||
"UNPACKED", "UNSATCONSTR", "UNSIGNED", "UNUSED", "UNUSEDGENVAR", "UNUSEDLOOP",
|
||||
"UNUSEDPARAM", "UNUSEDSIGNAL", "USERERROR", "USERFATAL", "USERINFO", "USERWARN",
|
||||
"VARHIDDEN", "WAITCONST", "WIDTH", "WIDTHCONCAT", "WIDTHEXPAND", "WIDTHTRUNC",
|
||||
"WIDTHXZEXPAND", "ZERODLY", "ZEROREPL", " MAX"};
|
||||
"MODMISSING", "MULTIDRIVEN", "MULTIDRIVENPROC", "MULTITOP", "NEWERSTD", "NOEFFECT",
|
||||
"NOLATCH", "NONSTD", "NORETURN", "NOTREDOP", "NULLPORT", "PARAMNODEFAULT",
|
||||
"PINCONNECTEMPTY", "PINMISSING", "PINNOCONNECT", "PINNOTFOUND", "PKGNODECL",
|
||||
"PREPROCZERO", "PROCASSINIT", "PROCASSWIRE", "PROFOUTOFDATE", "PROTECTED",
|
||||
"PROTOTYPEMIS", "RANDC", "REALCVT", "REDEFMACRO", "RISEFALLDLY", "SELRANGE",
|
||||
"SHORTREAL", "SIDEEFFECT", "SPECIFYIGN", "SPLITVAR", "STATICVAR", "STMTDLY",
|
||||
"SUPERNFIRST", "SYMRSVDWORD", "SYNCASYNCNET", "TICKCOUNT", "TIMESCALEMOD", "UNDRIVEN",
|
||||
"UNOPT", "UNOPTFLAT", "UNOPTTHREADS", "UNPACKED", "UNSATCONSTR", "UNSIGNED", "UNUSED",
|
||||
"UNUSEDGENVAR", "UNUSEDLOOP", "UNUSEDPARAM", "UNUSEDSIGNAL", "USERERROR", "USERFATAL",
|
||||
"USERINFO", "USERWARN", "VARHIDDEN", "WAITCONST", "WIDTH", "WIDTHCONCAT",
|
||||
"WIDTHEXPAND", "WIDTHTRUNC", "WIDTHXZEXPAND", "ZERODLY", "ZEROREPL", " MAX"};
|
||||
return names[m_e];
|
||||
}
|
||||
// Warnings that default to off
|
||||
|
|
@ -296,7 +297,8 @@ public:
|
|||
return (m_e == ASSIGNDLY // More than style, but for backward compatibility
|
||||
|| m_e == BLKSEQ || m_e == DECLFILENAME || m_e == DEFPARAM || m_e == EOFNEWLINE
|
||||
|| m_e == GENUNNAMED || m_e == IMPORTSTAR || m_e == INCABSPATH
|
||||
|| m_e == PINCONNECTEMPTY || m_e == PINNOCONNECT || m_e == PROCASSINIT
|
||||
|| m_e == MULTIDRIVENPROC || m_e == PINCONNECTEMPTY || m_e == PINNOCONNECT
|
||||
|| m_e == PROCASSINIT
|
||||
|| m_e == SYNCASYNCNET || m_e == UNDRIVEN || m_e == UNUSEDGENVAR
|
||||
|| m_e == UNUSEDLOOP || m_e == UNUSEDPARAM || m_e == UNUSEDSIGNAL
|
||||
|| m_e == VARHIDDEN);
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ class UndrivenVarEntry final {
|
|||
const AstAlways* m_alwCombp
|
||||
= nullptr; // always_comb of var if driven within always_comb, else nullptr
|
||||
const AstAlways* m_alwFFp = nullptr; // always_ff of var if driven within always_ff
|
||||
const AstAlways* m_alwPlainp = nullptr; // plain always of var if driven within plain always
|
||||
const AstNodeVarRef* m_nodep = nullptr; // varref if driven, else nullptr
|
||||
const AstNode* m_initStaticp = nullptr; // varref if in InitialStatic driven
|
||||
const AstNode* m_initialp = nullptr; // varref if driven in an explicit initial block
|
||||
|
|
@ -62,7 +63,8 @@ class UndrivenVarEntry final {
|
|||
FLAG_DRIVEN = 1, // Signal or bit has been written/driven
|
||||
FLAG_DRIVEN_ALWCOMB = 2, // Whole signal has been driven from always_comb
|
||||
FLAG_DRIVEN_ALWFF = 3, // Whole signal has been driven from always_ff
|
||||
FLAGS_PER_BIT = 4 // Number of flags stored for each tracked bit
|
||||
FLAG_DRIVEN_ALWPLAIN = 4, // Whole signal has been driven from a plain always
|
||||
FLAGS_PER_BIT = 5 // Number of flags stored for each tracked bit
|
||||
};
|
||||
|
||||
public:
|
||||
|
|
@ -158,6 +160,10 @@ public:
|
|||
m_wholeFlags[FLAG_DRIVEN_ALWFF] = true;
|
||||
m_alwFFp = alwFFp;
|
||||
}
|
||||
void drivenAlwaysPlainWhole(const AstAlways* alwPlainp) {
|
||||
m_wholeFlags[FLAG_DRIVEN_ALWPLAIN] = true;
|
||||
m_alwPlainp = alwPlainp;
|
||||
}
|
||||
|
||||
const AstNode* initStaticp() const { return m_initStaticp; }
|
||||
void initStaticp(const AstNode* nodep) { m_initStaticp = nodep; }
|
||||
|
|
@ -172,10 +178,12 @@ public:
|
|||
bool isDrivenWhole() const { return m_wholeFlags[FLAG_DRIVEN]; }
|
||||
bool isDrivenAlwaysCombWhole() const { return m_wholeFlags[FLAG_DRIVEN_ALWCOMB]; }
|
||||
bool isDrivenAlwaysFFWhole() const { return m_wholeFlags[FLAG_DRIVEN_ALWFF]; }
|
||||
bool isDrivenAlwaysPlainWhole() const { return m_wholeFlags[FLAG_DRIVEN_ALWPLAIN]; }
|
||||
bool isFtaskDriven() const { return m_ftaskDriven; }
|
||||
const AstNodeVarRef* getNodep() const { return m_nodep; }
|
||||
const AstAlways* getAlwCombp() const { return m_alwCombp; }
|
||||
const AstAlways* getAlwFFp() const { return m_alwFFp; }
|
||||
const AstAlways* getAlwPlainp() const { return m_alwPlainp; }
|
||||
void usedBit(int bit, int width, const AstNode* nodep) {
|
||||
UINFO(9, "set u[" << (bit + width - 1) << ":" << bit << "] " << m_varp->name());
|
||||
for (int i = 0; i < width; i++) {
|
||||
|
|
@ -375,6 +383,7 @@ class UndrivenVisitor final : public VNVisitorConst {
|
|||
const AstAlways* m_alwaysp = nullptr; // Current always of either type
|
||||
const AstAlways* m_alwaysCombp = nullptr; // Current always if combo, otherwise nullptr
|
||||
const AstAlways* m_alwaysFFp = nullptr; // Current always if ff, otherwise nullptr
|
||||
const AstAlways* m_alwaysPlainp = nullptr; // Current always if plain (not comb/ff/latch)
|
||||
|
||||
V3UndrivenCapture* const m_capturep = nullptr; // Capture object. 'nullptr' if disabled.
|
||||
|
||||
|
|
@ -536,7 +545,8 @@ class UndrivenVisitor final : public VNVisitorConst {
|
|||
&& !VN_IS(nodep->dtypep()->skipRefp(), UnpackArrayDType) && !sameFileLine
|
||||
&& !entryp->isUnderGen() && otherWritep && !entryp->isFtaskDriven()
|
||||
&& !ftaskDef && !m_inSelLhs
|
||||
&& !nodep->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVEN)) {
|
||||
&& (!nodep->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVEN)
|
||||
|| !nodep->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVENPROC))) {
|
||||
const bool otherWriteIsStaticInit
|
||||
= nodep->varp()->hasUserInit() && otherWritep == entryp->initStaticp();
|
||||
|
||||
|
|
@ -587,6 +597,35 @@ class UndrivenVisitor final : public VNVisitorConst {
|
|||
<< "... Location of always_ff write\n"
|
||||
<< otherWritep->warnContextSecondary());
|
||||
}
|
||||
// Two plain always blocks driving the whole signal: legal
|
||||
// SystemVerilog, but a driver conflict for synthesis. The
|
||||
// always_ff/always_comb cases above already cover mixes with
|
||||
// an explicit process, so only warn for plain+plain here.
|
||||
// When the two blocks are clocked differently, the
|
||||
// (on-by-default) MULTIDRIVEN check in V3Delayed already reports
|
||||
// the conflict, so don't also emit MULTIDRIVENPROC for that case.
|
||||
const AstAlways* const otherAlwaysp = entryp->getAlwPlainp();
|
||||
const AstSenTree* const senp
|
||||
= m_alwaysPlainp ? m_alwaysPlainp->sentreep() : nullptr;
|
||||
const AstSenTree* const otherSenp
|
||||
= otherAlwaysp ? otherAlwaysp->sentreep() : nullptr;
|
||||
const bool differentClocking = senp && otherSenp && senp->hasClocked()
|
||||
&& otherSenp->hasClocked()
|
||||
&& !senp->sameTree(otherSenp);
|
||||
if (m_alwaysPlainp && entryp->isDrivenAlwaysPlainWhole()
|
||||
&& m_alwaysPlainp != otherAlwaysp
|
||||
&& m_alwaysPlainp->fileline() != otherAlwaysp->fileline()
|
||||
&& !differentClocking) {
|
||||
nodep->v3warn(
|
||||
MULTIDRIVENPROC,
|
||||
"Variable written to in always block also written by another always "
|
||||
"block: "
|
||||
<< nodep->prettyNameQ() << '\n'
|
||||
<< nodep->warnOther() << '\n'
|
||||
<< nodep->warnContextPrimary() << '\n'
|
||||
<< otherWritep->warnOther() << "... Location of other write\n"
|
||||
<< otherWritep->warnContextSecondary());
|
||||
}
|
||||
}
|
||||
if (!m_inInitialSetup || nodep->varp()->hasUserInit()) {
|
||||
// Else don't count default initialization as a driver to a net/variable
|
||||
|
|
@ -598,6 +637,7 @@ class UndrivenVisitor final : public VNVisitorConst {
|
|||
entryp->underGenerate();
|
||||
if (m_alwaysCombp) entryp->drivenAlwaysCombWhole(m_alwaysCombp);
|
||||
if (m_alwaysFFp) entryp->drivenAlwaysFFWhole(m_alwaysFFp, nodep->varp());
|
||||
if (m_alwaysPlainp) entryp->drivenAlwaysPlainWhole(m_alwaysPlainp);
|
||||
}
|
||||
if (nodep->access().isWriteOrRW() && !VN_IS(nodep, VarXRef)) {
|
||||
// Ignoring xrefs as the initial and assignment to track might refer to two
|
||||
|
|
@ -689,6 +729,7 @@ class UndrivenVisitor final : public VNVisitorConst {
|
|||
VL_RESTORER(m_alwaysp);
|
||||
VL_RESTORER(m_alwaysCombp);
|
||||
VL_RESTORER(m_alwaysFFp);
|
||||
VL_RESTORER(m_alwaysPlainp);
|
||||
AstNode::user2ClearTree();
|
||||
m_alwaysp = nodep;
|
||||
if (nodep->keyword() == VAlwaysKwd::ALWAYS_COMB) {
|
||||
|
|
@ -698,6 +739,7 @@ class UndrivenVisitor final : public VNVisitorConst {
|
|||
m_alwaysCombp = nullptr;
|
||||
}
|
||||
m_alwaysFFp = nodep->keyword() == VAlwaysKwd::ALWAYS_FF ? nodep : nullptr;
|
||||
m_alwaysPlainp = nodep->keyword() == VAlwaysKwd::ALWAYS ? nodep : nullptr;
|
||||
iterateChildrenConst(nodep);
|
||||
if (nodep->keyword() == VAlwaysKwd::ALWAYS_COMB) UINFO(9, " Done " << nodep);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,20 @@
|
|||
%Warning-MULTIDRIVENPROC: t/t_lint_multidriven_proc_bad.v:16:25: Variable written to in always block also written by another always block: 'q'
|
||||
: ... note: In instance 't'
|
||||
t/t_lint_multidriven_proc_bad.v:16:25:
|
||||
16 | always @(posedge clk) q <= ~d;
|
||||
| ^
|
||||
t/t_lint_multidriven_proc_bad.v:15:25: ... Location of other write
|
||||
15 | always @(posedge clk) q <= d;
|
||||
| ^
|
||||
... For warning description see https://verilator.org/warn/MULTIDRIVENPROC?v=latest
|
||||
... Use "/* verilator lint_off MULTIDRIVENPROC */" and lint_on around source to disable this message.
|
||||
%Warning-MULTIDRIVEN: t/t_lint_multidriven_proc_bad.v:26:18: Signal has multiple driving blocks with different clocking: 't2.q'
|
||||
t/t_lint_multidriven_proc_bad.v:29:26: ... Location of first driving block
|
||||
29 | always @(posedge clka) q <= d;
|
||||
| ^
|
||||
t/t_lint_multidriven_proc_bad.v:30:26: ... Location of other driving block
|
||||
30 | always @(posedge clkb) q <= ~d;
|
||||
| ^
|
||||
... For warning description see https://verilator.org/warn/MULTIDRIVEN?v=latest
|
||||
... Use "/* verilator lint_off MULTIDRIVEN */" and lint_on around source to disable this message.
|
||||
%Error: Exiting due to
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
#!/usr/bin/env python3
|
||||
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
|
||||
#
|
||||
# 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-FileCopyrightText: 2026 Wilson Snyder
|
||||
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
|
||||
|
||||
import vltest_bootstrap
|
||||
|
||||
test.scenarios('linter')
|
||||
|
||||
# MULTIDRIVENPROC is off by default; enable it explicitly. Module 't' (same
|
||||
# clock) triggers it; module 't2' (different clocks) is reported by MULTIDRIVEN
|
||||
# instead, confirming MULTIDRIVENPROC is suppressed there.
|
||||
# -Wno-MULTITOP: both modules are top-level here on purpose (two independent
|
||||
# demonstrations), so silence the unrelated multiple-top-module warning.
|
||||
test.lint(fails=True, verilator_flags2=['-Wwarn-MULTIDRIVENPROC', '-Wno-MULTITOP'],
|
||||
expect_filename=test.golden_filename)
|
||||
|
||||
test.extract(in_filename=test.top_filename,
|
||||
out_filename=test.root + "/docs/gen/ex_MULTIDRIVENPROC_faulty.rst",
|
||||
lines="15-16")
|
||||
|
||||
test.extract(in_filename=test.golden_filename,
|
||||
out_filename=test.root + "/docs/gen/ex_MULTIDRIVENPROC_msg.rst",
|
||||
lines="1-5")
|
||||
|
||||
test.passes()
|
||||
|
|
@ -0,0 +1,32 @@
|
|||
// DESCRIPTION: Verilator: Verilog Test module
|
||||
//
|
||||
// This file ONLY is placed under the Creative Commons Public Domain.
|
||||
// SPDX-FileCopyrightText: 2026 Aisha Salimgereyeva
|
||||
// SPDX-License-Identifier: CC0-1.0
|
||||
|
||||
// Same clock: two plain always blocks drive the whole of 'q', reported by
|
||||
// MULTIDRIVENPROC.
|
||||
module t (
|
||||
input wire clk,
|
||||
input wire d,
|
||||
output logic q
|
||||
);
|
||||
|
||||
always @(posedge clk) q <= d;
|
||||
always @(posedge clk) q <= ~d;
|
||||
|
||||
endmodule
|
||||
|
||||
// Different clocks: reported by MULTIDRIVEN (on by default). MULTIDRIVENPROC is
|
||||
// suppressed here so the conflict is reported only once.
|
||||
module t2 (
|
||||
input wire clka,
|
||||
input wire clkb,
|
||||
input wire d,
|
||||
output logic q
|
||||
);
|
||||
|
||||
always @(posedge clka) q <= d;
|
||||
always @(posedge clkb) q <= ~d;
|
||||
|
||||
endmodule
|
||||
Loading…
Reference in New Issue