From 77f0883b06b157e3c278d6a57718312e9f83ddb3 Mon Sep 17 00:00:00 2001 From: Zubin Jain Date: Fri, 29 May 2026 04:03:27 +0800 Subject: [PATCH] Fix forceable signal with a procedural continuous assign (#7638) (#7639) --- src/V3Force.cpp | 58 +++++++++++++++++++++++- src/V3Force.h | 3 +- src/Verilator.cpp | 9 ++-- test_regress/t/t_forceable_assigncont.py | 16 +++++++ test_regress/t/t_forceable_assigncont.v | 14 ++++++ 5 files changed, 91 insertions(+), 9 deletions(-) create mode 100755 test_regress/t/t_forceable_assigncont.py create mode 100644 test_regress/t/t_forceable_assigncont.v diff --git a/src/V3Force.cpp b/src/V3Force.cpp index 3b9ee8635..ab2fda534 100644 --- a/src/V3Force.cpp +++ b/src/V3Force.cpp @@ -133,9 +133,14 @@ public: }; private: + using ScopeVarCache = std::unordered_map; + // NODE STATE // AstVarRef::user1 -> Flag indicating not to replace reference // AstAssignForce::user2 -> true if force is synthetic (externally forceable) + // AstVar::user3p() -> AstVarScope*: Generated __VforceRd helper + // AstVar::user4p() -> AstVarScope*: Generated __VforceEn helper + // AstVarScope::user3p() -> AstVarScope*: Generated __VforceVal helper const VNUser1InUse m_user1InUse; const VNUser2InUse m_user2InUse; @@ -143,6 +148,7 @@ private: std::unordered_map m_varToId; std::unordered_set m_clockedWrites; std::unordered_map> m_rhsDepToForces; + std::unordered_map m_scopeVarCaches; bool m_doingAssign = false; // If true, we're processing procedural continuous assign // statements instead of force statements @@ -381,6 +387,17 @@ public: return it != m_varToId.end() ? &m_varInfos[it->second] : nullptr; } + AstVarScope* findScopeVar(AstScope* scopep, const AstVar* varp) { + ScopeVarCache& cache = m_scopeVarCaches[scopep]; + if (cache.empty()) { + for (AstVarScope* vscp = scopep->varsp(); vscp; + vscp = VN_AS(vscp->nextp(), VarScope)) { + cache.emplace(vscp->varp(), vscp); + } + } + const auto it = cache.find(varp); + return it != cache.end() ? it->second : nullptr; + } void addForceAssignment(AstVar* varp, AstVarScope* vscp, AstNodeExpr* rhsExprp, AstAssignForce* forceStmtp, int rangeLsb, int rangeMsb, int padLsb, int padMsb, bool hasArraySel) { @@ -741,6 +758,27 @@ class ForceDiscoveryVisitor final : public VNVisitorConst { void visit(AstVarScope* nodep) override { if (nodep->varp()->isForceable()) { + // assignAll() runs after forceAll() and traverses the same netlist with a fresh + // ForceState. Reuse already-created public helper vars instead of regenerating + // duplicate __Vforce* members for every forceable signal. + if (m_state.doingAssign()) { + AstVar* const varp = nodep->varp(); + AstVarScope* const rdVscp = VN_CAST(varp->user3p(), VarScope); + AstVarScope* const enVscp = VN_CAST(varp->user4p(), VarScope); + AstVarScope* const valVscp = VN_CAST(nodep->user3p(), VarScope); + if (rdVscp || enVscp || valVscp) { + UASSERT_OBJ(rdVscp && enVscp && valVscp, nodep, + "Incomplete pre-existing force helper set"); + ForceState::VarForceInfo& info = m_state.getOrCreateVarInfo(varp); + info.m_forceRdVscp = rdVscp; + info.m_forceEnVscp = enVscp; + info.m_forceValVscp = valVscp; + info.m_varVscp = nodep; + iterateChildrenConst(nodep); + return; + } + } + if (VN_IS(nodep->varp()->dtypeSkipRefp(), UnpackArrayDType)) { nodep->varp()->v3warn( E_UNSUPPORTED, @@ -778,6 +816,9 @@ class ForceDiscoveryVisitor final : public VNVisitorConst { nodep->scopep()->addVarsp(rdVscp); nodep->scopep()->addVarsp(enVscp); nodep->scopep()->addVarsp(valVscp); + varp->user3p(rdVscp); + varp->user4p(enVscp); + nodep->user3p(valVscp); // Register force metadata so later transforms can find these helper vars. ForceState::VarForceInfo& info = m_state.getOrCreateVarInfo(varp); @@ -1280,7 +1321,14 @@ public: //###################################################################### // V3Force - Main entry point -void V3Force::forceAll(AstNetlist* nodep) { +namespace { +class ForceUserSlots final { + const VNUser3InUse m_user3InUse; + const VNUser4InUse m_user4InUse; +}; +} // namespace + +static void forceAllImpl(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":\n"); if (!v3Global.hasForceableSignals()) return; ForceState state{false}; @@ -1291,7 +1339,7 @@ void V3Force::forceAll(AstNetlist* nodep) { V3Global::dumpCheckGlobalTree("force", 0, dumpTreeEitherLevel() >= 3); } -void V3Force::assignAll(AstNetlist* nodep) { +static void assignAllImpl(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":\n"); if (!v3Global.hasAssignDeassign()) return; @@ -1327,3 +1375,9 @@ void V3Force::assignAll(AstNetlist* nodep) { { ForceReplaceVisitor{nodep, state}; } V3Global::dumpCheckGlobalTree("assign-deassign", 0, dumpTreeEitherLevel() >= 3); } + +void V3Force::forceAndAssignAll(AstNetlist* nodep) { + ForceUserSlots userSlots; + forceAllImpl(nodep); + assignAllImpl(nodep); +} diff --git a/src/V3Force.h b/src/V3Force.h index 01f84f6a0..8c7480e96 100644 --- a/src/V3Force.h +++ b/src/V3Force.h @@ -27,8 +27,7 @@ class AstNetlist; class V3Force final { public: - static void forceAll(AstNetlist* nodep) VL_MT_DISABLED; - static void assignAll(AstNetlist* nodep) VL_MT_DISABLED; + static void forceAndAssignAll(AstNetlist* nodep) VL_MT_DISABLED; }; #endif // Guard diff --git a/src/Verilator.cpp b/src/Verilator.cpp index 5bce5643f..77e9272d3 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -424,11 +424,10 @@ static void process() { // Convert forceable signals, process force/release statements. // After V3TraceDecl so we don't trace additional signals inserted to implement // forcing. - V3Force::forceAll(v3Global.rootp()); - - // Convert assign/deassign statements to forces on generated variables, so they can be - // handled by the same logic as regular force/release statements. - V3Force::assignAll(v3Global.rootp()); + // Convert forceable signals and assign/deassign statements in one combined pass set. + // We reserve AST user slots across both sub-passes so helper pointers can be handed + // directly from force discovery to assign/deassign lowering without rediscovery. + V3Force::forceAndAssignAll(v3Global.rootp()); // DFG optimization if (v3Global.opt.fDfg()) V3DfgOptimizer::optimize(v3Global.rootp()); diff --git a/test_regress/t/t_forceable_assigncont.py b/test_regress/t/t_forceable_assigncont.py new file mode 100755 index 000000000..77cc3f349 --- /dev/null +++ b/test_regress/t/t_forceable_assigncont.py @@ -0,0 +1,16 @@ +#!/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('vlt') + +test.compile(verilator_flags2=["-Wno-IEEEMAYDEPRECATE"]) + +test.passes() diff --git a/test_regress/t/t_forceable_assigncont.v b/test_regress/t/t_forceable_assigncont.v new file mode 100644 index 000000000..e19dd673d --- /dev/null +++ b/test_regress/t/t_forceable_assigncont.v @@ -0,0 +1,14 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Zubin Jain +// SPDX-License-Identifier: CC0-1.0 + +module t; + logic forceable_q /*verilator forceable*/ = 1'b0; + logic assigned_q = 1'b0; + + // Regression for V3Force: assignAll() should reuse the helper vars created by + // forceAll() for a forceable signal instead of generating a duplicate set. + initial assign assigned_q = forceable_q; +endmodule