From e0e0fb08a55b09a449a6c5813ffd0a570c42f5bf Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 21 Sep 2025 13:02:50 -0400 Subject: [PATCH] Fix missing BLKSEQ when connecting module port to array (#2973). --- Changes | 1 + src/V3Active.cpp | 22 ------ src/V3WidthCommit.cpp | 28 +++++++- test_regress/t/t_delay_stmtdly_bad.out | 13 ++-- test_regress/t/t_lint_blkseq_bad.out | 30 +++++--- test_regress/t/t_lint_blkseq_bad.v | 95 +++++++++++++++++--------- 6 files changed, 118 insertions(+), 71 deletions(-) diff --git a/Changes b/Changes index 8ae8a6269..f78996e05 100644 --- a/Changes +++ b/Changes @@ -30,6 +30,7 @@ Verilator 5.041 devel * Improve DFG variable removal and temporary insertion (#6401). [Geza Lore] * Optimize dead functions in more cases (#6380) (#6430). [Artur Bieniek, Antmicro Ltd.] * Optimize constant folding in wide expression expansion (#6381). [Geza Lore] +* Fix missing BLKSEQ when connecting module port to array (#2973). * Fix false CONSTVAR error on initializers (#4992). * Fix cell scoping performance (#6059). [Jerry Tianchen] * Fix hierarchical `--prof-pgo` (#6213). [Bartłomiej Chmiel, Antmicro Ltd.] diff --git a/src/V3Active.cpp b/src/V3Active.cpp index e71f68382..7e31b4ea4 100644 --- a/src/V3Active.cpp +++ b/src/V3Active.cpp @@ -384,28 +384,6 @@ private: VL_DO_DANGLING(nodep->deleteTree(), nodep); } - void visit(AstAssign* nodep) override { - // Blocking assignments are always OK in combinational (and initial/final) processes - if (m_check != CT_SEQ) return; - - const bool ignore = nodep->lhsp()->forall([&](const AstVarRef* refp) { - // Ignore reads (e.g.: index expressions) - if (refp->access().isReadOnly()) return true; - const AstVar* const varp = refp->varp(); - // Ignore ... - return varp->isUsedLoopIdx() // ... loop indices - || varp->isTemp() // ... temporaries - || varp->fileline()->warnIsOff(V3ErrorCode::BLKSEQ); // ... user said so - }); - - if (ignore) return; - - nodep->v3warn(BLKSEQ, - "Blocking assignment '=' in sequential logic process\n" - << nodep->warnMore() // - << "... Suggest using delayed assignment '<='"); - } - //-------------------- void visit(AstNode* nodep) override { iterateChildren(nodep); } diff --git a/src/V3WidthCommit.cpp b/src/V3WidthCommit.cpp index 8d826e1a7..bc75e3e0d 100644 --- a/src/V3WidthCommit.cpp +++ b/src/V3WidthCommit.cpp @@ -50,7 +50,8 @@ class WidthCommitVisitor final : public VNVisitor { bool m_dynsizedelem = false; // Writing a dynamically-sized array element, not the array itself VMemberMap m_memberMap; // Member names cached for fast lookup - bool m_underSel = false; // Whether is currently under AstMemberSel or AstSel + bool m_underSel = false; // Under AstMemberSel or AstSel + bool m_underAlwaysClocked = false; // Under always with sequential SenTree std::vector m_virtualNewsp; // Instantiations of virtual classes std::vector m_tasksp; // All the tasks, we will check if they are ever called @@ -208,6 +209,9 @@ private: return; } } + VL_RESTORER(m_underAlwaysClocked); + m_underAlwaysClocked + = nodep->sentreep() && nodep->sentreep()->sensesp() && nodep->sentreep()->hasClocked(); // Iterate will delete ComboStar sentrees, so after above iterateChildren(nodep); editDType(nodep); @@ -369,6 +373,28 @@ private: classEncapCheck(nodep, nodep->varp(), VN_CAST(nodep->classOrPackagep(), Class)); if (nodep->access().isWriteOrRW()) varLifetimeCheck(nodep, nodep->varp()); } + void visit(AstAssign* nodep) override { + iterateChildren(nodep); + editDType(nodep); + // Lint + if (m_underAlwaysClocked) { + const bool ignore = nodep->lhsp()->forall([&](const AstVarRef* refp) { + // Ignore reads (e.g.: index expressions) + if (refp->access().isReadOnly()) return true; + const AstVar* const varp = refp->varp(); + // Ignore ... + return varp->isUsedLoopIdx() // ... loop indices + || varp->isTemp() // ... temporaries + || varp->fileline()->warnIsOff(V3ErrorCode::BLKSEQ); // ... user said so + }); + if (!ignore) { + nodep->v3warn(BLKSEQ, + "Blocking assignment '=' in sequential logic process\n" + << nodep->warnMore() // + << "... Suggest using delayed assignment '<='"); + } + } + } void visit(AstAssignDly* nodep) override { iterateAndNextNull(nodep->timingControlp()); iterateAndNextNull(nodep->rhsp()); diff --git a/test_regress/t/t_delay_stmtdly_bad.out b/test_regress/t/t_delay_stmtdly_bad.out index 7a41e39a7..4622e46ce 100644 --- a/test_regress/t/t_delay_stmtdly_bad.out +++ b/test_regress/t/t_delay_stmtdly_bad.out @@ -30,6 +30,13 @@ | ^ ... For warning description see https://verilator.org/warn/STMTDLY?v=latest ... Use "/* verilator lint_off STMTDLY */" and lint_on around source to disable this message. +%Warning-BLKSEQ: t/t_delay.v:45:20: Blocking assignment '=' in sequential logic process + : ... note: In instance 't' + : ... Suggest using delayed assignment '<=' + 45 | dly_s.dly = 55; + | ^ + ... For warning description see https://verilator.org/warn/BLKSEQ?v=latest + ... Use "/* verilator lint_off BLKSEQ */" and lint_on around source to disable this message. %Warning-UNUSEDSIGNAL: t/t_delay.v:21:16: Signal is not used: 'dly4' : ... note: In instance 't' 21 | wire [31:0] dly4; @@ -44,10 +51,4 @@ : ... note: In instance 't.sub' 59 | realtime delay = 2.3; | ^~~~~ -%Warning-BLKSEQ: t/t_delay.v:45:20: Blocking assignment '=' in sequential logic process - : ... Suggest using delayed assignment '<=' - 45 | dly_s.dly = 55; - | ^ - ... For warning description see https://verilator.org/warn/BLKSEQ?v=latest - ... Use "/* verilator lint_off BLKSEQ */" and lint_on around source to disable this message. %Error: Exiting due to diff --git a/test_regress/t/t_lint_blkseq_bad.out b/test_regress/t/t_lint_blkseq_bad.out index fad072836..6fc385e82 100644 --- a/test_regress/t/t_lint_blkseq_bad.out +++ b/test_regress/t/t_lint_blkseq_bad.out @@ -1,17 +1,29 @@ -%Warning-BLKSEQ: t/t_lint_blkseq_bad.v:24:16: Blocking assignment '=' in sequential logic process +%Warning-BLKSEQ: t/t_lint_blkseq_bad.v:22:14: Blocking assignment '=' in sequential logic process + : ... note: In instance 't' : ... Suggest using delayed assignment '<=' - 24 | sync_blk = 1'b1; - | ^ + 22 | sync_blk = 1'b1; + | ^ ... For warning description see https://verilator.org/warn/BLKSEQ?v=latest ... Use "/* verilator lint_off BLKSEQ */" and lint_on around source to disable this message. -%Warning-BLKSEQ: t/t_lint_blkseq_bad.v:25:17: Blocking assignment '=' in sequential logic process +%Warning-BLKSEQ: t/t_lint_blkseq_bad.v:23:15: Blocking assignment '=' in sequential logic process + : ... note: In instance 't' : ... Suggest using delayed assignment '<=' - 25 | sync_blk2 = 1'b1; - | ^ -%Warning-COMBDLY: t/t_lint_blkseq_bad.v:31:18: Non-blocking assignment '<=' in combinational logic process + 23 | sync_blk2 = 1'b1; + | ^ +%Warning-BLKSEQ: t/t_lint_blkseq_bad.v:66:29: Blocking assignment '=' in sequential logic process + : ... note: In instance 't.a' + : ... Suggest using delayed assignment '<=' + 66 | always @(posedge clk) out = 1; + | ^ +%Warning-BLKSEQ: t/t_lint_blkseq_bad.v:75:29: Blocking assignment '=' in sequential logic process + : ... note: In instance 't.b' + : ... Suggest using delayed assignment '<=' + 75 | always @(posedge clk) out = 1; + | ^ +%Warning-COMBDLY: t/t_lint_blkseq_bad.v:29:16: Non-blocking assignment '<=' in combinational logic process : ... This will be executed as a blocking assignment '='! - 31 | combo_nblk <= 1'b1; - | ^~ + 29 | combo_nblk <= 1'b1; + | ^~ ... For warning description see https://verilator.org/warn/COMBDLY?v=latest ... Use "/* verilator lint_off COMBDLY */" and lint_on around source to disable this message. *** See https://verilator.org/warn/COMBDLY?v=latest before disabling this, diff --git a/test_regress/t/t_lint_blkseq_bad.v b/test_regress/t/t_lint_blkseq_bad.v index 839b0db6b..380176e1a 100644 --- a/test_regress/t/t_lint_blkseq_bad.v +++ b/test_regress/t/t_lint_blkseq_bad.v @@ -4,45 +4,74 @@ // any use, without warranty, 2010 by Wilson Snyder. // SPDX-License-Identifier: CC0-1.0 -module t (/*AUTOARG*/ - // Inputs - clk - ); - input clk; +module t ( + input clk +); - integer i; + integer i; - reg sync_blk; - reg sync_blk2; - reg sync_nblk; - reg sync2_ok; - reg sync3_ok; - reg combo_blk; - reg combo_nblk; + reg sync_blk; + reg sync_blk2; + reg sync_nblk; + reg sync2_ok; + reg sync3_ok; + reg combo_blk; + reg combo_nblk; - always @(posedge clk) begin - sync_blk = 1'b1; - sync_blk2 = 1'b1; // Only warn once per block - sync_nblk <= 1'b1; - end + always @(posedge clk) begin + sync_blk = 1'b1; + sync_blk2 = 1'b1; // Only warn once per block + sync_nblk <= 1'b1; + end - always_comb begin - combo_blk = 1'b1; - combo_nblk <= 1'b1; - end + always_comb begin + combo_blk = 1'b1; + combo_nblk <= 1'b1; + end - always @(posedge clk) begin - for (int i=0; i<20; i++) begin - sync2_ok <= 1'b1; - end - end + always @(posedge clk) begin + for (int i = 0; i < 20; i++) begin + sync2_ok <= 1'b1; + end + end - always @(posedge clk) begin - sync3_ok <= f(sync3_ok); - end + always @(posedge clk) begin + sync3_ok <= f(sync3_ok); + end - function f (input v); - f = ~v; - endfunction + function f(input v); + f = ~v; + endfunction + + logic single; + logic array[1:0]; + + DoesBlockingAssignA a ( + .clk(clk), + .out(single) + ); + + DoesBlockingAssignB b ( + .clk(clk), + .out(array[0]) + ); + +endmodule + +module DoesBlockingAssignA ( + input wire clk, + output reg out +); + + always @(posedge clk) out = 1; + +endmodule + +module DoesBlockingAssignB ( + input wire clk, + output reg out +); + + always @(posedge clk) out = 1; endmodule