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

Fix bounds checks in expressions with read/write references (#7694)

This commit is contained in:
Ryszard Rozak 2026-06-12 12:55:06 +02:00 committed by GitHub
parent 60f729639b
commit e6ee6dd106
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 228 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
src/V3Unknown.cpp
View File

@ -51,15 +51,14 @@ class UnknownVisitor final : public VNVisitor {
static const std::string s_xrandPrefix;
// STATE - across all visitors
VDouble0 m_statUnkVars; // Statistic tracking
VDouble0 m_statUnkVars; // Statistic of xrand variable created
VDouble0 m_statElses; // Statistic of else branches created for array selects
V3UniqueNames m_lvboundNames; // For generating unique temporary variable names
std::unique_ptr<V3UniqueNames> m_xrandNames; // For generating unique temporary variable names
// STATE - for current visit position (use VL_RESTORER)
AstNodeModule* m_modp = nullptr; // Current module
AstNodeFTask* m_ftaskp = nullptr; // Current function/task
AstAssignDly* m_assigndlyp = nullptr; // Current assignment
AstNode* m_timingControlp = nullptr; // Current assignment's intra timing control
bool m_constXCvt = false; // Convert X's
bool m_allowXUnique = true; // Allow unique assignments
@ -69,38 +68,21 @@ class UnknownVisitor final : public VNVisitor {
if (m_ftaskp) {
varp->funcLocal(true);
varp->lifetime(VLifetime::AUTOMATIC_EXPLICIT);
m_ftaskp->stmtsp()->addHereThisAsNext(varp);
if (m_ftaskp->stmtsp())
m_ftaskp->stmtsp()->addHereThisAsNext(varp);
else
m_ftaskp->addStmtsp(varp);
} else {
m_modp->stmtsp()->addHereThisAsNext(varp);
if (m_modp->stmtsp())
m_modp->stmtsp()->addHereThisAsNext(varp);
else
m_modp->addStmtsp(varp);
}
}
void replaceBoundLvalue(AstNodeExpr* nodep, AstNodeExpr* condp) {
// Spec says a out-of-range LHS SEL results in a NOP.
// This is a PITA. We could:
// 1. IF(...) around an ASSIGN,
// but that would break a "foo[TOO_BIG]=$fopen(...)".
// 2. Hack to extend the size of the output structure
// by one bit, and write to that temporary, but never read it.
// That makes there be two widths() and is likely a bug farm.
// 3. Make a special SEL to choose between the real lvalue
// and a temporary NOP register.
// 4. Assign to a temp, then IF that assignment.
// This is suspected to be nicest to later optimizations.
// 4 seems best but breaks later optimizations. 3 was tried,
// but makes a mess in the emitter as lvalue switching is needed. So 4.
// SEL(...) -> temp
// if (COND(LTE(bit<=maxlsb))) ASSIGN(SEL(...)),temp)
const bool needDly = (m_assigndlyp != nullptr);
if (m_assigndlyp) {
// Delayed assignments become normal assignments,
// then the temp created becomes the delayed assignment
AstNode* const newp = new AstAssign{m_assigndlyp->fileline(),
m_assigndlyp->lhsp()->unlinkFrBackWithNext(),
m_assigndlyp->rhsp()->unlinkFrBackWithNext()};
m_assigndlyp->replaceWith(newp);
VL_DO_CLEAR(pushDeletep(m_assigndlyp), m_assigndlyp = nullptr);
}
// We wrap the expression into IF
AstNodeExpr* prep = nodep;
// Scan back to put the condlvalue above all selects (IE top of the lvalue)
@ -121,32 +103,35 @@ class UnknownVisitor final : public VNVisitor {
// Already exists; rather than IF(a,... IF(b... optimize to IF(a&&b,
// Saves us teaching V3Const how to optimize, and it won't be needed again.
if (const AstIf* const ifp = VN_AS(prep->user2p(), If)) {
UASSERT_OBJ(!needDly, prep, "Should have already converted to non-delay");
VNRelinker replaceHandle;
AstNodeExpr* const earliercondp = ifp->condp()->unlinkFrBack(&replaceHandle);
AstNodeExpr* const newp = new AstLogAnd{condp->fileline(), condp, earliercondp};
UINFO(4, "Edit BOUNDLVALUE " << newp);
replaceHandle.relink(newp);
} else {
AstVar* const varp
= new AstVar{fl, VVarType::MODULETEMP, m_lvboundNames.get(prep), prep->dtypep()};
addVar(varp);
AstNode* stmtp = prep->backp(); // Grab above point before we replace 'prep'
while (!VN_IS(stmtp, NodeStmt)) stmtp = stmtp->backp();
prep->replaceWith(new AstVarRef{fl, varp, VAccess::WRITE});
if (m_timingControlp) m_timingControlp->unlinkFrBack();
AstIf* const newp = new AstIf{
fl, condp,
(needDly
? static_cast<AstNode*>(new AstAssignDly{
fl, prep, new AstVarRef{fl, varp, VAccess::READ}, m_timingControlp})
: static_cast<AstNode*>(new AstAssign{
fl, prep, new AstVarRef{fl, varp, VAccess::READ}, m_timingControlp}))};
VNRelinker replaceHandle;
AstNode* const origStmtp = stmtp->unlinkFrBack(&replaceHandle);
AstNode* elseStmtp = nullptr;
const bool hasSideEffects = origStmtp->exists(
[](AstNode* const np) { return !np->isPure() || np->isTimingControl(); });
if (hasSideEffects) {
// Copy original statement and replace `prep` with reference to tmp var to make
// sure that side effect will take place
m_statElses++;
elseStmtp = origStmtp->cloneTree(false);
AstVar* const varp = new AstVar{fl, VVarType::MODULETEMP, m_lvboundNames.get(prep),
prep->dtypep()};
addVar(varp);
AstNode* const prepCopyp = prep->clonep();
prepCopyp->replaceWith(new AstVarRef{fl, varp, VAccess::WRITE});
pushDeletep(prepCopyp);
}
AstIf* const newp = new AstIf{fl, condp, origStmtp, elseStmtp};
replaceHandle.relink(newp);
newp->branchPred(VBranchPred::BP_LIKELY);
newp->isBoundsCheck(true);
UINFOTREE(9, newp, "", "_new");
stmtp->addNextHere(newp);
prep->user2p(newp); // Save so we may LogAnd it next time
}
}
@ -208,23 +193,6 @@ class UnknownVisitor final : public VNVisitor {
m_ftaskp = nodep;
iterateChildren(nodep);
}
void visit(AstAssignDly* nodep) override {
VL_RESTORER(m_assigndlyp);
VL_RESTORER(m_timingControlp);
m_assigndlyp = nodep;
m_timingControlp = nodep->timingControlp();
VL_DO_DANGLING(iterateChildren(nodep), nodep); // May delete nodep.
}
void visit(AstAssignW* nodep) override {
VL_RESTORER(m_timingControlp);
m_timingControlp = nodep->timingControlp();
VL_DO_DANGLING(iterateChildren(nodep), nodep); // May delete nodep.
}
void visit(AstNodeAssign* nodep) override {
VL_RESTORER(m_timingControlp);
m_timingControlp = nodep->timingControlp();
iterateChildren(nodep);
}
void visit(AstCaseItem* nodep) override {
VL_RESTORER(m_constXCvt);
m_constXCvt = false; // Avoid losing the X's in casex
@ -584,6 +552,7 @@ public:
}
~UnknownVisitor() override { //
V3Stats::addStat("Unknowns, variables created", m_statUnkVars);
V3Stats::addStat("Unknowns, else branches created", m_statElses);
}
};

20
test_regress/t/t_select_bound4.py Executable file
View File

@ -0,0 +1,20 @@
#!/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: 2025 Wilson Snyder
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile(verilator_flags2=["--stats"])
test.execute()
test.file_grep(test.stats, r'Unknowns, else branches created\s+(\d+)', 0)
test.passes()

29
test_regress/t/t_select_bound4.v Normal file
View File

@ -0,0 +1,29 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// 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 Antmicro
// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
module t;
int q[2][$];
task automatic pop_q(input int qid, input int expected);
int actual;
actual = q[qid].pop_front();
if (qid < 2 && actual !== expected) $stop;
endtask
initial begin
for (int i = 0; i < 4; i++) begin
q[i].push_back(i);
end
for (int i = 0; i < 4; i++) begin
pop_q(i, i);
end
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

20
test_regress/t/t_select_bound_side_effect.py Executable file
View File

@ -0,0 +1,20 @@
#!/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: 2025 Wilson Snyder
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile(verilator_flags2=["--stats"])
test.execute()
test.file_grep(test.stats, r'Unknowns, else branches created\s+(\d+)', 1)
test.passes()

74
test_regress/t/t_select_bound_side_effect.v Normal file
View File

@ -0,0 +1,74 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// 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 Antmicro
// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
// verilog_format: off
`define stop $stop
`define checkh(gotv, expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
// verilog_format: on
module t;
int arr[5];
int x = 0;
int y = 0;
int z = 0;
task automatic incr(input int i, input int expected);
arr[i] = x++;
if (i < 5) `checkh(arr[i], expected);
endtask
function int get_y;
y++;
return y;
endfunction
task automatic assign_side_effect(input int i, input int expected);
arr[i] = get_y();
if (i < 5) `checkh(arr[i], expected);
endtask
task automatic add_z(inout int a);
a += z;
z++;
endtask
task automatic assign_side_effect_inout(input int i, input int expected);
if (i < 5) arr[i] = 1;
add_z(arr[i]);
if (i < 5) `checkh(arr[i], expected);
endtask
initial begin
incr(0, 0);
incr(7, 0);
incr(4, 2);
assign_side_effect(3, 1);
assign_side_effect(8, 0);
assign_side_effect(9, 0);
assign_side_effect(3, 4);
assign_side_effect_inout(3, 1);
assign_side_effect_inout(4, 2);
assign_side_effect_inout(5, 0);
assign_side_effect_inout(1, 4);
y = 0;
for (int i = 0; i < 10; i++) begin
arr[get_y()] = i;
if (y < 5) `checkh(arr[y], i);
`checkh(y, 2 * i + 1);
arr[get_y() % (i + 1)] = i;
if (y % (i + 1) < 5) `checkh(arr[y % (i + 1)], i);
`checkh(y, 2 * (i + 1));
end
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

20
test_regress/t/t_select_bound_timing_intra.py Executable file
View File

@ -0,0 +1,20 @@
#!/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: 2024 Wilson Snyder
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile(verilator_flags2=["--binary", "--stats"])
test.execute()
test.file_grep(test.stats, r'Unknowns, else branches created\s+(\d+)', 1)
test.passes()

34
test_regress/t/t_select_bound_timing_intra.v Normal file
View File

@ -0,0 +1,34 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// 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 Antmicro
// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
// verilog_format: off
`define stop $stop
`define checkh(gotv, expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
// verilog_format: on
module t;
int arr[5];
task automatic intra(input int i);
time t = $time;
arr[i] = #1 i;
#1;
if (i < 5) `checkh(arr[i], i);
`checkh($time, t + 2);
endtask
initial begin
intra(0);
intra(7);
intra(4);
intra(1);
$write("*-* All Finished *-*\n");
$finish;
end
endmodule