From 6595e5cf55ecac422713ca84542d499c01dd5537 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Fri, 10 Apr 2026 13:46:59 +0100 Subject: [PATCH] Remove Dfg extract pass (#7394) This has no measurable benefit and algorithmically will be superseded by some upcoming work. --- src/V3DfgOptimizer.cpp | 212 +---------------------------------------- src/V3DfgOptimizer.h | 3 - src/Verilator.cpp | 3 - 3 files changed, 1 insertion(+), 217 deletions(-) diff --git a/src/V3DfgOptimizer.cpp b/src/V3DfgOptimizer.cpp index 1d15c122c..7bca7d2d4 100644 --- a/src/V3DfgOptimizer.cpp +++ b/src/V3DfgOptimizer.cpp @@ -22,225 +22,15 @@ #include "V3DfgOptimizer.h" -#include "V3AstUserAllocator.h" #include "V3Const.h" #include "V3Dfg.h" #include "V3DfgPasses.h" #include "V3Error.h" -#include "V3Graph.h" -#include "V3UniqueNames.h" #include VL_DEFINE_DEBUG_FUNCTIONS; -// Extract more combinational logic equations from procedures for better optimization opportunities -class DataflowExtractVisitor final : public VNVisitor { - // NODE STATE - // AstVar::user3 -> bool: Flag indicating variable is subject of force or release - // statement AstVar::user4 -> bool: Flag indicating variable is combinationally driven - // AstNodeModule::user4 -> Extraction candidates (via m_extractionCandidates) - const VNUser3InUse m_user3InUse; - const VNUser4InUse m_user4InUse; - - // Expressions considered for extraction as separate assignment to gain more opportunities for - // optimization, together with the list of variables they read. - using Candidates = std::vector>>; - - // Expressions considered for extraction. All the candidates are pure expressions. - AstUser4Allocator m_extractionCandidates; - - // STATE - AstNodeModule* m_modp = nullptr; // The module being visited - Candidates* m_candidatesp = nullptr; - bool m_impure = false; // True if the visited tree has a side effect - bool m_inForceReleaseLhs = false; // Iterating LHS of force/release - // List of AstVar nodes read by the visited tree. 'vector' rather than 'set' as duplicates are - // somewhat unlikely and we can handle them later. - std::vector m_readVars; - - // METHODS - - // Node considered for extraction as a combinational equation. Trace variable usage/purity. - void iterateExtractionCandidate(AstNode* nodep) { - UASSERT_OBJ(!VN_IS(nodep->backp(), NodeExpr), nodep, - "Should not try to extract nested expressions (only root expressions)"); - - // Simple VarRefs should not be extracted, as they only yield trivial assignments. - // Similarly, don't extract anything if no candidate map is set up (for non-modules). - // We still need to visit them though, to mark hierarchical references. - if (VN_IS(nodep, NodeVarRef) || !m_candidatesp) { - iterate(nodep); - return; - } - - // Don't extract plain constants - if (VN_IS(nodep, Const)) return; - - // Candidates can't nest, so no need for VL_RESTORER, just initialize iteration state - m_impure = false; - m_readVars.clear(); - - // Trace variable usage - iterate(nodep); - - // We only extract pure expressions - if (m_impure) return; - - // Do not extract expressions without any variable references - if (m_readVars.empty()) return; - - // Add to candidate list - m_candidatesp->emplace_back(VN_AS(nodep, NodeExpr), std::move(m_readVars)); - } - - // VISIT methods - - void visit(AstNetlist* nodep) override { - // Analyze the whole design - iterateChildrenConst(nodep); - - // Replace candidate expressions only reading combinationally driven signals with variables - V3UniqueNames names{"__VdfgExtracted"}; - for (AstNodeModule* modp = nodep->modulesp(); modp; - modp = VN_AS(modp->nextp(), NodeModule)) { - // Only extract from proper modules - if (!VN_IS(modp, Module)) continue; - - for (const auto& pair : m_extractionCandidates(modp)) { - AstNodeExpr* const cnodep = pair.first; - - // Do not extract expressions without any variable references - if (pair.second.empty()) continue; - - // Check if all variables read by this expression are driven combinationally, - // and move on if not. Also don't extract it if one of the variables is subject - // to a force/release, as releasing nets must have immediate effect, but adding - // extra combinational logic can change semantics (see t_force_release_net*). - { - bool hasBadVar = false; - for (const AstVar* const readVarp : pair.second) { - // variable is target of force/release or not combinationally driven - if (readVarp->user3() || !readVarp->user4()) { - hasBadVar = true; - break; - } - } - if (hasBadVar) continue; - } - - // Create temporary variable - FileLine* const flp = cnodep->fileline(); - const string name = names.get(cnodep); - AstVar* const varp = new AstVar{flp, VVarType::MODULETEMP, name, cnodep->dtypep()}; - varp->trace(false); - modp->addStmtsp(varp); - - // Replace expression with temporary variable - cnodep->replaceWith(new AstVarRef{flp, varp, VAccess::READ}); - - // Add assignment driving temporary variable - AstAssignW* const ap - = new AstAssignW{flp, new AstVarRef{flp, varp, VAccess::WRITE}, cnodep}; - modp->addStmtsp(new AstAlways{ap}); - } - } - } - - void visit(AstNodeModule* nodep) override { - VL_RESTORER(m_modp); - m_modp = nodep; - iterateChildrenConst(nodep); - } - - void visit(AstAlways* nodep) override { - VL_RESTORER(m_candidatesp); - // Only extract from combinational logic under proper modules - const bool isComb = !nodep->sentreep() - && (nodep->keyword() == VAlwaysKwd::ALWAYS - || nodep->keyword() == VAlwaysKwd::ALWAYS_COMB - || nodep->keyword() == VAlwaysKwd::ALWAYS_LATCH); - m_candidatesp - = isComb && VN_IS(m_modp, Module) ? &m_extractionCandidates(m_modp) : nullptr; - iterateChildrenConst(nodep); - } - - void visit(AstAssignW* nodep) override { - // Mark LHS variable as combinationally driven - if (const AstVarRef* const vrefp = VN_CAST(nodep->lhsp(), VarRef)) { - vrefp->varp()->user4(true); - } - // - iterateChildrenConst(nodep); - } - - void visit(AstAssign* nodep) override { - iterateExtractionCandidate(nodep->rhsp()); - iterate(nodep->lhsp()); - } - - void visit(AstAssignDly* nodep) override { - iterateExtractionCandidate(nodep->rhsp()); - iterate(nodep->lhsp()); - } - - void visit(AstIf* nodep) override { - iterateExtractionCandidate(nodep->condp()); - iterateAndNextConstNull(nodep->thensp()); - iterateAndNextConstNull(nodep->elsesp()); - } - - void visit(AstAssignForce* nodep) override { - VL_RESTORER(m_inForceReleaseLhs); - iterate(nodep->rhsp()); - UASSERT_OBJ(!m_inForceReleaseLhs, nodep, "Should not nest"); - m_inForceReleaseLhs = true; - iterate(nodep->lhsp()); - } - - void visit(AstRelease* nodep) override { - VL_RESTORER(m_inForceReleaseLhs); - UASSERT_OBJ(!m_inForceReleaseLhs, nodep, "Should not nest"); - m_inForceReleaseLhs = true; - iterate(nodep->lhsp()); - } - - void visit(AstNodeExpr* nodep) override { iterateChildrenConst(nodep); } - void visit(AstArg* nodep) override { iterateChildrenConst(nodep); } - - void visit(AstNodeVarRef* nodep) override { - if (nodep->access().isWriteOrRW()) { - // If it writes a variable, mark as impure - m_impure = true; - // Mark target of force/release - if (m_inForceReleaseLhs) nodep->varp()->user3(true); - } else { - // Otherwise, add read reference - m_readVars.push_back(nodep->varp()); - } - } - - void visit(AstNode* nodep) override { - // Conservatively assume unhandled nodes are impure. - m_impure = true; - // Still need to gather all references/force/release, etc. - iterateChildrenConst(nodep); - } - - // CONSTRUCTOR - explicit DataflowExtractVisitor(AstNetlist* netlistp) { iterate(netlistp); } - -public: - static void apply(AstNetlist* netlistp) { DataflowExtractVisitor{netlistp}; } -}; - -void V3DfgOptimizer::extract(AstNetlist* netlistp) { - UINFO(2, __FUNCTION__ << ":"); - // Extract more optimization candidates - DataflowExtractVisitor::apply(netlistp); - V3Global::dumpCheckGlobalTree("dfg-extract", 0, dumpTreeEitherLevel() >= 3); -} - class DataflowOptimize final { // NODE STATE // AstVar::user1, AstVarScope::user1 -> int, used as a bit-field @@ -258,7 +48,7 @@ class DataflowOptimize final { V3DfgContext m_ctx; // The context holding values that need to persist across multiple graphs void endOfStage(const std::string& name) { - if (VL_UNLIKELY(v3Global.opt.stats())) V3Stats::statsStage("dfg-optimize-" + name); + if (VL_UNLIKELY(v3Global.opt.stats())) V3Stats::statsStage("dfg-" + name); } void endOfStage(const std::string& name, const DfgGraph& dfg, diff --git a/src/V3DfgOptimizer.h b/src/V3DfgOptimizer.h index bea3bb89e..0af073074 100644 --- a/src/V3DfgOptimizer.h +++ b/src/V3DfgOptimizer.h @@ -25,9 +25,6 @@ //============================================================================ namespace V3DfgOptimizer { -// Extract further logic blocks from the design for additional optimization opportunities -void extract(AstNetlist*) VL_MT_DISABLED; - // Optimize the design void optimize(AstNetlist*) VL_MT_DISABLED; } // namespace V3DfgOptimizer diff --git a/src/Verilator.cpp b/src/Verilator.cpp index 6c42faa3b..b09f72e82 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -307,9 +307,6 @@ static void process() { v3Global.constRemoveXs(true); } - // If doing DFG optimization, extract some additional candidates - if (v3Global.opt.fDfg()) V3DfgOptimizer::extract(v3Global.rootp()); - if (!(v3Global.opt.serializeOnly() && !v3Global.opt.flatten())) { // Module inlining // Cannot remove dead variables after this, as alias information for final