From b29a4dfd58d2d8c485419f2fe968a81c121f729e Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 7 Jul 2026 17:17:35 +0100 Subject: [PATCH] Internals: Remove DfgGraph::clone Only used by V3DfgBreakCycles, which today either improves the graph, or leaves it unchanged. Remove unnecessary complexity. --- src/V3Dfg.cpp | 174 --------------------------------------- src/V3Dfg.h | 3 - src/V3DfgBreakCycles.cpp | 69 ++++++---------- src/V3DfgContext.h | 4 +- src/V3DfgOptimizer.cpp | 18 ++-- src/V3DfgPasses.h | 11 +-- src/astgen | 24 ------ 7 files changed, 35 insertions(+), 268 deletions(-) diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index 2fd5afb95..b729e841a 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -34,180 +34,6 @@ DfgGraph::~DfgGraph() { forEachVertex([&](DfgVertex& vtx) { vtx.unlinkDelete(*this); }); } -std::unique_ptr DfgGraph::clone() const { - // Create the new graph - DfgGraph* const clonep = new DfgGraph{name()}; - - // Map from original vertex to clone - std::unordered_map vtxp2clonep(size() * 2); - - // Clone constVertices - for (const DfgConst& vtx : m_constVertices) { - DfgConst* const cp = new DfgConst{*clonep, vtx.fileline(), vtx.num()}; - vtxp2clonep.emplace(&vtx, cp); - } - // Clone variable vertices - for (const DfgVertexVar& vtx : m_varVertices) { - const DfgVertexVar* const vp = vtx.as(); - DfgVertexVar* cp = nullptr; - - switch (vtx.type()) { - case VDfgType::VarArray: { - cp = new DfgVarArray{*clonep, vp->vscp()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::VarPacked: { - cp = new DfgVarPacked{*clonep, vp->vscp()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - default: { - vtx.v3fatalSrc("Unhandled variable vertex type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - - if (AstVarScope* const tmpForp = vp->tmpForp()) cp->tmpForp(tmpForp); - } - // Clone ast reference vertices - for (const DfgVertexAst& vtx : m_astVertices) { // LCOV_EXCL_START - switch (vtx.type()) { - case VDfgType::AstRd: { - const DfgAstRd* const vp = vtx.as(); - DfgAstRd* const cp = new DfgAstRd{*clonep, vp->exprp(), vp->inSenItem(), vp->inLoop()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - default: { - vtx.v3fatalSrc("Unhandled ast reference vertex type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - } // LCOV_EXCL_STOP - // Clone operation vertices - for (const DfgVertex& vtx : m_opVertices) { - switch (vtx.type()) { -#include "V3Dfg__gen_clone_cases.h" // From ./astgen - case VDfgType::CReset: { // LCOV_EXCL_START - No algorithm actually hits this today - DfgCReset* const cp = new DfgCReset{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } // LCOV_EXCL_STOP - case VDfgType::MatchMasked: { - DfgMatchMasked* const cp = new DfgMatchMasked{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Sel: { - DfgSel* const cp = new DfgSel{*clonep, vtx.fileline(), vtx.dtype()}; - cp->lsb(vtx.as()->lsb()); - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Rep: { - DfgRep* const cp = new DfgRep{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::UnitArray: { - DfgUnitArray* const cp = new DfgUnitArray{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Mux: { - DfgMux* const cp = new DfgMux{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::SpliceArray: { - DfgSpliceArray* const cp = new DfgSpliceArray{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::SplicePacked: { - DfgSplicePacked* const cp = new DfgSplicePacked{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Logic: { - vtx.v3fatalSrc("DfgLogic cannot be cloned"); - VL_UNREACHABLE; - break; - } - case VDfgType::Unresolved: { - vtx.v3fatalSrc("DfgUnresolved cannot be cloned"); - VL_UNREACHABLE; - break; - } - case VDfgType::AstRd: // LCOV_EXCL_START - case VDfgType::Const: - case VDfgType::VarArray: - case VDfgType::VarPacked: { - vtx.v3fatalSrc("Vertex should have been handled above: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } // LCOV_EXCL_STOP - } - } - UASSERT(size() == clonep->size(), "Size of clone should be the same"); - - // Constants have no inputs - // Hook up inputs of cloned variables - for (const DfgVertexVar& vtx : m_varVertices) { - DfgVertexVar* const cp = vtxp2clonep.at(&vtx)->as(); - if (const DfgVertex* const srcp = vtx.srcp()) cp->srcp(vtxp2clonep.at(srcp)); - if (const DfgVertex* const defp = vtx.defaultp()) cp->defaultp(vtxp2clonep.at(defp)); - } - // Hook up inputs of cloned ast references - for (const DfgVertexAst& vtx : m_astVertices) { // LCOV_EXCL_START - switch (vtx.type()) { - case VDfgType::AstRd: { - const DfgAstRd* const vp = vtx.as(); - DfgAstRd* const cp = vtxp2clonep.at(&vtx)->as(); - if (const DfgVertex* const srcp = vp->srcp()) cp->srcp(vtxp2clonep.at(srcp)); - break; - } - default: { - vtx.v3fatalSrc("Unhandled DfgVertexAst sub type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - } // LCOV_EXCL_STOP - // Hook up inputs of cloned operation vertices - for (const DfgVertex& vtx : m_opVertices) { - if (vtx.is()) { - switch (vtx.type()) { - case VDfgType::SpliceArray: - case VDfgType::SplicePacked: { - const DfgVertexSplice* const vp = vtx.as(); - DfgVertexSplice* const cp = vtxp2clonep.at(vp)->as(); - vp->foreachDriver([&](const DfgVertex& src, uint32_t lo, FileLine* flp) { - cp->addDriver(vtxp2clonep.at(&src), lo, flp); - return false; - }); - break; - } - default: { - vtx.v3fatalSrc("Unhandled DfgVertexVariadic sub type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - } else { - DfgVertex* const cp = vtxp2clonep.at(&vtx); - for (size_t i = 0; i < vtx.nInputs(); ++i) { - cp->inputp(i, vtxp2clonep.at(vtx.inputp(i))); - } - } - } - - return std::unique_ptr{clonep}; -} - void DfgGraph::mergeGraphs(std::vector>&& otherps) { if (otherps.empty()) return; diff --git a/src/V3Dfg.h b/src/V3Dfg.h index d80be4d84..c66d2d1a6 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -487,9 +487,6 @@ public: for (const DfgVertex& vtx : m_opVertices) f(vtx); } - // Return an identical, independent copy of this graph. Vertex and edge order might differ. - std::unique_ptr clone() const VL_MT_DISABLED; - // Merge contents of other graphs into this graph. Deletes the other graphs. // DfgVertexVar instances representing the same Ast variable are unified. void mergeGraphs(std::vector>&& otherps) VL_MT_DISABLED; diff --git a/src/V3DfgBreakCycles.cpp b/src/V3DfgBreakCycles.cpp index c1e3b28c5..9839beab7 100644 --- a/src/V3DfgBreakCycles.cpp +++ b/src/V3DfgBreakCycles.cpp @@ -1583,35 +1583,18 @@ public: } }; -std::pair, bool> // -breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { +bool breakCycles(DfgGraph& dfg, V3DfgBreakCyclesContext& ctx) { // Shorthand for dumping graph at given dump level const auto dump = [&](int level, const DfgGraph& dfg, const std::string& name) { if (dumpDfgLevel() >= level) dfg.dumpDotFilePrefixed("breakCycles-" + name); }; - // Can't do much with trivial things ('a = a' or 'a[1] = a[0]'), so bail - if (dfg.size() <= 2) { - UINFO(7, "Graph is trivial"); - dump(9, dfg, "trivial"); - ++ctx.m_breakCyclesContext.m_nTrivial; - return {nullptr, false}; - } - // AstNetlist/AstNodeModule user2 used as sequence numbers for temporaries const VNUser2InUse user2InUse; // Show input for debugging dump(7, dfg, "input"); - // We might fail to make any improvements, so first create a clone of the - // graph. This is what we will be working on, and return if successful. - // Do not touch the input graph. - std::unique_ptr resultp = dfg.clone(); - // Just shorthand for code below - DfgGraph& res = *resultp; - dump(9, res, "clone"); - // How many improvements have we made size_t nImprovements = 0; size_t prevNImprovements; @@ -1619,16 +1602,16 @@ breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { // Iterate while an improvement can be made and the graph is still cyclic do { // Compute SCCs - SccInfo sccInfo{res}; + SccInfo sccInfo{dfg}; // Fix up independent ranges in vertices UINFO(9, "New iteration after " << nImprovements << " improvements"); prevNImprovements = nImprovements; - const size_t nFixed = FixUp::apply(res, sccInfo); + const size_t nFixed = FixUp::apply(dfg, sccInfo); if (nFixed) { nImprovements += nFixed; - ctx.m_breakCyclesContext.m_nImprovements += nFixed; - dump(9, res, "FixUp"); + ctx.m_nImprovements += nFixed; + dump(9, dfg, "FixUp"); } // Validate SccInfo if in debug mode @@ -1637,42 +1620,38 @@ breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { // Congrats if it has become acyclic if (!sccInfo.isCyclic()) { UINFO(7, "Graph became acyclic after " << nImprovements << " improvements"); - dump(7, res, "result-acyclic"); - ++ctx.m_breakCyclesContext.m_nFixed; - return {std::move(resultp), true}; + dump(7, dfg, "result-acyclic"); + ++ctx.m_nFixed; + return true; } } while (nImprovements != prevNImprovements); + // Debug dump if (dumpDfgLevel() >= 9) { - const SccInfo sccInfo{res}; - res.dumpDotFilePrefixed("breakCycles-remaining", [&](const DfgVertex& vtx) { + const SccInfo sccInfo{dfg}; + dfg.dumpDotFilePrefixed("breakCycles-remaining", [&](const DfgVertex& vtx) { return sccInfo.get(vtx); // }); } - // If an improvement was made, return the still cyclic improved graph + // Accounting if (nImprovements) { UINFO(7, "Graph was improved " << nImprovements << " times"); - dump(7, res, "result-improved"); - ++ctx.m_breakCyclesContext.m_nImproved; - return {std::move(resultp), false}; + dump(7, dfg, "result-improved"); + ++ctx.m_nImproved; + } else { + UINFO(7, "Graph NOT improved"); + dump(7, dfg, "result-original"); + ++ctx.m_nUnchanged; } - - // No improvement was made - UINFO(7, "Graph NOT improved"); - dump(7, res, "result-original"); - ++ctx.m_breakCyclesContext.m_nUnchanged; - return {nullptr, false}; + return false; } } //namespace V3DfgBreakCycles -std::pair, bool> // -V3DfgPasses::breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { - auto pair = V3DfgBreakCycles::breakCycles(dfg, ctx); - if (pair.first) { - if (v3Global.opt.debugCheck()) V3DfgPasses::typeCheck(*pair.first); - V3DfgPasses::removeUnused(*pair.first); - } - return pair; +bool V3DfgPasses::breakCycles(DfgGraph& dfg, V3DfgContext& ctx) { + const bool res = V3DfgBreakCycles::breakCycles(dfg, ctx.m_breakCyclesContext); + if (v3Global.opt.debugCheck()) V3DfgPasses::typeCheck(dfg); + V3DfgPasses::removeUnused(dfg); + return res; } diff --git a/src/V3DfgContext.h b/src/V3DfgContext.h index 21e7af31a..ee34df5f6 100644 --- a/src/V3DfgContext.h +++ b/src/V3DfgContext.h @@ -100,9 +100,8 @@ class V3DfgBreakCyclesContext final : public V3DfgSubContext { public: // STATE VDouble0 m_nFixed; // Number of graphs that became acyclic - VDouble0 m_nImproved; // Number of graphs that were imporoved but still cyclic + VDouble0 m_nImproved; // Number of graphs that were improved but still cyclic VDouble0 m_nUnchanged; // Number of graphs that were left unchanged - VDouble0 m_nTrivial; // Number of graphs that were not changed VDouble0 m_nImprovements; // Number of changes made to graphs private: @@ -112,7 +111,6 @@ private: addStat("made acyclic", m_nFixed); addStat("improved", m_nImproved); addStat("left unchanged", m_nUnchanged); - addStat("trivial", m_nTrivial); addStat("changes applied", m_nImprovements); } }; diff --git a/src/V3DfgOptimizer.cpp b/src/V3DfgOptimizer.cpp index 07002049d..f18672598 100644 --- a/src/V3DfgOptimizer.cpp +++ b/src/V3DfgOptimizer.cpp @@ -126,19 +126,15 @@ class DataflowOptimize final { std::vector> madeAcyclicComponents; if (v3Global.opt.fDfgBreakCycles()) { for (auto it = cyclicComps.begin(); it != cyclicComps.end();) { - auto result = V3DfgPasses::breakCycles(**it, m_ctx); - if (!result.first) { - // No improvement, moving on. + const bool madeAcyclic = V3DfgPasses::breakCycles(**it, m_ctx); + // If not made acyclic, keep it in 'cyclicComps' + if (!madeAcyclic) { ++it; - } else if (!result.second) { - // Improved, but still cyclic. Replace the original cyclic component. - *it = std::move(result.first); - ++it; - } else { - // Result became acyclic. Move to madeAcyclicComponents, delete original. - madeAcyclicComponents.emplace_back(std::move(result.first)); - it = cyclicComps.erase(it); + continue; } + // Otherwise move to 'madeAcyclicComponents' + madeAcyclicComponents.emplace_back(std::move(*it)); + it = cyclicComps.erase(it); } } // Merge those that were made acyclic back to the graph, this enables optimizing more diff --git a/src/V3DfgPasses.h b/src/V3DfgPasses.h index d0d8e62d8..3392f0703 100644 --- a/src/V3DfgPasses.h +++ b/src/V3DfgPasses.h @@ -44,14 +44,9 @@ void synthesize(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; // Remove redundant selects void removeSelects(DfgGraph& dfg, V3DfgRemoveSelectsContext& ctx) VL_MT_DISABLED; // Attempt to make the given cyclic graph into an acyclic, or "less cyclic" -// equivalent. If the returned pointer is null, then no improvement was -// possible on the input graph. Otherwise the returned graph is an improvement -// on the input graph, with at least some cycles eliminated. The returned -// graph is always independent of the original. If an imporoved graph is -// returned, then the returned 'bool' flag indicated if the returned graph is -// acyclic (flag 'true'), or still cyclic (flag 'false'). -std::pair, bool> // -breakCycles(const DfgGraph&, V3DfgContext&) VL_MT_DISABLED; +// equivalent. Genuine combinational cycles can exist, so this might be +// unsuccessful. Returns true if the graph became acyclic, false otherwise. +bool breakCycles(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; // Construct binary to oneHot decoders void binToOneHot(DfgGraph&, V3DfgBinToOneHotContext&) VL_MT_DISABLED; // Common subexpression elimination diff --git a/src/astgen b/src/astgen index 75f68d5fb..7359833ff 100755 --- a/src/astgen +++ b/src/astgen @@ -1279,29 +1279,6 @@ def write_dfg_auto_classes(filename): fh.write("\n") -def write_dfg_clone_cases(filename): - with open_file(filename) as fh: - - def emitBlock(pattern, **fmt): - fh.write(textwrap.dedent(pattern).format(**fmt)) - - for node in DfgVertexList: - # Only generate code for automatically derived leaf nodes - if (node.file is not None) or not node.isLeaf: - continue - - emitBlock('''\ - case VDfgType::{t}: {{ - Dfg{t}* const cp = new Dfg{t}{{*clonep, vtx.fileline(), vtx.dtype()}}; - vtxp2clonep.emplace(&vtx, cp); - break; - }} - ''', - t=node.name, - s=node.superClass.name) - fh.write("\n") - - def write_dfg_ast_to_dfg(filename): with open_file(filename) as fh: for node in DfgVertexList: @@ -1499,7 +1476,6 @@ if Args.classes: write_type_tests("Dfg", DfgVertexList) write_dfg_macros("V3Dfg__gen_macros.h") write_dfg_auto_classes("V3Dfg__gen_auto_classes.h") - write_dfg_clone_cases("V3Dfg__gen_clone_cases.h") write_dfg_ast_to_dfg("V3Dfg__gen_ast_to_dfg.h") write_dfg_dfg_to_ast("V3Dfg__gen_dfg_to_ast.h")