Mario1159/verilator
1
0
Fork 0
forked from github/verilator
Code Pull Requests Releases 1 Activity

Issue consistent INITIALDLY/COMBDLY/BLKSEQ warnings

Browse Source 
Some cases of warnings about the use of blocking and non-blocking
assignments in combinational vs sequential processes were suppressed in
a way that is inconsistent with the *actual* current execution model of
Verilator. Turning these back on to, well, warn the user that these might
cause unexpected results. V5 will clean these up, but until then err on
the side of caution.

Fixes #864.
This commit is contained in:
Geza Lore 2022-04-29 16:32:02 +01:00
parent 8395004d25
commit 49c90ecbce
13 changed files with 139 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Changes
View File

@ -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]

105
src/V3Active.cpp
View File

@ -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<AstVarRef>([&](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};
}

4
test_regress/t/t_delay_stmtdly_bad.out
View File

@ -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

8
test_regress/t/t_initial_dlyass_bad.out
View File

@ -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

12
test_regress/t/t_lint_blksync_bad.out
View File

@ -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,

9
test_regress/t/t_lint_latch_1.out Normal file
View File

@ -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

2
test_regress/t/t_lint_latch_1.pl
View File

@ -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);

13
test_regress/t/t_lint_latch_5.out Normal file
View File

@ -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

2
test_regress/t/t_lint_latch_5.pl
View File

@ -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);

16
test_regress/t/t_lint_latch_bad.out
View File

@ -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

10
test_regress/t/t_lint_latch_bad_2.out
View File

@ -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

32
test_regress/t/t_lint_latch_bad_3.out
View File

@ -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

10
test_regress/t/t_lint_nolatch_bad.out
View File

@ -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