diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index 727e97840..c8851280d 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -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 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(new AstAssignDly{ - fl, prep, new AstVarRef{fl, varp, VAccess::READ}, m_timingControlp}) - : static_cast(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); } }; diff --git a/test_regress/t/t_select_bound4.py b/test_regress/t/t_select_bound4.py new file mode 100755 index 000000000..00e3aae74 --- /dev/null +++ b/test_regress/t/t_select_bound4.py @@ -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() diff --git a/test_regress/t/t_select_bound4.v b/test_regress/t/t_select_bound4.v new file mode 100644 index 000000000..d09a59a52 --- /dev/null +++ b/test_regress/t/t_select_bound4.v @@ -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 diff --git a/test_regress/t/t_select_bound_side_effect.py b/test_regress/t/t_select_bound_side_effect.py new file mode 100755 index 000000000..d696b53d9 --- /dev/null +++ b/test_regress/t/t_select_bound_side_effect.py @@ -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() diff --git a/test_regress/t/t_select_bound_side_effect.v b/test_regress/t/t_select_bound_side_effect.v new file mode 100644 index 000000000..5fd21bdc8 --- /dev/null +++ b/test_regress/t/t_select_bound_side_effect.v @@ -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 diff --git a/test_regress/t/t_select_bound_timing_intra.py b/test_regress/t/t_select_bound_timing_intra.py new file mode 100755 index 000000000..4fae43d09 --- /dev/null +++ b/test_regress/t/t_select_bound_timing_intra.py @@ -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() diff --git a/test_regress/t/t_select_bound_timing_intra.v b/test_regress/t/t_select_bound_timing_intra.v new file mode 100644 index 000000000..31d30c48c --- /dev/null +++ b/test_regress/t/t_select_bound_timing_intra.v @@ -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