From 60cff3e1bcb6123cf502fb5ef3a33487785f5f97 Mon Sep 17 00:00:00 2001 From: Philip Axer Date: Tue, 14 Jul 2026 15:16:12 +0200 Subject: [PATCH] Fix V3Life dropping variable writes across opaque calls V3Life's setNoopt() cleared the current block's variable map when it hit an opaque operation (a call to an entry-point function, timing control, or a force assign). This discarded the record that variables had already been written, so lifeToAbove() failed to report those writes to the enclosing block. A read after a loop whose body contained such a call was then constant-propagated back to the variable's pre-loop value, and a pre-loop assignment kept live only by the call could be deleted as redundant. Replace clear() with noopt(), which downgrades every tracked entry to a complex assignment. This clears the known constant and deletable assignment (so neither is used) while keeping the entry alive so lifeToAbove() still reports the write. The opaque operation sits inside every enclosing block, so all of them must be invalidated as well: noopt() sets a flag on the block, and lifeToAbove() propagates the invalidation one level up when the block exits, cascading outward. Ancestor maps are never consulted until control returns to them, so deferring the invalidation to block exit is equivalent and avoids walking the ancestor chain at each opaque operation. Loop and jumpblock entry now set only the visitor-level noopt (disable optimization within the body) without flagging the block, as a pure body must not invalidate constant tracking in enclosing blocks. --- src/V3Life.cpp | 22 ++++++-- test_regress/t/t_opt_life_loop_call.py | 18 ++++++ test_regress/t/t_opt_life_loop_call.v | 76 ++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 4 deletions(-) create mode 100755 test_regress/t/t_opt_life_loop_call.py create mode 100644 test_regress/t/t_opt_life_loop_call.v diff --git a/src/V3Life.cpp b/src/V3Life.cpp index 7b4d5ab1a..ee5f0f65a 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -132,6 +132,7 @@ class LifeBlock final { std::unordered_map m_map; LifeBlock* const m_aboveLifep; // Upper life, or nullptr LifeState* const m_statep; // Current global state + bool m_noopt = false; // Reached an opaque operation; invalidate the block above on exit bool m_replacedVref = false; // Replaced a variable reference since last clearing VNDeleter m_deleter; // Used to delay deletion of nodes @@ -219,6 +220,8 @@ public: void lifeToAbove() { // Any varrefs under a if/else branch affect statements outside and after the if/else UASSERT(m_aboveLifep, "Pushing life when already at the top level"); + // An opaque operation inside this block is inside the block above too + if (m_noopt) m_aboveLifep->noopt(); for (auto& itr : m_map) { AstVarScope* const nodep = itr.first; m_aboveLifep->complexAssignFind(nodep); @@ -252,7 +255,15 @@ public: } // this->lifeDump(); } - void clear() { m_map.clear(); } + void noopt() { + // Reached an opaque operation (entry-point call, timing, force) that may touch any + // non-local. Downgrade tracked entries to complex assignments, so no stale constant + // is substituted nor live assignment deleted. Blocks above are invalidated when this + // block exits, see lifeToAbove(). Always rescan, as entries recorded since an + // earlier opaque operation may hold constants this one invalidates. + for (auto& itr : m_map) itr.second.complexAssign(); + m_noopt = true; + } // DEBUG void lifeDump() { UINFO(5, " LifeMap:"); @@ -283,7 +294,7 @@ class LifeVisitor final : public VNVisitor { (void)reasonp; // UINFO(9, "setNoopt " << reasonp); m_noopt = true; - m_lifep->clear(); + m_lifep->noopt(); } void processAssignment(AstNodeStmt* nodep, AstNodeExpr* lhsp, AstNodeExpr* rhsp) { @@ -384,7 +395,9 @@ class LifeVisitor final : public VNVisitor { VL_RESTORER(m_noopt); VL_RESTORER(m_lifep); m_lifep = new LifeBlock{m_lifep, m_statep}; - setNoopt("loop"); + // Structural only: disable optimization within the body; a pure body must not + // invalidate tracking in the blocks above, so don't set the block's noopt flag + m_noopt = true; iterateAndNextNull(nodep->stmtsp()); UINFO(4, " joinloop"); // For the next assignments, clear any variables that were read or written in the block @@ -402,7 +415,8 @@ class LifeVisitor final : public VNVisitor { VL_RESTORER(m_noopt); VL_RESTORER(m_lifep); m_lifep = new LifeBlock{m_lifep, m_statep}; - setNoopt("jumpblock"); + // Structural only, as for AstLoop above + m_noopt = true; iterateAndNextNull(nodep->stmtsp()); UINFO(4, " joinjump"); // For the next assignments, clear any variables that were read or written in the block diff --git a/test_regress/t/t_opt_life_loop_call.py b/test_regress/t/t_opt_life_loop_call.py new file mode 100755 index 000000000..8a938befd --- /dev/null +++ b/test_regress/t/t_opt_life_loop_call.py @@ -0,0 +1,18 @@ +#!/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: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_opt_life_loop_call.v b/test_regress/t/t_opt_life_loop_call.v new file mode 100644 index 000000000..0e8a296c1 --- /dev/null +++ b/test_regress/t/t_opt_life_loop_call.v @@ -0,0 +1,76 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +// A call inside a loop must invalidate V3Life's tracked knowledge in the loop +// body and every enclosing block. Checks: (A) a local written in the loop body, +// (B) an enclosing constant changed only by the call, (C) a pre-loop assignment +// kept live by the call. + +// verilog_format: off +`define stop $stop +`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); +// verilog_format: on + +class c; + int unsigned x; + int unsigned calls; + int unsigned v_mem; + int unsigned sum; + + function void bump(); + calls++; + endfunction + function void clobber(); + v_mem = 7; + endfunction + function void accum(); + sum += v_mem; + endfunction + + // A: store to a local inside the loop body must survive. + function void run_local(); + int unsigned v = 0; + for (int i = 0; i < 4; i++) begin + v = 1; + bump(); + end + x = v; // must read 1, not the initializer 0 + endfunction + + // B: constant in the enclosing block, changed only by the call in the loop. + function void run_enclosing_const(); + v_mem = 5; + for (int i = 0; i < 4; i++) clobber(); + x = v_mem; // must read 7, not the pre-loop 5 + endfunction + + // C: pre-loop assignment kept live by a call that reads it. + function void run_enclosing_del(); + sum = 0; + v_mem = 5; + for (int i = 0; i < 4; i++) accum(); + v_mem = 6; // reassignment must not delete the live v_mem = 5 + endfunction +endclass + +module t; + initial begin + automatic c o = new(); + + o.run_local(); + `checkd(o.x, 1); + `checkd(o.calls, 4); + + o.run_enclosing_const(); + `checkd(o.x, 7); + + o.run_enclosing_del(); + `checkd(o.sum, 20); + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule