From d84af81a11cab7ba8d6b5e42a27bf53b4e797787 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 10 Jun 2026 15:59:44 +0100 Subject: [PATCH] Optimize Dfg with relaxed live variable analysis (#7739) Relax the live variable analysis performed by Dfg to bail on fewer cases. This analysis was already conservative (meaning it might think variables are live when they are not), which is good enough for Dfg use. This change in particular enables synthesizing more complex logic involving arrays, e.g. those introduce by V3Table creating lookup tables. --- src/V3AstNodeOther.h | 1 + src/V3CfgLiveVariables.cpp | 32 +++++++------------ src/V3Dfg.cpp | 9 ++++-- test_regress/t/t_case_huge.py | 3 +- test_regress/t/t_dfg_synthesis.v | 9 ++++++ .../t_gate_inline_wide_noexclude_arraysel.py | 2 +- 6 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 903130658..f3692d05e 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -987,6 +987,7 @@ public: bool maybePointedTo() const override VL_MT_SAFE { return true; } void cloneRelink() override { V3ERROR_NA; } // Not cloneable AstModule* modp() const { return m_modp; } + AstScope* scopep() const { return m_scopep; } // Find a table (unpacked array) within the constant pool which is initialized with the // given value, or create one if one does not already exists. The returned VarScope *might* diff --git a/src/V3CfgLiveVariables.cpp b/src/V3CfgLiveVariables.cpp index 79033af9d..162e0837d 100644 --- a/src/V3CfgLiveVariables.cpp +++ b/src/V3CfgLiveVariables.cpp @@ -51,28 +51,18 @@ class CfgLiveVariables final : VNVisitorConst { // METHODS - // This is to match DFG, but can be extended independently - eventually should handle all - static bool isSupportedPackedDType(const AstNodeDType* dtypep) { - dtypep = dtypep->skipRefp(); + static bool isPacked(const AstVarScope* vscp) { + AstNodeDType* const dtypep = vscp->dtypep()->skipRefp(); if (const AstBasicDType* const typep = VN_CAST(dtypep, BasicDType)) { return typep->keyword().isIntNumeric(); } - if (const AstPackArrayDType* const typep = VN_CAST(dtypep, PackArrayDType)) { - return isSupportedPackedDType(typep->subDTypep()); - } + if (VN_IS(dtypep, PackArrayDType)) return true; if (const AstNodeUOrStructDType* const typep = VN_CAST(dtypep, NodeUOrStructDType)) { return typep->packed(); } return false; } - // Check and return if variable is incompatible - bool incompatible(AstVarScope* vscp) { - if (!isSupportedPackedDType(vscp->dtypep())) return true; - if (vscp->varp()->ignoreSchedWrite()) return true; - return false; - } - void updateGen(AstNode* nodep) { UASSERT_OBJ(!m_abort, nodep, "Doing useless work"); m_abort = nodep->exists([&](const AstVarRef* refp) { @@ -80,8 +70,6 @@ class CfgLiveVariables final : VNVisitorConst { if (refp->access().isWriteOnly()) return false; // Grab referenced variable AstVarScope* const vscp = refp->varScopep(); - // Bail if not a compatible type - if (incompatible(vscp)) return true; // Add to gen set, if not killed - assume whole variable read if (m_currp->m_kill.count(vscp)) return false; m_currp->m_gen.emplace(vscp); @@ -96,11 +84,13 @@ class CfgLiveVariables final : VNVisitorConst { if (refp->access().isReadOnly()) return false; // Grab referenced variable AstVarScope* const vscp = refp->varScopep(); - // Bail if not a compatible type - if (incompatible(vscp)) return true; - // If whole written, add to kill set - if (refp->nextp()) return false; - if (VN_IS(refp->abovep(), Sel)) return false; + // Safety case: bail on variables with unusual semantics + if (vscp->varp()->ignoreSchedWrite()) return true; + // If not a packed variable, assume it's not a whole update, don't add to kill set + if (!isPacked(vscp)) return false; + // If packed, and updated partially, don't add to kill set + if (refp->backp()->nextp() != refp && VN_IS(refp->abovep(), Sel)) return false; + // Whole variable is writen, add to kill set m_currp->m_kill.emplace(vscp); return false; }); @@ -198,7 +188,7 @@ public: CfgLiveVariables analysis{cfg}; // If failed, return nullptr if (analysis.m_abort) return nullptr; - // Gather variables live in to the entry blockstd::unique_ptr> + // Gather variables live in to the entry block const std::unordered_set& lin = analysis.m_blockState[cfg.enter()].m_liveIn; std::vector* const resultp = new std::vector{lin.begin(), lin.end()}; diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index 273d1dcff..637cf184f 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -906,17 +906,20 @@ AstScope* DfgVertex::scopep(ScopeCache& cache, bool tryResultVar) VL_MT_DISABLED } } + AstScope* const rootp = v3Global.rootp()->topScopep()->scopep(); + AstScope* const constPoolp = v3Global.rootp()->constPoolp()->scopep(); + // Note: the recursive invocation can cause a re-hash but that will not invalidate references AstScope*& resultr = cache[this]; if (!resultr) { // Mark to prevent infinite recursion on circular graphs - should never be called on such resultr = reinterpret_cast(1); - // Find scope based on sources, falling back on the root scope - AstScope* const rootp = v3Global.rootp()->topScopep()->scopep(); + // Find scope based on sources, falling back on the root scope, + // also make sure it's not the constant pool scope, which is special. AstScope* foundp = nullptr; foreachSource([&](DfgVertex& src) { AstScope* const scp = src.scopep(cache, true); - if (scp != rootp) { + if (scp != rootp && scp != constPoolp) { foundp = scp; return true; } diff --git a/test_regress/t/t_case_huge.py b/test_regress/t/t_case_huge.py index 81fcffbad..0ac31a2f8 100755 --- a/test_regress/t/t_case_huge.py +++ b/test_regress/t/t_case_huge.py @@ -11,7 +11,8 @@ import vltest_bootstrap test.scenarios('simulator') -test.compile(verilator_flags2=["--stats"]) +# This tests combining CFuncs, but Dfg would inline the submodule, disabling +test.compile(verilator_flags2=["--stats", "-fno-dfg"]) test.execute() diff --git a/test_regress/t/t_dfg_synthesis.v b/test_regress/t/t_dfg_synthesis.v index 9076b4101..c13d4f47c 100644 --- a/test_regress/t/t_dfg_synthesis.v +++ b/test_regress/t/t_dfg_synthesis.v @@ -577,4 +577,13 @@ module t ( end `signal(VIA_CRESET, via_creset); + logic [1:0] LUT [2] = '{0: 2'b11, 1: 2'b10}; + logic [1:0] array_read; + always_comb begin + array_read = 2'd0; + array_read += LUT[rand_a[0]]; + array_read += LUT[rand_a[1]]; + end + `signal(ARRAY_READ, array_read); + endmodule diff --git a/test_regress/t/t_gate_inline_wide_noexclude_arraysel.py b/test_regress/t/t_gate_inline_wide_noexclude_arraysel.py index ccbd8c7e7..8c99e4b33 100755 --- a/test_regress/t/t_gate_inline_wide_noexclude_arraysel.py +++ b/test_regress/t/t_gate_inline_wide_noexclude_arraysel.py @@ -11,7 +11,7 @@ import vltest_bootstrap test.scenarios('vlt') -test.lint(verilator_flags2=['--stats', '--expand-limit 5']) +test.lint(verilator_flags2=['--stats', '--expand-limit 5', '-fno-dfg']) test.file_grep(test.stats, r'Optimizations, Gate excluded wide expressions\s+(\d+)', 0) test.file_grep(test.stats, r'Optimizations, Gate sigs deleted\s+(\d+)', 1)