diff --git a/Changes b/Changes index acac7dee0..ad4f5463c 100644 --- a/Changes +++ b/Changes @@ -16,6 +16,7 @@ Verilator 4.221 devel * Split --prof-threads into --prof-exec and --prof-pgo (#3365). [Geza Lore, Shunyao CAD] * Deprecate 'vluint64_t' and similar types (#3255). * Raise error on assignment to const in initial blocks. [Geza Lore, Shunyao CAD] +* Issue INITIALDLY/COMBDLY/BLKSEQ warnings consistent with Verilator execution. [Geza Lore, Shunyao CAD] * Fix MSVC localtime_s (#3124). * Fix Bison 3.8.2 error (#3366). [elike-ypq] * Fix rare bug in -Oz (V3Localize) (#3286). [Geza Lore, Shunyao CAD] diff --git a/src/V3Active.cpp b/src/V3Active.cpp index a0ed963ef..9f1758bbb 100644 --- a/src/V3Active.cpp +++ b/src/V3Active.cpp @@ -321,75 +321,70 @@ public: }; //###################################################################### -// Active AssignDly replacement functions +// Replace unsupported non-blocking assignments with blocking assignments class ActiveDlyVisitor final : public ActiveBaseVisitor { public: - enum CheckType : uint8_t { CT_SEQ, CT_COMBO, CT_INITIAL, CT_LATCH }; + enum CheckType : uint8_t { CT_SEQ, CT_COMB, CT_INITIAL }; private: - const CheckType m_check; // Combo logic or other - const AstNode* const m_alwaysp; // Always we're under - const AstNode* m_assignp = nullptr; // In assign + // MEMBERS + const CheckType m_check; // Process type we are checking + // VISITORS virtual void visit(AstAssignDly* nodep) override { - if (m_check != CT_SEQ) { - // Convert to a non-delayed assignment - UINFO(5, " ASSIGNDLY " << nodep << endl); - if (m_check == CT_INITIAL) { - nodep->v3warn(INITIALDLY, "Delayed assignments (<=) in initial or final block\n" - << nodep->warnMore() - << "... Suggest blocking assignments (=)"); - } else if (m_check == CT_LATCH) { - // Suppress. Shouldn't matter that the interior of the latch races - } else if (!(VN_IS(nodep->lhsp(), VarRef) - && VN_AS(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* const newp = new AstAssign(nodep->fileline(), nodep->lhsp()->unlinkFrBack(), - nodep->rhsp()->unlinkFrBack()); - nodep->replaceWith(newp); - VL_DO_DANGLING(nodep->deleteTree(), nodep); + // Non-blocking assignments are OK in sequential processes + if (m_check == CT_SEQ) return; + + // Issue appropriate warning + if (m_check == CT_INITIAL) { + nodep->v3warn(INITIALDLY, + "Non-blocking assignment '<=' in initial/final block\n" + << nodep->warnMore() + << "... This will be executed as a blocking assignment '='!"); + } else { + nodep->v3warn(COMBDLY, + "Non-blocking assignment '<=' in combinational logic process\n" + << nodep->warnMore() + << "... This will be executed as a blocking assignment '='!"); } + + // Convert to blocking assignment + nodep->replaceWith(new AstAssign{nodep->fileline(), // + nodep->lhsp()->unlinkFrBack(), // + nodep->rhsp()->unlinkFrBack()}); + VL_DO_DANGLING(nodep->deleteTree(), nodep); } + virtual void visit(AstAssign* nodep) override { - if (m_check == CT_SEQ) { - VL_RESTORER(m_assignp); - m_assignp = nodep; - iterateAndNextNull(nodep->lhsp()); - } - } - virtual void visit(AstVarRef* nodep) override { - const AstVar* const varp = nodep->varp(); - if (m_check == CT_SEQ && m_assignp && !varp->isUsedLoopIdx() // Ignore loop indices - && !varp->isTemp()) { - // Allow turning off warnings on the always, or the variable also - if (!m_alwaysp->fileline()->warnIsOff(V3ErrorCode::BLKSEQ) - && !m_assignp->fileline()->warnIsOff(V3ErrorCode::BLKSEQ) - && !varp->fileline()->warnIsOff(V3ErrorCode::BLKSEQ)) { - m_assignp->v3warn(BLKSEQ, - "Blocking assignments (=) in sequential (flop or latch) block\n" - << m_assignp->warnMore() - << "... Suggest delayed assignments (<=)"); - m_alwaysp->fileline()->modifyWarnOff( - V3ErrorCode::BLKSEQ, true); // Complain just once for the entire always - varp->fileline()->modifyWarnOff(V3ErrorCode::BLKSEQ, true); - } - } + // 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 '<='"); } + //-------------------- virtual void visit(AstNode* nodep) override { iterateChildren(nodep); } public: // CONSTRUCTORS ActiveDlyVisitor(AstNode* nodep, CheckType check) - : m_check{check} - , m_alwaysp{nodep} { + : m_check{check} { iterate(nodep); } virtual ~ActiveDlyVisitor() override = default; @@ -535,12 +530,8 @@ private: // Warn and/or convert any delayed assignments if (combo && !sequent) { + ActiveDlyVisitor{nodep, ActiveDlyVisitor::CT_COMB}; const ActiveLatchCheckVisitor latchvisitor{nodep, kwd}; - if (kwd == VAlwaysKwd::ALWAYS_LATCH) { - ActiveDlyVisitor{nodep, ActiveDlyVisitor::CT_LATCH}; - } else { - ActiveDlyVisitor{nodep, ActiveDlyVisitor::CT_COMBO}; - } } else if (!combo && sequent) { ActiveDlyVisitor{nodep, ActiveDlyVisitor::CT_SEQ}; } diff --git a/test_regress/t/t_delay_stmtdly_bad.out b/test_regress/t/t_delay_stmtdly_bad.out index 8b8482f3e..87c088ddc 100644 --- a/test_regress/t/t_delay_stmtdly_bad.out +++ b/test_regress/t/t_delay_stmtdly_bad.out @@ -20,8 +20,8 @@ : ... In instance t 20 | dly_s_t dly_s; | ^~~~~ -%Warning-BLKSEQ: t/t_delay.v:37:20: Blocking assignments (=) in sequential (flop or latch) block - : ... Suggest delayed assignments (<=) +%Warning-BLKSEQ: t/t_delay.v:37:20: Blocking assignment '=' in sequential logic process + : ... Suggest using delayed assignment '<=' 37 | dly_s.dly = 55; | ^ %Error: Exiting due to diff --git a/test_regress/t/t_initial_dlyass_bad.out b/test_regress/t/t_initial_dlyass_bad.out index 686788c11..dfebc49eb 100644 --- a/test_regress/t/t_initial_dlyass_bad.out +++ b/test_regress/t/t_initial_dlyass_bad.out @@ -1,11 +1,11 @@ -%Warning-INITIALDLY: t/t_initial_dlyass.v:18:9: Delayed assignments (<=) in initial or final block - : ... Suggest blocking assignments (=) +%Warning-INITIALDLY: t/t_initial_dlyass.v:18:9: Non-blocking assignment '<=' in initial/final block + : ... This will be executed as a blocking assignment '='! 18 | a <= 22; | ^~ ... For warning description see https://verilator.org/warn/INITIALDLY?v=latest ... Use "/* verilator lint_off INITIALDLY */" and lint_on around source to disable this message. -%Warning-INITIALDLY: t/t_initial_dlyass.v:19:9: Delayed assignments (<=) in initial or final block - : ... Suggest blocking assignments (=) +%Warning-INITIALDLY: t/t_initial_dlyass.v:19:9: Non-blocking assignment '<=' in initial/final block + : ... This will be executed as a blocking assignment '='! 19 | b <= 33; | ^~ %Error: Exiting due to diff --git a/test_regress/t/t_lint_blksync_bad.out b/test_regress/t/t_lint_blksync_bad.out index 5491f6d0e..a9ca1c217 100644 --- a/test_regress/t/t_lint_blksync_bad.out +++ b/test_regress/t/t_lint_blksync_bad.out @@ -1,11 +1,15 @@ -%Warning-BLKSEQ: t/t_lint_blksync_bad.v:24:16: Blocking assignments (=) in sequential (flop or latch) block - : ... Suggest delayed assignments (<=) +%Warning-BLKSEQ: t/t_lint_blksync_bad.v:24:16: Blocking assignment '=' in sequential logic process + : ... Suggest using delayed assignment '<=' 24 | 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-COMBDLY: t/t_lint_blksync_bad.v:31:18: Delayed assignments (<=) in non-clocked (non flop or latch) block - : ... Suggest blocking assignments (=) +%Warning-BLKSEQ: t/t_lint_blksync_bad.v:25:17: Blocking assignment '=' in sequential logic process + : ... Suggest using delayed assignment '<=' + 25 | sync_blk2 = 1'b1; + | ^ +%Warning-COMBDLY: t/t_lint_blksync_bad.v:31:18: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! 31 | combo_nblk <= 1'b1; | ^~ *** See https://verilator.org/warn/COMBDLY before disabling this, diff --git a/test_regress/t/t_lint_latch_1.out b/test_regress/t/t_lint_latch_1.out new file mode 100644 index 000000000..22f8aceb5 --- /dev/null +++ b/test_regress/t/t_lint_latch_1.out @@ -0,0 +1,9 @@ +%Warning-COMBDLY: t/t_lint_latch_1.v:14:10: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 14 | o <= b; + | ^~ + ... 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 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_1.pl b/test_regress/t/t_lint_latch_1.pl index 629a44bbb..07964a1b5 100755 --- a/test_regress/t/t_lint_latch_1.pl +++ b/test_regress/t/t_lint_latch_1.pl @@ -11,6 +11,8 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(vlt => 1); lint( + fails => 1, + expect_filename => $Self->{golden_filename}, ); ok(1); diff --git a/test_regress/t/t_lint_latch_5.out b/test_regress/t/t_lint_latch_5.out new file mode 100644 index 000000000..30dab57fe --- /dev/null +++ b/test_regress/t/t_lint_latch_5.out @@ -0,0 +1,13 @@ +%Warning-COMBDLY: t/t_lint_latch_5.v:13:13: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 13 | z[0] <= a[0]; + | ^~ + ... 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 before disabling this, + else you may end up with different sim results. +%Warning-COMBDLY: t/t_lint_latch_5.v:17:13: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 17 | z[1] <= a[1]; + | ^~ +%Error: Exiting due to diff --git a/test_regress/t/t_lint_latch_5.pl b/test_regress/t/t_lint_latch_5.pl index 629a44bbb..07964a1b5 100755 --- a/test_regress/t/t_lint_latch_5.pl +++ b/test_regress/t/t_lint_latch_5.pl @@ -11,6 +11,8 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(vlt => 1); lint( + fails => 1, + expect_filename => $Self->{golden_filename}, ); ok(1); diff --git a/test_regress/t/t_lint_latch_bad.out b/test_regress/t/t_lint_latch_bad.out index e4c196c62..e24bb171c 100644 --- a/test_regress/t/t_lint_latch_bad.out +++ b/test_regress/t/t_lint_latch_bad.out @@ -1,12 +1,16 @@ +%Warning-COMBDLY: t/t_lint_latch_bad.v:18:10: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 18 | bl <= a; + | ^~ + ... 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 before disabling this, + else you may end up with different sim results. %Warning-NOLATCH: t/t_lint_latch_bad.v:17:4: No latches detected in always_latch block 17 | always_latch begin | ^~~~~~~~~~~~ - ... For warning description see https://verilator.org/warn/NOLATCH?v=latest - ... 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 (=) +%Warning-COMBDLY: t/t_lint_latch_bad.v:25:10: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! 25 | bc <= a; | ^~ - *** See https://verilator.org/warn/COMBDLY 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 index 129267f09..d5fb68aec 100644 --- a/test_regress/t/t_lint_latch_bad_2.out +++ b/test_regress/t/t_lint_latch_bad_2.out @@ -1,7 +1,13 @@ +%Warning-COMBDLY: t/t_lint_latch_bad_2.v:13:10: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 13 | o <= b; + | ^~ + ... 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 before disabling this, + else you may end up with different sim results. %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) | ^~~~~~ - ... For warning description see https://verilator.org/warn/LATCH?v=latest - ... 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_3.out b/test_regress/t/t_lint_latch_bad_3.out index 17b014783..b154019fa 100644 --- a/test_regress/t/t_lint_latch_bad_3.out +++ b/test_regress/t/t_lint_latch_bad_3.out @@ -1,13 +1,29 @@ +%Warning-COMBDLY: t/t_lint_latch_bad_3.v:25:8: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 25 | o5 <= 1'b0; + | ^~ + ... 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 before disabling this, + else you may end up with different sim results. +%Warning-COMBDLY: t/t_lint_latch_bad_3.v:37:16: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 37 | o5 <= 1'b1; + | ^~ +%Warning-COMBDLY: t/t_lint_latch_bad_3.v:42:16: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 42 | o5 <= a; + | ^~ +%Warning-COMBDLY: t/t_lint_latch_bad_3.v:63:16: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 63 | o5 <= ~b; + | ^~ +%Warning-COMBDLY: t/t_lint_latch_bad_3.v:70:12: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 70 | o4 <= 1'b0; + | ^~ %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) | ^~~~~~ - ... For warning description see https://verilator.org/warn/LATCH?v=latest - ... 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 https://verilator.org/warn/COMBDLY before disabling this, - else you may end up with different sim results. %Error: Exiting due to diff --git a/test_regress/t/t_lint_nolatch_bad.out b/test_regress/t/t_lint_nolatch_bad.out index f4683645e..764e77aaa 100644 --- a/test_regress/t/t_lint_nolatch_bad.out +++ b/test_regress/t/t_lint_nolatch_bad.out @@ -1,6 +1,12 @@ +%Warning-COMBDLY: t/t_lint_nolatch_bad.v:13:10: Non-blocking assignment '<=' in combinational logic process + : ... This will be executed as a blocking assignment '='! + 13 | o <= b; + | ^~ + ... 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 before disabling this, + else you may end up with different sim results. %Warning-NOLATCH: t/t_lint_nolatch_bad.v:11:4: No latches detected in always_latch block 11 | always_latch @(a or b) | ^~~~~~~~~~~~ - ... For warning description see https://verilator.org/warn/NOLATCH?v=latest - ... Use "/* verilator lint_off NOLATCH */" and lint_on around source to disable this message. %Error: Exiting due to