luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add MULTIDRIVEN checks for clocking block outputs (#7987)

This commit is contained in:
Aisha 2026-08-03 03:54:43 -05:00 committed by GitHub
parent daf5826610
commit 29fd2cf90d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 251 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
docs/guide/warnings.rst
View File

@ -1469,6 +1469,13 @@ List Of Warnings
q <= d;
end
A further case is when a signal named as a clocking block ``output`` is
also driven by a continuous assignment, or is named as an ``output`` of a
second clocking block. The clocking block drives the signal, so the design
and the testbench contend for it and the synchronous drive may be silently
lost. Declare the clocking block ``input`` if the intent is only to
observe the signal.
Ignoring this warning may hide clock domain crossing, timing, or
portability bugs. It may also cause longer simulation runtimes due to
reduced optimizations.
@ -1495,6 +1502,12 @@ List Of Warnings
.. include:: ../../docs/gen/ex_MULTIDRIVENPROC_msg.rst
Also warns when a signal named as a clocking block ``output`` is driven
by a plain ``always`` block. Driving a signal from both a clocking block
and a plain ``always`` block is a deliberate idiom in some testbenches,
so it is reported as MULTIDRIVENPROC rather than under the on-by-default
:option:`MULTIDRIVEN`.
To fix, drive the signal from a single ``always`` block, or use
``always_ff``/``always_comb`` if the intent is a single specialized
process.

101
src/V3Undriven.cpp
View File

@ -48,6 +48,7 @@ class UndrivenVarEntry final {
= 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 AstClocking* m_clockingp = nullptr; // clocking block of var if driven as output
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
@ -64,7 +65,8 @@ class UndrivenVarEntry final {
FLAG_DRIVEN_ALWCOMB = 2, // Whole signal has been driven from always_comb
FLAG_DRIVEN_ALWFF = 3, // Whole signal has been driven from always_ff
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
FLAG_DRIVEN_CLOCKING = 5, // Whole signal has been driven as a clocking block output
FLAGS_PER_BIT = 6 // Number of flags stored for each tracked bit
};
public:
@ -164,6 +166,10 @@ public:
m_wholeFlags[FLAG_DRIVEN_ALWPLAIN] = true;
m_alwPlainp = alwPlainp;
}
void drivenClockingWhole(const AstClocking* clockingp) {
m_wholeFlags[FLAG_DRIVEN_CLOCKING] = true;
m_clockingp = clockingp;
}
const AstNode* initStaticp() const { return m_initStaticp; }
void initStaticp(const AstNode* nodep) { m_initStaticp = nodep; }
@ -179,11 +185,13 @@ public:
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 isDrivenClockingWhole() const { return m_wholeFlags[FLAG_DRIVEN_CLOCKING]; }
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; }
const AstClocking* getClockingp() const { return m_clockingp; }
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++) {
@ -384,6 +392,7 @@ class UndrivenVisitor final : public VNVisitorConst {
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)
const AstClocking* m_clockingp = nullptr; // Current clocking block, otherwise nullptr
V3UndrivenCapture* const m_capturep = nullptr; // Capture object. 'nullptr' if disabled.
@ -431,6 +440,84 @@ class UndrivenVisitor final : public VNVisitorConst {
true); // Complain just once for any usage
}
}
// A clocking block 'output' is an additional driver of the signal it names.
// The conflict is found from whichever driver is reached second.
bool clockingDrivesOther(const UndrivenVarEntry* entryp, bool otherWriteIsStaticInit) const {
const bool otherIsExplicitProc
= entryp->isDrivenAlwaysCombWhole() || entryp->isDrivenAlwaysFFWhole();
return m_clockingp && !otherIsExplicitProc && !otherWriteIsStaticInit
&& m_clockingp != entryp->getClockingp();
}
bool otherDrivesClocking(const UndrivenVarEntry* entryp) const {
return !m_clockingp && !m_alwaysCombp && !m_alwaysFFp && !m_inInitialStatic
&& entryp->isDrivenClockingWhole();
}
// A continuous assignment or a second clocking block contending with a
// clocking block 'output' is an unambiguous driver conflict.
void warnClockingDriven(AstNodeVarRef* nodep, const UndrivenVarEntry* entryp,
const AstNode* otherWritep, bool otherWriteIsStaticInit) {
const AstClocking* const otherClockingp = entryp->getClockingp();
if (clockingDrivesOther(entryp, otherWriteIsStaticInit)) {
if (otherClockingp) {
nodep->v3warn(MULTIDRIVEN,
"Variable written to in clocking block also written by another "
"clocking block"
<< " (IEEE 1800-2023 14.3): " << nodep->prettyNameQ() << '\n'
<< nodep->warnOther() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< otherWritep->warnOther()
<< "... Location of other clocking block output\n"
<< otherWritep->warnContextSecondary());
} else if (otherWritep == entryp->contAssignp()) {
nodep->v3warn(MULTIDRIVEN,
"Variable written to in clocking block also driven by continuous "
"assignment"
<< " (IEEE 1800-2023 14.3): " << nodep->prettyNameQ() << '\n'
<< nodep->warnOther() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< otherWritep->warnOther()
<< "... Location of continuous assignment\n"
<< otherWritep->warnContextSecondary());
}
}
if (otherDrivesClocking(entryp) && m_inContAssign) {
nodep->v3warn(MULTIDRIVEN,
"Variable driven by continuous assignment also written to in "
"clocking block"
<< " (IEEE 1800-2023 14.3): " << nodep->prettyNameQ() << '\n'
<< nodep->warnOther() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< otherWritep->warnOther()
<< "... Location of clocking block output\n"
<< otherWritep->warnContextSecondary());
}
}
// Driving a signal from both a clocking block and a plain process is a deliberate
// idiom in some testbenches, as with the plain always conflicts above.
void warnClockingDrivenProc(AstNodeVarRef* nodep, const UndrivenVarEntry* entryp,
const AstNode* otherWritep, bool otherWriteIsStaticInit) {
if (clockingDrivesOther(entryp, otherWriteIsStaticInit) && !entryp->getClockingp()
&& otherWritep != entryp->contAssignp()) {
nodep->v3warn(MULTIDRIVENPROC,
"Variable written to in clocking block also written by another "
"process"
<< " (IEEE 1800-2023 14.3): " << nodep->prettyNameQ() << '\n'
<< nodep->warnOther() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< otherWritep->warnOther() << "... Location of other write\n"
<< otherWritep->warnContextSecondary());
}
if (otherDrivesClocking(entryp) && !m_inContAssign) {
nodep->v3warn(MULTIDRIVENPROC,
"Variable written to in process also written to in clocking block"
<< " (IEEE 1800-2023 14.3): " << nodep->prettyNameQ() << '\n'
<< nodep->warnOther() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< otherWritep->warnOther()
<< "... Location of clocking block output\n"
<< otherWritep->warnContextSecondary());
}
}
// VISITORS
void visit(AstVar* nodep) override {
@ -551,10 +638,10 @@ class UndrivenVisitor final : public VNVisitorConst {
// declaration's fileline, as v3warn suppression will check
// the driving fileline and still warn even if the warning
// was suppressed with lint_off at the declaration.
const bool otherWriteIsStaticInit
= nodep->varp()->hasUserInit() && otherWritep == entryp->initStaticp();
if (multidrivenCommon
&& !nodep->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVEN)) {
const bool otherWriteIsStaticInit
= nodep->varp()->hasUserInit() && otherWritep == entryp->initStaticp();
if (m_alwaysCombp
&& (!entryp->isDrivenAlwaysCombWhole()
|| (m_alwaysCombp != entryp->getAlwCombp()
@ -602,6 +689,7 @@ class UndrivenVisitor final : public VNVisitorConst {
<< "... Location of always_ff write\n"
<< otherWritep->warnContextSecondary());
}
warnClockingDriven(nodep, entryp, otherWritep, otherWriteIsStaticInit);
}
if (multidrivenCommon
&& !nodep->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVENPROC)) {
@ -634,6 +722,7 @@ class UndrivenVisitor final : public VNVisitorConst {
<< otherWritep->warnOther() << "... Location of other write\n"
<< otherWritep->warnContextSecondary());
}
warnClockingDrivenProc(nodep, entryp, otherWritep, otherWriteIsStaticInit);
}
if (!m_inInitialSetup || nodep->varp()->hasUserInit()) {
// Else don't count default initialization as a driver to a net/variable
@ -646,6 +735,7 @@ class UndrivenVisitor final : public VNVisitorConst {
if (m_alwaysCombp) entryp->drivenAlwaysCombWhole(m_alwaysCombp);
if (m_alwaysFFp) entryp->drivenAlwaysFFWhole(m_alwaysFFp, nodep->varp());
if (m_alwaysPlainp) entryp->drivenAlwaysPlainWhole(m_alwaysPlainp);
if (m_clockingp) entryp->drivenClockingWhole(m_clockingp);
}
if (nodep->access().isWriteOrRW() && !VN_IS(nodep, VarXRef)) {
// Ignoring xrefs as the initial and assignment to track might refer to two
@ -751,6 +841,11 @@ class UndrivenVisitor final : public VNVisitorConst {
iterateChildrenConst(nodep);
if (nodep->keyword() == VAlwaysKwd::ALWAYS_COMB) UINFO(9, " Done " << nodep);
}
void visit(AstClocking* nodep) override {
VL_RESTORER(m_clockingp);
m_clockingp = nodep;
iterateChildrenConst(nodep);
}
void visit(AstNodeFTaskRef* nodep) override {
VL_RESTORER(m_inFTaskRef);

45
test_regress/t/t_lint_multidriven_clocking_bad.out Normal file
View File

@ -0,0 +1,45 @@
%Warning-MULTIDRIVEN: t/t_lint_multidriven_clocking_bad.v:21:10: Variable driven by continuous assignment also written to in clocking block (IEEE 1800-2023 14.3): 'early_assign'
: ... note: In instance 't'
t/t_lint_multidriven_clocking_bad.v:21:10:
21 | assign early_assign = 1'b1;
| ^~~~~~~~~~~~
t/t_lint_multidriven_clocking_bad.v:17:12: ... Location of clocking block output
17 | output early_assign;
| ^~~~~~~~~~~~
... 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.
%Warning-MULTIDRIVENPROC: t/t_lint_multidriven_clocking_bad.v:22:25: Variable written to in process also written to in clocking block (IEEE 1800-2023 14.3): 'early_always'
: ... note: In instance 't'
t/t_lint_multidriven_clocking_bad.v:22:25:
22 | always @(posedge clk) early_always <= 1'b1;
| ^~~~~~~~~~~~
t/t_lint_multidriven_clocking_bad.v:18:12: ... Location of clocking block output
18 | output early_always;
| ^~~~~~~~~~~~
... 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_clocking_bad.v:45:12: Variable written to in clocking block also driven by continuous assignment (IEEE 1800-2023 14.3): 'late_assign'
: ... note: In instance 't'
t/t_lint_multidriven_clocking_bad.v:45:12:
45 | output late_assign;
| ^~~~~~~~~~~
t/t_lint_multidriven_clocking_bad.v:27:10: ... Location of continuous assignment
27 | assign late_assign = 1'b1;
| ^~~~~~~~~~~
%Warning-MULTIDRIVENPROC: t/t_lint_multidriven_clocking_bad.v:46:12: Variable written to in clocking block also written by another process (IEEE 1800-2023 14.3): 'late_always'
: ... note: In instance 't'
t/t_lint_multidriven_clocking_bad.v:46:12:
46 | output late_always;
| ^~~~~~~~~~~
t/t_lint_multidriven_clocking_bad.v:32:25: ... Location of other write
32 | always @(posedge clk) late_always <= 1'b1;
| ^~~~~~~~~~~
%Warning-MULTIDRIVEN: t/t_lint_multidriven_clocking_bad.v:53:12: Variable written to in clocking block also written by another clocking block (IEEE 1800-2023 14.3): 'dual_clocking'
: ... note: In instance 't'
t/t_lint_multidriven_clocking_bad.v:53:12:
53 | output dual_clocking;
| ^~~~~~~~~~~~~
t/t_lint_multidriven_clocking_bad.v:47:12: ... Location of other clocking block output
47 | output dual_clocking;
| ^~~~~~~~~~~~~
%Error: Exiting due to

25
test_regress/t/t_lint_multidriven_clocking_bad.py Executable file
View File

@ -0,0 +1,25 @@
#!/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')
# A clocking block 'output' drives the signal it names. Against a continuous
# assignment or a second clocking block that is an unambiguous driver conflict
# (MULTIDRIVEN, on by default); against a plain always block it is a common
# testbench idiom, so it is only reported under MULTIDRIVENPROC, enabled here
# explicitly. Signals driven by a single clocking block, only read by one, named
# by two clockvars of one clocking block, or carrying just a declaration
# initializer must not warn at all.
test.lint(fails=True,
verilator_flags2=['-Wwarn-MULTIDRIVENPROC'],
expect_filename=test.golden_filename)
test.passes()

70
test_regress/t/t_lint_multidriven_clocking_bad.v Normal file
View File

@ -0,0 +1,70 @@
// 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
module t (
input wire clk
);
// Clocking block declared before the drivers it contends with, so the
// conflict is found on reaching the design driver.
logic early_assign;
logic early_always;
clocking cb_early @(posedge clk);
output early_assign;
output early_always;
endclocking
assign early_assign = 1'b1;
always @(posedge clk) early_always <= 1'b1;
// Continuous assignment against a clocking block output: an unambiguous
// driver conflict, reported by MULTIDRIVEN.
logic late_assign;
assign late_assign = 1'b1;
// Plain always block against a clocking block output: a deliberate testbench
// idiom, so reported only by the off-by-default MULTIDRIVENPROC.
logic late_always;
always @(posedge clk) late_always <= 1'b1;
// Driven by two clocking blocks, reported by MULTIDRIVEN.
logic dual_clocking;
// Driven only by a clocking block: legal, must not warn.
logic clocking_only;
// Read by a clocking block rather than driven: legal, must not warn.
logic observed;
always_ff @(posedge clk) observed <= ~observed;
clocking cb_late @(posedge clk);
output late_assign;
output late_always;
output dual_clocking;
output clocking_only;
input observed;
endclocking
clocking cb_dual @(posedge clk);
output dual_clocking;
endclocking
// Declaration initializer alongside a clocking block output: the initializer
// is not a competing driver, must not warn.
logic decl_init = 1'b0;
// One signal named by two clockvars of the same clocking block: still a
// single driver, must not warn.
logic aliased;
clocking cb_alias @(posedge clk);
output decl_init;
output cv_a = aliased;
output cv_b = aliased;
endclocking
endmodule