From 8f8b5ade550925dc6e68b6efc2d87772bf758896 Mon Sep 17 00:00:00 2001 From: Aisha <87562264+aishasalim@users.noreply.github.com> Date: Fri, 24 Jul 2026 07:54:09 -0500 Subject: [PATCH] Add MULTIDRIVENPROC warning for signals driven by multiple plain always blocks (#7968) --- docs/CONTRIBUTORS | 1 + docs/gen/ex_MULTIDRIVENPROC_faulty.rst | 6 +++ docs/gen/ex_MULTIDRIVENPROC_msg.rst | 8 ++++ docs/guide/warnings.rst | 26 +++++++++++ src/V3Error.h | 26 ++++++----- src/V3Undriven.cpp | 46 ++++++++++++++++++- .../t/t_lint_multidriven_proc_bad.out | 20 ++++++++ test_regress/t/t_lint_multidriven_proc_bad.py | 30 ++++++++++++ test_regress/t/t_lint_multidriven_proc_bad.v | 32 +++++++++++++ 9 files changed, 181 insertions(+), 14 deletions(-) create mode 100644 docs/gen/ex_MULTIDRIVENPROC_faulty.rst create mode 100644 docs/gen/ex_MULTIDRIVENPROC_msg.rst create mode 100644 test_regress/t/t_lint_multidriven_proc_bad.out create mode 100644 test_regress/t/t_lint_multidriven_proc_bad.py create mode 100644 test_regress/t/t_lint_multidriven_proc_bad.v diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index 7191a19ea..f7881b717 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -14,6 +14,7 @@ Adrian Sampson Adrien Le Masle أحمد المحمودي (Ahmed El-Mahmoudy) Aidan McNay +Aisha Salimgereyeva Aleksander Kiryk Alex Chadwick Alex Solomatnikov diff --git a/docs/gen/ex_MULTIDRIVENPROC_faulty.rst b/docs/gen/ex_MULTIDRIVENPROC_faulty.rst new file mode 100644 index 000000000..1a331b66e --- /dev/null +++ b/docs/gen/ex_MULTIDRIVENPROC_faulty.rst @@ -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; diff --git a/docs/gen/ex_MULTIDRIVENPROC_msg.rst b/docs/gen/ex_MULTIDRIVENPROC_msg.rst new file mode 100644 index 000000000..38b7fffd8 --- /dev/null +++ b/docs/gen/ex_MULTIDRIVENPROC_msg.rst @@ -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; + | ^ diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index ce3a0d58c..04a955e9c 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -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 diff --git a/src/V3Error.h b/src/V3Error.h index 01e7d6447..d5649f646 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -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); diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp index 0ac631d3b..3c9397bd9 100644 --- a/src/V3Undriven.cpp +++ b/src/V3Undriven.cpp @@ -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); } diff --git a/test_regress/t/t_lint_multidriven_proc_bad.out b/test_regress/t/t_lint_multidriven_proc_bad.out new file mode 100644 index 000000000..014ca8a31 --- /dev/null +++ b/test_regress/t/t_lint_multidriven_proc_bad.out @@ -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 diff --git a/test_regress/t/t_lint_multidriven_proc_bad.py b/test_regress/t/t_lint_multidriven_proc_bad.py new file mode 100644 index 000000000..b9dbac65c --- /dev/null +++ b/test_regress/t/t_lint_multidriven_proc_bad.py @@ -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() diff --git a/test_regress/t/t_lint_multidriven_proc_bad.v b/test_regress/t/t_lint_multidriven_proc_bad.v new file mode 100644 index 000000000..dafc8774e --- /dev/null +++ b/test_regress/t/t_lint_multidriven_proc_bad.v @@ -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