From b4a0ca8ba6e5fbdd6c6cd88b23aff81989a39ad9 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 1 Apr 2026 10:52:56 +0100 Subject: [PATCH] Optimize Ast read references in Dfg directly (#7354) Introduce a new DfgAstRd vertex, which holds an AstNodeExpr that is a reference to a variable. This enables tracking all read references in Dfg, which both enables more optimization, and allows inlining of expressions in place of the reference more intelligently (e.g, when the expression is only used once, and is not in a loop). This can get rid of 20-30% of temporary variables introduced in Dfg in some designs. Note V3Gate later got rid of a lot of those, this is a step towards making V3Gate redundant. The more intelligent expression inlining is worth ~10% runtime speed on some designs. --- src/V3Dfg.cpp | 112 +++++++--- src/V3Dfg.h | 23 ++- src/V3DfgAstToDfg.cpp | 100 ++++++++- src/V3DfgColorSCCs.cpp | 6 + src/V3DfgContext.h | 13 +- src/V3DfgCse.cpp | 13 +- src/V3DfgDecomposition.cpp | 10 +- src/V3DfgDfgToAst.cpp | 30 ++- src/V3DfgOptimizer.cpp | 37 +++- src/V3DfgPasses.cpp | 51 +++-- src/V3DfgPasses.h | 6 + src/V3DfgPeephole.cpp | 14 +- src/V3DfgRegularize.cpp | 191 +++++++----------- src/V3DfgSynthesize.cpp | 31 ++- src/V3DfgVertices.h | 49 ++++- test_regress/t/t_debug_width.out | 6 +- test_regress/t/t_dfg_astrefs.out | 31 +++ test_regress/t/t_dfg_astrefs.py | 18 ++ test_regress/t/t_dfg_astrefs.v | 47 +++++ test_regress/t/t_dfg_inline_const_clock.py | 18 ++ test_regress/t/t_dfg_inline_const_clock.v | 25 +++ .../t/t_dfg_stats_patterns_post_inline.out | 1 + test_regress/t/t_opt_dedupe_clk_gate.py | 2 +- 23 files changed, 610 insertions(+), 224 deletions(-) create mode 100644 test_regress/t/t_dfg_astrefs.out create mode 100755 test_regress/t/t_dfg_astrefs.py create mode 100644 test_regress/t/t_dfg_astrefs.v create mode 100755 test_regress/t/t_dfg_inline_const_clock.py create mode 100644 test_regress/t/t_dfg_inline_const_clock.v diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index 76676790e..cb203d66c 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -80,6 +80,22 @@ std::unique_ptr DfgGraph::clone() const { if (AstNode* 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()) { @@ -136,6 +152,22 @@ std::unique_ptr DfgGraph::clone() const { 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()) { @@ -201,6 +233,14 @@ void DfgGraph::mergeGraphs(std::vector>&& otherps) { #endif } m_varVertices.splice(m_varVertices.end(), otherp->m_varVertices); + // Process Ast references + for (DfgVertexAst* const vtxp : otherp->m_astVertices.unlinkable()) { + vtxp->m_userGeneration = 0; +#ifdef VL_DEBUG + vtxp->m_dfgp = this; +#endif + } + m_astVertices.splice(m_astVertices.end(), otherp->m_astVertices); // Process constants for (DfgConst& vtx : otherp->m_constVertices) { vtx.m_userGeneration = 0; @@ -270,7 +310,6 @@ static const std::string toDotId(const DfgVertex& vtx) { return '"' + cvtToHex(& static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { if (const DfgVertexVar* const varVtxp = vtx.cast()) { const AstNode* const nodep = varVtxp->nodep(); - const AstVar* const varp = varVtxp->varp(); os << toDotId(vtx); // Begin attributes os << " ["; @@ -292,7 +331,7 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { os << " / "; static const char* const rwmn[2][2] = {{"_", "W"}, {"R", "M"}}; os << rwmn[varVtxp->hasExtRdRefs()][varVtxp->hasExtWrRefs()]; - os << rwmn[varVtxp->hasModRdRefs()][varVtxp->hasModWrRefs()]; + os << rwmn[false][varVtxp->hasModWrRefs()]; os << (varVtxp->hasDfgRefs() ? "D" : "_"); // End 'label' os << '"'; @@ -305,21 +344,13 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { varVtxp->v3fatalSrc("Unhandled DfgVertexVar sub-type"); } // Color - if (varp->direction() == VDirection::INPUT) { - os << ", style=filled, fillcolor=chartreuse2"; // Green - } else if (varp->direction() == VDirection::OUTPUT) { - os << ", style=filled, fillcolor=cyan2"; // Cyan - } else if (varp->direction() == VDirection::INOUT) { - os << ", style=filled, fillcolor=darkorchid2"; // Purple - } else if (varVtxp->hasExtRefs()) { - os << ", style=filled, fillcolor=firebrick2"; // Red - } else if (varVtxp->hasModRefs()) { - os << ", style=filled, fillcolor=darkorange1"; // Orange - } else if (varVtxp->hasDfgRefs()) { - os << ", style=filled, fillcolor=gold2"; // Yellow - } else if (varVtxp->tmpForp()) { - os << ", style=filled, fillcolor=gray95"; - } + const char* const colorp = varVtxp->hasExtRefs() ? "firebrick2" // Red + : varVtxp->hasModWrRefs() ? "darkorange1" // Orange + : varVtxp->hasDfgRefs() ? "gold2" // Yellow + : varVtxp->tmpForp() ? "gray95" // Gray + : "white"; + os << ", style=filled"; + os << ", fillcolor=\"" << colorp << "\""; // End attributes os << "]\n"; return; @@ -379,7 +410,7 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { os << toDotId(vtx); std::stringstream ss; V3EmitV::debugVerilogForTree(logicp->nodep(), ss); - std::string str = ss.str(); + std::string str = "AstNode: " + cvtToHex(logicp->nodep()) + "\n" + ss.str(); str = VString::quoteBackslash(str); str = VString::quoteAny(str, '"', '\\'); str = VString::replaceSubstr(str, "\n", "\\l"); @@ -390,7 +421,32 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { os << " [label=\""; os << str; - os << "\\n" << cvtToHex(&vtx); + os << '\n' << vtx.typeName() << '\n' << cvtToHex(&vtx); + os << "\"\n"; + os << ", shape=box, style=\"rounded,filled\", nojustify=true"; + os << ", fillcolor=\"" << colorp << "\""; + os << "]\n"; + return; + } + + if (const DfgVertexAst* const astVtxp = vtx.cast()) { + os << toDotId(vtx); + std::stringstream ss; + V3EmitV::debugVerilogForTree(astVtxp->exprp(), ss); + std::string str = "AstNode: " + cvtToHex(astVtxp->exprp()) + "\n" + ss.str() + "\n"; + str = VString::quoteBackslash(str); + str = VString::quoteAny(str, '"', '\\'); + str = VString::replaceSubstr(str, "\n", "\\l"); + const DfgAstRd* const astRdVtxp = astVtxp->cast(); + const char* const colorp = astRdVtxp ? "#80ff80" // Green + : "#ffff80"; // Yellow + os << " [label=\""; + os << str; + os << '\n' << vtx.typeName() << '\n' << cvtToHex(&vtx) << '\n'; + vtx.dtype().astDtypep()->dumpSmall(os); + os << " / "; + os << "_S"[astRdVtxp->inSenItem()]; + os << "_L"[astRdVtxp->inLoop()]; os << "\"\n"; os << ", shape=box, style=\"rounded,filled\", nojustify=true"; os << ", fillcolor=\"" << colorp << "\""; @@ -550,6 +606,11 @@ void DfgVertex::typeCheck(const DfgGraph& dfg) const { CHECK(isPacked(), "Should be Packed type"); return; } + case VDfgType::AstRd: { + const DfgAstRd& v = *as(); + CHECK(v.isPacked() || v.isArray(), "Should be Packed or Array type"); + return; + } case VDfgType::VarArray: case VDfgType::VarPacked: { const DfgVertexVar& v = *as(); @@ -781,16 +842,21 @@ DfgVertexVar* DfgVertex::getResultVar() { if (!resp->hasDfgRefs()) resp = varp; return false; } - // Prefer those that already have module references - if (resp->hasModRdRefs() != varp->hasModRdRefs()) { - if (!resp->hasModRdRefs()) resp = varp; + + // Prefer real variables over temporaries + const bool resIsTemp + = resp->varp() ? resp->varp()->isTemp() : resp->varScopep()->varp()->isTemp(); + const bool varIsTemp + = varp->varp() ? varp->varp()->isTemp() : varp->varScopep()->varp()->isTemp(); + if (resIsTemp != varIsTemp) { + if (resIsTemp) resp = varp; return false; } - // Prefer real variabels over temporaries if (!resp->tmpForp() != !varp->tmpForp()) { if (resp->tmpForp()) resp = varp; return false; } + // Prefer the earlier one in source order const FileLine& oldFlp = *(resp->fileline()); const FileLine& newFlp = *(varp->fileline()); diff --git a/src/V3Dfg.h b/src/V3Dfg.h index 94e588c26..9e9ab18b9 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -382,6 +382,7 @@ class DfgGraph final { // enables significant Verilation performance gains, so we keep these in separate lists for // direct access. DfgVertex::List m_varVertices; // The variable vertices in the graph + DfgVertex::List m_astVertices; // The ast reference vertices in the graph DfgVertex::List m_constVertices; // The constant vertices in the graph DfgVertex::List m_opVertices; // The operation vertices in the graph size_t m_size = 0; // Number of vertices in the graph @@ -417,6 +418,8 @@ public: // Access to vertex lists DfgVertex::List& varVertices() { return m_varVertices; } const DfgVertex::List& varVertices() const { return m_varVertices; } + DfgVertex::List& astVertices() { return m_astVertices; } + const DfgVertex::List& astVertices() const { return m_astVertices; } DfgVertex::List& constVertices() { return m_constVertices; } const DfgVertex::List& constVertices() const { return m_constVertices; } DfgVertex::List& opVertices() { return m_opVertices; } @@ -429,10 +432,12 @@ public: #endif // Note: changes here need to be replicated in DfgGraph::mergeGraphs ++m_size; - if (DfgConst* const cVtxp = vtx.cast()) { - m_constVertices.linkBack(cVtxp); - } else if (DfgVertexVar* const vVtxp = vtx.cast()) { + if (DfgVertexVar* const vVtxp = vtx.cast()) { m_varVertices.linkBack(vVtxp); + } else if (DfgVertexAst* const aVtxp = vtx.cast()) { + m_astVertices.linkBack(aVtxp); + } else if (DfgConst* const cVtxp = vtx.cast()) { + m_constVertices.linkBack(cVtxp); } else { m_opVertices.linkBack(&vtx); } @@ -449,10 +454,12 @@ public: #endif // Note: changes here need to be replicated in DfgGraph::mergeGraphs --m_size; - if (DfgConst* const cVtxp = vtx.cast()) { - m_constVertices.unlink(cVtxp); - } else if (DfgVertexVar* const vVtxp = vtx.cast()) { + if (DfgVertexVar* const vVtxp = vtx.cast()) { m_varVertices.unlink(vVtxp); + } else if (DfgVertexAst* const aVtxp = vtx.cast()) { + m_astVertices.unlink(aVtxp); + } else if (DfgConst* const cVtxp = vtx.cast()) { + m_constVertices.unlink(cVtxp); } else { m_opVertices.unlink(&vtx); } @@ -467,6 +474,7 @@ public: // not safe to delete/unlink any vertex in the same graph other than the one passed to 'f'. void forEachVertex(std::function f) { for (DfgVertexVar* const vtxp : m_varVertices.unlinkable()) f(*vtxp); + for (DfgVertexAst* const vtxp : m_astVertices.unlinkable()) f(*vtxp); for (DfgConst* const vtxp : m_constVertices.unlinkable()) f(*vtxp); for (DfgVertex* const vtxp : m_opVertices.unlinkable()) f(*vtxp); } @@ -474,6 +482,7 @@ public: // 'const' variant of 'forEachVertex'. No mutation allowed. void forEachVertex(std::function f) const { for (const DfgVertexVar& vtx : m_varVertices) f(vtx); + for (const DfgVertexAst& vtx : m_astVertices) f(vtx); for (const DfgConst& vtx : m_constVertices) f(vtx); for (const DfgVertex& vtx : m_opVertices) f(vtx); } @@ -809,6 +818,8 @@ void DfgEdge::relinkSrcp(DfgVertex* srcp) { // DfgVertex {{{ bool DfgVertex::isCheaperThanLoad() const { + // Constants + if (is()) return true; // Array sels are just address computation if (is()) return true; // Small constant select from variable diff --git a/src/V3DfgAstToDfg.cpp b/src/V3DfgAstToDfg.cpp index 52f2aec74..02bc6f7f6 100644 --- a/src/V3DfgAstToDfg.cpp +++ b/src/V3DfgAstToDfg.cpp @@ -28,10 +28,89 @@ #include "V3Dfg.h" #include "V3DfgPasses.h" -#include - VL_DEFINE_DEBUG_FUNCTIONS; +template +class AstToDfgAddAstRefs final : public VNVisitorConst { + // TYPES + using Variable = std::conditional_t; + + // STATE + DfgGraph& m_dfg; // The graph being processed + // Function to get the DfgVertexVar for a Variable + const std::function m_getVarVertex; + bool m_inSenItem = false; // Inside an AstSenItem + bool m_inLoop = false; // Inside an AstLoop + + // METHODS + static Variable* getTarget(const AstVarRef* refp) { + // TODO: remove the useless reinterpret_casts when C++17 'if constexpr' actually works + if VL_CONSTEXPR_CXX17 (T_Scoped) { + return reinterpret_cast(refp->varScopep()); + } else { + return reinterpret_cast(refp->varp()); + } + } + + // VISITORS + void visit(AstNode* nodep) override { iterateChildrenConst(nodep); } + + void visit(AstSenItem* nodep) override { + VL_RESTORER(m_inSenItem); + m_inSenItem = true; + iterateChildrenConst(nodep); + } + + void visit(AstLoop* nodep) override { + VL_RESTORER(m_inLoop); + m_inLoop = true; + iterateChildrenConst(nodep); + } + + void visit(AstVarRef* nodep) override { + // Disguised hierarchical reference handled as external reference, ignore + if (nodep->classOrPackagep()) return; + // If target is not supported, ignore + Variable* const tgtp = getTarget(nodep); + if (!V3Dfg::isSupported(tgtp)) return; + // V3Dfg::isSupported should reject vars with READWRITE references + UASSERT_OBJ(!nodep->access().isRW(), nodep, "Variable with READWRITE ref not rejected"); + // Get target variable vergtex, ignore if not given one + DfgVertexVar* const varp = m_getVarVertex(tgtp); + if (!varp) return; + // Create Ast reference vertices + if (nodep->access().isReadOnly()) { + DfgAstRd* const astp = new DfgAstRd{m_dfg, nodep, m_inSenItem, m_inLoop}; + astp->srcp(varp); + return; + } + // Mark as written from non-DFG logic + DfgVertexVar::setHasModWrRefs(tgtp); + } + + AstToDfgAddAstRefs(DfgGraph& dfg, AstNode* nodep, + std::function getVarVertex) + : m_dfg{dfg} + , m_getVarVertex{getVarVertex} { + iterateConst(nodep); + } + +public: + static void apply(DfgGraph& dfg, AstNode* nodep, + std::function getVarVertex) { + AstToDfgAddAstRefs{dfg, nodep, getVarVertex}; + } +}; + +void V3DfgPasses::addAstRefs(DfgGraph& dfg, AstNode* nodep, + std::function getVarVertex) { + if (dfg.modulep()) { + AstToDfgAddAstRefs::apply(dfg, nodep, getVarVertex); + } else { + AstToDfgAddAstRefs::apply(dfg, nodep, getVarVertex); + } +} + template class AstToDfgVisitor final : public VNVisitor { // NODE STATE @@ -72,13 +151,9 @@ class AstToDfgVisitor final : public VNVisitor { } // Mark variables referenced under node - static void markReferenced(const AstNode* nodep) { - nodep->foreach([](const AstVarRef* refp) { - Variable* const tgtp = getTarget(refp); - // Mark as read from non-DFG logic - if (refp->access().isReadOrRW()) DfgVertexVar::setHasModRdRefs(tgtp); - // Mark as written from non-DFG logic - if (refp->access().isWriteOrRW()) DfgVertexVar::setHasModWrRefs(tgtp); + void markReferenced(AstNode* nodep) { + V3DfgPasses::addAstRefs(m_dfg, nodep, [this](AstNode* varp) { // + return getVarVertex(static_cast(varp)); }); } @@ -286,7 +361,12 @@ public: AstToDfgVisitor{dfg, root, ctx}; // Remove unread and undriven variables (created when something failed to convert) for (DfgVertexVar* const varp : dfg.varVertices().unlinkable()) { - if (!varp->srcp() && !varp->hasSinks()) VL_DO_DANGLING(varp->unlinkDelete(dfg), varp); + if (varp->srcp()) continue; + const bool keep = varp->foreachSink([&](DfgVertex& d) { return !d.is(); }); + if (!keep) { + while (varp->hasSinks()) varp->firtsSinkp()->unlinkDelete(dfg); + VL_DO_DANGLING(varp->unlinkDelete(dfg), varp); + } } } }; diff --git a/src/V3DfgColorSCCs.cpp b/src/V3DfgColorSCCs.cpp index b1ca2bb05..6bdce8ffb 100644 --- a/src/V3DfgColorSCCs.cpp +++ b/src/V3DfgColorSCCs.cpp @@ -105,6 +105,12 @@ class ColorStronglyConnectedComponents final { component(vtx) = 0; } + // We know ast references have no output or input edges + for (const DfgVertexAst& vtx : m_dfg.astVertices()) { + index(vtx) = 0; + component(vtx) = 0; + } + // Initialize state of variable vertices for (const DfgVertexVar& vtx : m_dfg.varVertices()) { // If it has no inputs or no outputs, it cannot be part of a non-trivial SCC. diff --git a/src/V3DfgContext.h b/src/V3DfgContext.h index dfeb0533e..dd1a05f2f 100644 --- a/src/V3DfgContext.h +++ b/src/V3DfgContext.h @@ -146,6 +146,8 @@ public: VDouble0 m_outputVariables; // Number of output variables VDouble0 m_outputVariablesWithDefault; // Number of outptu variables with a default driver VDouble0 m_resultEquations; // Number of result combinational equations + VDouble0 m_expressionsInlined; // Number of expressions inlined directly into the Ast + VDouble0 m_varRefsSubstituted; // Number of variable references substituted in the Ast private: V3DfgDfgToAstContext(V3DfgContext& ctx, const std::string& label) @@ -154,6 +156,8 @@ private: addStat("output variables", m_outputVariables); addStat("output variables with default driver", m_outputVariablesWithDefault); addStat("result equations", m_resultEquations); + addStat("expressions inlined", m_expressionsInlined); + addStat("var refs substituted", m_varRefsSubstituted); } }; class V3DfgPeepholeContext final : public V3DfgSubContext { @@ -166,6 +170,7 @@ public: std::array m_enabled; // Count of applications for each optimization (for statistics) std::array m_count; + std::vector m_deleteps; // AstVar/AstVarScope that can be deleted at the end private: V3DfgPeepholeContext(V3DfgContext& ctx, const std::string& label) VL_MT_DISABLED; @@ -193,13 +198,10 @@ class V3DfgRegularizeContext final : public V3DfgSubContext { public: // STATE - VDouble0 m_temporariesOmitted; // Number of temporaries omitted as cheaper to re-compute - VDouble0 m_temporariesIntroduced; // Number of temporaries introduced - std::vector m_deleteps; // AstVar/AstVarScope that can be deleted at the end - VDouble0 m_usedVarsReplaced; // Number of used variables replaced with equivalent ones VDouble0 m_usedVarsInlined; // Number of used variables inlined VDouble0 m_unusedRemoved; // Number of unused vertices remoevd + VDouble0 m_temporariesIntroduced; // Number of temporaries introduced for reuse private: V3DfgRegularizeContext(V3DfgContext& ctx, const std::string& label) @@ -208,11 +210,8 @@ private: for (AstNode* const nodep : m_deleteps) { VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); } - addStat("used variables replaced", m_usedVarsReplaced); addStat("used variables inlined", m_usedVarsInlined); addStat("unused vertices removed", m_unusedRemoved); - - addStat("temporaries omitted", m_temporariesOmitted); addStat("temporaries introduced", m_temporariesIntroduced); } }; diff --git a/src/V3DfgCse.cpp b/src/V3DfgCse.cpp index 0257c4117..bfc0cb5c7 100644 --- a/src/V3DfgCse.cpp +++ b/src/V3DfgCse.cpp @@ -53,7 +53,8 @@ class V3DfgCse final { // Special vertices case VDfgType::Const: // LCOV_EXCL_START case VDfgType::VarArray: - case VDfgType::VarPacked: // LCOV_EXCL_STOP + case VDfgType::VarPacked: + case VDfgType::AstRd: // LCOV_EXCL_STOP vtx.v3fatalSrc("Hash should have been pre-computed"); // Vertices with internal information @@ -164,12 +165,16 @@ class V3DfgCse final { case VDfgType::Unresolved: // LCOV_EXCL_STOP a.v3fatalSrc("Should not have reached CSE"); + // Not reachable via operation vertices + case VDfgType::AstRd: // LCOV_EXCL_LINE + a.v3fatalSrc("Should not be reachable via operation vertices"); + // Special vertices case VDfgType::Const: return a.as()->num().isCaseEq(b.as()->num()); case VDfgType::VarArray: - case VDfgType::VarPacked: - return false; // CSE does not combine variables + case VDfgType::VarPacked: // CSE does not combine variables + return false; // Vertices with internal information case VDfgType::Sel: return a.as()->lsb() == b.as()->lsb(); @@ -298,6 +303,8 @@ class V3DfgCse final { // Pre-hash variables, these are all unique, so just set their hash to a unique value uint32_t varHash = 0; for (const DfgVertexVar& vtx : dfg.varVertices()) m_hashCache[vtx] = V3Hash{++varHash}; + // Pre-hash Ast references, these are all unique like variables + for (const DfgVertexAst& vtx : dfg.astVertices()) m_hashCache[vtx] = V3Hash{++varHash}; // Similarly pre-hash constants for speed. While we don't combine constants, we do want // expressions using the same constants to be combined, so we do need to hash equal diff --git a/src/V3DfgDecomposition.cpp b/src/V3DfgDecomposition.cpp index c867adf1a..ccf4f52f7 100644 --- a/src/V3DfgDecomposition.cpp +++ b/src/V3DfgDecomposition.cpp @@ -107,6 +107,7 @@ class SplitIntoComponents final { } // Move the vertices to the component graphs moveVertices(m_dfg.varVertices()); + moveVertices(m_dfg.astVertices()); moveVertices(m_dfg.constVertices()); moveVertices(m_dfg.opVertices()); // @@ -202,12 +203,12 @@ class ExtractCyclicComponents final { } } } - // To ensure all component boundaries are at variables, expand // components to include all reachable non-variable vertices. Constants - // are reachable from their sinks, so only need to process op vertices. - // We do this by staring a DFS from each vertex that is part of an - // component and add all reachable non-variable vertices to the same. + // are reachable from their sinks, and Ast refs cannot be in an SCC, + // so only need to process op vertices. We do this by starting a DFS + // from each vertex that is part of a component and add all reachable + // non-variable vertices to the same. for (DfgVertex& vtx : m_dfg.opVertices()) { if (const uint64_t targetComponent = m_component.at(vtx)) { expandSiblings(vtx, targetComponent); @@ -349,6 +350,7 @@ class ExtractCyclicComponents final { // Move other vertices to their component graphs // After this, vertex states are invalid as we moved the vertices moveVertices(m_dfg.varVertices()); + moveVertices(m_dfg.astVertices()); moveVertices(m_dfg.constVertices()); moveVertices(m_dfg.opVertices()); diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index dd9d08f22..9c354f037 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -268,8 +268,10 @@ class DfgToAstVisitor final : DfgVisitor { , m_ctx{ctx} { if (v3Global.opt.debugCheck()) V3DfgPasses::typeCheck(dfg); - // Convert the graph back to combinational assignments - // The graph must have been regularized, so we only need to render assignments + // Convert the graph back to combinational assignments. The graph must have been + // regularized, so we only need to render variable assignments and update Ast references. + + // Render variable assignments for (DfgVertexVar& vtx : dfg.varVertices()) { // If there is no driver (this vertex is an input to the graph), then nothing to do. if (!vtx.srcp()) { @@ -309,6 +311,30 @@ class DfgToAstVisitor final : DfgVisitor { // convetDriver always clones lhsp VL_DO_DANGLING(lhsp->deleteTree(), lhsp); } + + // Update Ast References + for (DfgVertexAst& vtx : dfg.astVertices()) { + if (DfgAstRd* const rVtxp = vtx.cast()) { + // Render the driver + AstNodeExpr* const exprp = convertDfgVertexToAstNodeExpr(rVtxp->srcp()); + // If it's the same as the reference, do not repalce it so FileLines are preserved + if (exprp->sameTree(rVtxp->exprp())) { + VL_DO_DANGLING(exprp->deleteTree(), exprp); + continue; + } + // Replace the reference with the expression + if (VN_IS(exprp, VarRef)) { + ++m_ctx.m_varRefsSubstituted; + } else { + UASSERT_OBJ(!dfg.modulep(), &vtx, + "Expressions should only be inlined on final scoped run"); + ++m_ctx.m_expressionsInlined; + } + rVtxp->exprp()->replaceWith(exprp); + rVtxp->exprp()->deleteTree(); + rVtxp->exprp(exprp); + } + } } public: diff --git a/src/V3DfgOptimizer.cpp b/src/V3DfgOptimizer.cpp index 77f9a2308..0558c35e8 100644 --- a/src/V3DfgOptimizer.cpp +++ b/src/V3DfgOptimizer.cpp @@ -23,6 +23,7 @@ #include "V3DfgOptimizer.h" #include "V3AstUserAllocator.h" +#include "V3Const.h" #include "V3Dfg.h" #include "V3DfgPasses.h" #include "V3Graph.h" @@ -281,16 +282,10 @@ class DataflowOptimize final { if (hasExtWr) DfgVertexVar::setHasExtWrRefs(vscp); return; } - // TODO: remove once Actives can tolerate NEVER SenItems - if (AstSenItem* senItemp = VN_CAST(nodep, SenItem)) { - senItemp->foreach([](const AstVarRef* refp) { - DfgVertexVar::setHasExtRdRefs(refp->varScopep()); - }); - return; - } // Check direct references if (const AstVarRef* const refp = VN_CAST(nodep, VarRef)) { if (refp->access().isRW()) DfgVertexVar::setHasRWRefs(refp->varScopep()); + UASSERT_OBJ(!refp->classOrPackagep(), refp, "V3Scope should have removed"); return; } } else { @@ -305,7 +300,13 @@ class DataflowOptimize final { } // Check direct references if (const AstVarRef* const refp = VN_CAST(nodep, VarRef)) { - if (refp->access().isRW()) DfgVertexVar::setHasRWRefs(refp->varp()); + AstVar* const varp = refp->varp(); + if (refp->access().isRW()) DfgVertexVar::setHasRWRefs(varp); + // With classOrPackagep set this is a disguised hierarchical reference, mark + if (refp->classOrPackagep()) { + if (refp->access().isReadOrRW()) DfgVertexVar::setHasExtRdRefs(varp); + if (refp->access().isWriteOrRW()) DfgVertexVar::setHasExtWrRefs(varp); + } return; } } @@ -396,6 +397,24 @@ class DataflowOptimize final { endOfStage("regularize", &dfg); } + void removeNeverActives(AstNetlist* netlistp) { + std::vector neverActiveps; + netlistp->foreach([&](AstActive* activep) { + AstSenTree* const senTreep = activep->sentreep(); + if (!senTreep) return; + const AstNode* const nodep = V3Const::constifyEdit(senTreep); + UASSERT_OBJ(nodep == senTreep, nodep, "Should not have been repalced"); + if (senTreep->sensesp()->isNever()) { + UASSERT_OBJ(!senTreep->sensesp()->nextp(), nodep, + "Never senitem should be alone, else the never should be eliminated."); + neverActiveps.emplace_back(activep); + } + }); + for (AstActive* const activep : neverActiveps) { + VL_DO_DANGLING(activep->unlinkFrBack()->deleteTree(), activep); + } + } + DataflowOptimize(AstNetlist* netlistp, const string& label) : m_ctx{label} , m_scoped{!!netlistp->topScopep()} { @@ -440,6 +459,8 @@ class DataflowOptimize final { // Convert back to Ast V3DfgPasses::dfgToAst(*dfgp, m_ctx); endOfStage("dfg-to-ast", dfgp.get()); + // Some sentrees might have become constant, remove them + removeNeverActives(netlistp); } // Reset interned types so the corresponding Ast types can be garbage collected diff --git a/src/V3DfgPasses.cpp b/src/V3DfgPasses.cpp index 6c411138c..3e9d1fd17 100644 --- a/src/V3DfgPasses.cpp +++ b/src/V3DfgPasses.cpp @@ -89,14 +89,20 @@ void V3DfgPasses::removeUnobservable(DfgGraph& dfg, V3DfgContext& dfgCtx) { // Finally remove logic from the Dfg if it drives no variables in the graph. // These should only be those with side effects. for (DfgVertex* const vtxp : dfg.opVertices().unlinkable()) { - if (!vtxp->is()) continue; - if (vtxp->hasSinks()) continue; - // Input variables will be read in Ast code, mark as such - vtxp->foreachSource([](DfgVertex& src) { - src.as()->setHasModRdRefs(); + DfgLogic* const logicp = vtxp->cast(); + if (!logicp) continue; + if (logicp->hasSinks()) continue; + // Input variables will be read in Ast code, add Ast reference vertices + // AstVar/AstVarScope::user2p() -> corresponding DfgVertexVar* in the graph + const VNUser2InUse m_user2InUse; + logicp->foreachSource([](DfgVertex& src) { + src.as()->nodep()->user2p(&src); return false; }); - VL_DO_DANGLING(vtxp->unlinkDelete(dfg), vtxp); + V3DfgPasses::addAstRefs(dfg, logicp->nodep(), [](AstNode* varp) { // + return varp->user2u().to(); + }); + VL_DO_DANGLING(logicp->unlinkDelete(dfg), logicp); ++ctx.m_logicRemoved; } } @@ -319,20 +325,17 @@ void V3DfgPasses::binToOneHot(DfgGraph& dfg, V3DfgBinToOneHotContext& ctx) { const DfgDataType& tabDType = DfgDataType::array(bitDType, nBits); // The index variable - AstVar* const idxVarp = [&]() { - // If there is an existing result variable, use that, otherwise create a new variable - DfgVarPacked* varp = nullptr; - if (DfgVertexVar* const vp = srcp->getResultVar()) { - varp = vp->as(); - } else { - const std::string name = dfg.makeUniqueName("BinToOneHot_Idx", nTables); - varp = dfg.makeNewVar(flp, name, idxDType, nullptr)->as(); - varp->varp()->isInternal(true); - varp->srcp(srcp); - } - varp->setHasModRdRefs(); - return varp->varp(); + DfgVarPacked* idxVtxp = [&]() { + // If there is an existing result variable, use that + if (DfgVertexVar* const vp = srcp->getResultVar()) return vp->as(); + // Otherwise create a new variable + const std::string name = dfg.makeUniqueName("BinToOneHot_Idx", nTables); + DfgVertexVar* const vtxp = dfg.makeNewVar(flp, name, idxDType, nullptr); + vtxp->varp()->isInternal(true); + vtxp->srcp(srcp); + return vtxp->as(); }(); + AstVar* const idxVarp = idxVtxp->varp(); // The previous index variable - we don't need a vertex for this AstVar* const preVarp = [&]() { const std::string name = dfg.makeUniqueName("BinToOneHot_Pre", nTables); @@ -385,16 +388,22 @@ void V3DfgPasses::binToOneHot(DfgGraph& dfg, V3DfgBinToOneHotContext& ctx) { new AstConst{flp, AstConst::BitFalse{}}}); } { // tab[idx] = 1 + AstVarRef* const idxRefp = new AstVarRef{flp, idxVarp, VAccess::READ}; logicp->addStmtsp(new AstAssign{ flp, // new AstArraySel{flp, new AstVarRef{flp, tabVtxp->varp(), VAccess::WRITE}, - new AstVarRef{flp, idxVarp, VAccess::READ}}, // + idxRefp}, // new AstConst{flp, AstConst::BitTrue{}}}); + DfgAstRd* const astRdp = new DfgAstRd{dfg, idxRefp, false, false}; + astRdp->srcp(idxVtxp); } { // pre = idx + AstVarRef* const idxRefp = new AstVarRef{flp, idxVarp, VAccess::READ}; logicp->addStmtsp(new AstAssign{flp, // new AstVarRef{flp, preVarp, VAccess::WRITE}, // - new AstVarRef{flp, idxVarp, VAccess::READ}}); + idxRefp}); + DfgAstRd* const astRdp = new DfgAstRd{dfg, idxRefp, false, false}; + astRdp->srcp(idxVtxp); } // Replace terms with ArraySels diff --git a/src/V3DfgPasses.h b/src/V3DfgPasses.h index cd184022f..da0aa1461 100644 --- a/src/V3DfgPasses.h +++ b/src/V3DfgPasses.h @@ -38,6 +38,12 @@ std::unique_ptr astToDfg(AstModule&, V3DfgContext&) VL_MT_DISABLED; // Same as above, but for the entire netlist, after V3Scope std::unique_ptr astToDfg(AstNetlist&, V3DfgContext&) VL_MT_DISABLED; +// Add DfgVertexAst to the given DfgGraph for all references in the given AstNode. +// The function 'getVarVertex' is used to get the DfgVertexVar for an AstVar/AstVarScope. +// If it returns nullptr, the reference will be ignored. +void addAstRefs(DfgGraph& dfg, AstNode* nodep, + std::function getVarVertex) VL_MT_DISABLED; + // Remove unobservable variabels and logic that drives only such variables void removeUnobservable(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index d40d61114..0b5c15de9 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -65,6 +65,9 @@ V3DfgPeepholeContext::V3DfgPeepholeContext(V3DfgContext& ctx, const std::string& } V3DfgPeepholeContext::~V3DfgPeepholeContext() { + for (AstNode* const nodep : m_deleteps) { + VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); + } const auto emitStat = [this](VDfgPeepholePattern id) { std::string str{id.ascii()}; std::transform(str.begin(), str.end(), str.begin(), [](unsigned char c) { // @@ -248,7 +251,7 @@ class V3DfgPeephole final : public DfgVisitor { if (varp && !varp->isVolatile() && !varp->hasDfgRefs()) { AstNode* const nodep = varp->nodep(); VL_DO_DANGLING(vtxp->unlinkDelete(m_dfg), vtxp); - VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); + m_ctx.m_deleteps.push_back(nodep); } else { VL_DO_DANGLING(vtxp->unlinkDelete(m_dfg), vtxp); } @@ -2575,12 +2578,16 @@ class V3DfgPeephole final : public DfgVisitor { // Otherwise remove if there is only one sink that is not a removable variable bool foundOne = false; - const bool keep = vtxp->srcp()->foreachSink([&](DfgVertex& sink) { + const bool keep = vtxp->srcp()->foreachSink([&](DfgVertex& sink) -> bool { // Ignore non-observable variable sinks. These can be eliminated. if (const DfgVertexVar* const varp = sink.cast()) { if (!varp->hasSinks() && !varp->isObserved()) return false; } + // Keep before final scoped run if feeds an Ast reference + if (sink.is() && m_dfg.modulep()) return true; + // Keep if found more than one sink if (foundOne) return true; + // Mark first sink found foundOne = true; return false; }); @@ -2634,6 +2641,9 @@ class V3DfgPeephole final : public DfgVisitor { continue; } + // Ast references are not considered + if (m_vtxp->is()) continue; + // Unsued vertices should have been removed immediately UASSERT_OBJ(m_vtxp->hasSinks(), m_vtxp, "Operation vertex should have sinks"); diff --git a/src/V3DfgRegularize.cpp b/src/V3DfgRegularize.cpp index 5f397f4ac..3f9a29af5 100644 --- a/src/V3DfgRegularize.cpp +++ b/src/V3DfgRegularize.cpp @@ -38,8 +38,7 @@ class DfgRegularize final { // For all operation vetices, if they drive multiple variables, pick // a "canonical" one and uninline the logic through that variable. void uninlineVariables() { - // Const vertices we can just emit as drivers to multiple sinks - // directly. Variable vertices, would have been inlined if equivalent, + // Variable vertices, would have been inlined if equivalent, // so no need to process them here, they are where they must be. for (DfgVertex& vtx : m_dfg.opVertices()) { // Don't process LValue operations @@ -55,6 +54,26 @@ class DfgRegularize final { vtx.replaceWith(varp); varp->srcp(&vtx); } + + // Const vertices driving an Ast reference can only be inlined in scoped + // mode as some algorithms assume VarRefs in certain places. + if (m_dfg.modulep()) { + for (DfgConst& vtx : m_dfg.constVertices()) { + const bool drivesAstRef = vtx.foreachSink([](const DfgVertex& dst) { // + return dst.is(); + }); + if (!drivesAstRef) continue; + + // The prefered result variable is the canonical one if exists + DfgVertexVar* const varp = vtx.getResultVar(); + if (!varp) continue; + + // Relink all other sinks reading this vertex to read 'varp' + varp->srcp(nullptr); + vtx.replaceWith(varp); + varp->srcp(&vtx); + } + } } std::unordered_set gatherCyclicVariables() { @@ -72,74 +91,55 @@ class DfgRegularize final { if (const DfgVertexVar* const varp = vtx.cast()) { // There is only one Dfg when running this pass UASSERT_OBJ(!varp->hasDfgRefs(), varp, "Should not have refs in other DfgGraph"); - if (varp->hasModRefs()) return false; + if (varp->hasModWrRefs()) return false; if (varp->hasExtRefs()) return false; } return true; } - // Given a variable and its driver, return true iff the variable can be - // replaced with its driver. Record replacement to be applied in the Ast - // in user2p of the replaced variable. - bool replaceable(DfgVertexVar* varp, DfgVertex* srcp) { - // The given variable has no external references, and is read in the module + // Predicate to determine if a temporary should be inserted or if a variable + // should be preserved. The given vertices are either the same, or aVtxp is + // the sole driver of bVtx, or aVtxp is cheaper to recompute and might have + // multiple sinks. In either case, bVtx can be used to check sinks, and aVtx + // can be used to check the operation. + bool needsTemporary(DfgVertex& aVtx, DfgVertex& bVtx) { + UASSERT_OBJ(&aVtx == &bVtx || aVtx.isCheaperThanLoad() || aVtx.singleSink() == &bVtx, + &aVtx, "Mismatched vertices"); + UASSERT_OBJ(!aVtx.is(), &aVtx, "Should be an operation vertex"); - // Make sure we are not trying to double replace something - AstNode* const nodep = varp->nodep(); - UASSERT_OBJ(!nodep->user2p(), nodep, "Replacement already exists"); + if (bVtx.hasMultipleSinks()) { + // We are not inlining expressions prior to the final scoped run + if (m_dfg.modulep()) return true; - // If it is driven from another variable, it can be replaced by that variable. - if (const DfgVarPacked* const drvp = srcp->cast()) { - // Record replacement - nodep->user2p(drvp->nodep()); - // The replacement will be read in the module, mark as such so it doesn't get removed. - drvp->setHasModRdRefs(); - drvp->varp()->propagateAttrFrom(varp->varp()); - return true; + // Add a temporary if it's cheaper to store and load from memory than recompute + if (!aVtx.isCheaperThanLoad()) return true; + + // Not adding temporary + return false; } - // Expressions can only be inlined after V3Scope, as some passes assume variables. - if (m_dfg.modulep()) return false; + DfgVertex& sink = *bVtx.singleSink(); - // If it is driven from a constant, it can be replaced by that constant. - if (const DfgConst* const constp = srcp->cast()) { - // Need to create the AstConst - AstConst* const newp = new AstConst{constp->fileline(), constp->num()}; - m_deleter.pushDeletep(newp); - // Record replacement - nodep->user2p(newp); - return true; + // No need to add a temporary if the single sink is a variable already + if (sink.is()) return false; + + // Do not inline expressions prior to the final scoped run, or if they are in a loop + if (const DfgAstRd* const astRdp = sink.cast()) { + return m_dfg.modulep() || astRdp->inLoop(); } - // Don't replace + // Make sure roots of wide concatenation trees are written to variables, + // this enables V3FuncOpt to split them which can be a big speed gain + // without expanding them. + if (aVtx.is()) { + if (sink.is()) return false; // Not root of tree + return VL_WORDS_I(static_cast(aVtx.width())) > v3Global.opt.expandLimit(); + } + + // No need for a temporary otherwise return false; } - template - static void applyReplacement(AstVarRef* refp) { - AstNode* const tgtp = T_Scoped ? static_cast(refp->varScopep()) - : static_cast(refp->varp()); - AstNode* replacementp = tgtp; - while (AstNode* const altp = replacementp->user2p()) replacementp = altp; - if (replacementp == tgtp) return; - UASSERT_OBJ(refp->access().isReadOnly(), refp, "Replacing write AstVarRef"); - // If it's an inlined expression, repalce the VarRef entirely - if (AstNodeExpr* const newp = VN_CAST(replacementp, NodeExpr)) { - refp->replaceWith(newp->cloneTreePure(false)); - VL_DO_DANGLING(refp->deleteTree(), refp); - return; - } - // Otherwise just re-point the VarRef to the new variable - if VL_CONSTEXPR_CXX17 (T_Scoped) { - AstVarScope* const newp = VN_AS(replacementp, VarScope); - refp->varScopep(newp); - refp->varp(newp->varp()); - } else { - AstVar* const newp = VN_AS(replacementp, Var); - refp->varp(newp); - } - } - void eliminateVars() { // Although we could eliminate some circular variables, doing so would // make UNOPTFLAT traces fairly usesless, so we will not do so. @@ -150,13 +150,10 @@ class DfgRegularize final { // Add all variables and all vertices with no sinks to the worklist m_dfg.forEachVertex([&](DfgVertex& vtx) { + if (vtx.is()) return; if (vtx.is() || !vtx.hasSinks()) workList.push_front(vtx); }); - // AstVar::user2p() : AstVar*/AstNodeExpr* -> The replacement variable/expression - // AstVarScope::user2p() : AstVarScope*/AstNodeExpr* -> The replacement variable/expression - const VNUser2InUse user2InUse; - // Remove vertex, enqueue it's sources const auto removeVertex = [&](DfgVertex& vtx) { // Add sources of removed vertex to work list @@ -172,9 +169,6 @@ class DfgRegularize final { vtx.unlinkDelete(m_dfg); }; - // Used to check if we need to apply variable replacements - const VDouble0 usedVarsReplacedBefore = m_ctx.m_usedVarsReplaced; - // Process the work list workList.foreach([&](DfgVertex& vtx) { // Remove unused vertices @@ -192,8 +186,6 @@ class DfgRegularize final { DfgVertex* const srcp = varp->srcp(); if (!srcp) return; - // If it has multiple sinks, it can't be eliminated - would increase logic size - if (varp->hasMultipleSinks()) return; // Can't eliminate if referenced external to the module - can't replace those refs if (varp->hasExtRefs()) return; // Can't eliminate if written in the module - the write needs to go somewhere, and @@ -202,50 +194,24 @@ class DfgRegularize final { // There is only one Dfg when running this pass UASSERT_OBJ(!varp->hasDfgRefs(), varp, "Should not have refs in other DfgGraph"); - // At this point, the variable is used, driven only in this Dfg, - // it has exactly 0 or 1 sinks in this Dfg, and might be read in - // the host module, but no other references exist. - // Do not eliminate circular variables - need to preserve UNOPTFLAT traces if (circularVariables.count(varp)) return; - // If it is not read in the module, it can be inlined into the - // Dfg unless partially driven (the partial driver network - // can't be fed into arbitrary logic. TODO: we should peeophole - // these away entirely). - if (!varp->hasModRdRefs()) { - UASSERT_OBJ(varp->hasSinks(), varp, "Shouldn't have made it here without sinks"); - // Don't inline if partially driven - if (varp->defaultp()) return; - if (srcp->is()) return; - if (srcp->is()) return; - // Inline this variable into its single sink - ++m_ctx.m_usedVarsInlined; - varp->replaceWith(varp->srcp()); - removeVertex(*varp); - return; - } + // Do not inline if partially driven (the partial driver network can't be fed into + // arbitrary logic. TODO: we should peeophole these away entirely) + if (varp->defaultp()) return; + if (srcp->is()) return; + if (srcp->is()) return; - // The varable is read in the module. We might still be able to replace it. - if (replaceable(varp, srcp)) { - // Replace this variable with its driver - ++m_ctx.m_usedVarsReplaced; - // Inline it if it has a sink - if (varp->hasSinks()) varp->replaceWith(srcp); - // Delete the repalced variabel - removeVertex(*varp); - } + // Do not eliminate variables that are driven from a vertex that needs a temporary + if (!srcp->is() && needsTemporary(*srcp, *varp)) return; + + // Inline this variable into its single sink + ++m_ctx.m_usedVarsInlined; + varp->replaceWith(varp->srcp()); + removeVertex(*varp); + return; }); - - // Job done if no replacements need to be applied - if (m_ctx.m_usedVarsReplaced == usedVarsReplacedBefore) return; - - // Apply variable replacements - if (AstModule* const modp = m_dfg.modulep()) { - modp->foreach([](AstVarRef* refp) { applyReplacement(refp); }); - } else { - v3Global.rootp()->foreach([](AstVarRef* refp) { applyReplacement(refp); }); - } } void insertTemporaries() { @@ -257,32 +223,19 @@ class DfgRegularize final { // Ensure intermediate values used multiple times are written to variables for (DfgVertex& vtx : m_dfg.opVertices()) { - // LValue vertices feed into variables eventually and need not temporaries + // LValue vertices feed into variables eventually and need no temporaries if (vtx.is()) continue; if (vtx.is()) continue; - // If this Vertex was driving a variable, 'unlinline' would have - // made that the single sink, so if there are multiple sinks - // remaining, they must be non-variables. So nothing to do if: - if (!vtx.hasMultipleSinks()) continue; - - // 'uninline' would have made the result var cannonical, so there shouldn't be one - UASSERT_OBJ(!vtx.getResultVar(), &vtx, "Failed to uninline variable"); - - // Do not add a temporary if it's cheaper to re-compute than to - // load it from memory. This also increases available parallelism. - if (vtx.isCheaperThanLoad()) { - ++m_ctx.m_temporariesOmitted; - continue; - } + if (!needsTemporary(vtx, vtx)) continue; // Need to create an intermediate variable + ++m_ctx.m_temporariesIntroduced; const std::string name = m_dfg.makeUniqueName("Regularize", m_nTmps); FileLine* const flp = vtx.fileline(); AstScope* const scopep = scoped ? vtx.scopep(scopeCache) : nullptr; DfgVertexVar* const newp = m_dfg.makeNewVar(flp, name, vtx.dtype(), scopep); ++m_nTmps; - ++m_ctx.m_temporariesIntroduced; // Replace vertex with the variable, make it drive the variable vtx.replaceWith(newp); newp->srcp(&vtx); diff --git a/src/V3DfgSynthesize.cpp b/src/V3DfgSynthesize.cpp index 9210dffa2..36333a950 100644 --- a/src/V3DfgSynthesize.cpp +++ b/src/V3DfgSynthesize.cpp @@ -492,9 +492,12 @@ public: return false; } // For now, only direct array assignment is supported (e.g. a = b, but not a = _ ? b : c) - if (rDtypep->isArray() && !VN_IS(rhsp, VarRef)) { - ++m_ctx.m_conv.nonRepDType; - return false; + if (rDtypep->isArray()) { + if (!VN_IS(rhsp, VarRef) + || !lhsp->dtypep()->skipRefp()->sameTree(rhsp->dtypep()->skipRefp())) { + ++m_ctx.m_conv.nonRepDType; + return false; + } } // Widths should match at this point UASSERT_OBJ(lhsp->width() == rhsp->width(), nodep, "Mismatched width reached DFG"); @@ -1746,11 +1749,16 @@ class AstToDfgSynthesize final { if (logicp->selectedForSynthesis()) continue; // There should be no sinks left for unselected DfgLogic, delete them here UASSERT_OBJ(!logicp->hasSinks(), vtxp, "Unselected 'DfgLogic' with sinks remaining"); - // Input variables will be read in Ast code, mark as such + // Input variables will be read in Ast code, add Ast reference vertices + // AstVar/AstVarScope::user4p() -> corresponding DfgVertexVar* in the graph + const VNUser4InUse m_user4InUse; logicp->foreachSource([](DfgVertex& src) { - src.as()->setHasModRdRefs(); + src.as()->nodep()->user4p(&src); return false; }); + V3DfgPasses::addAstRefs(m_dfg, logicp->nodep(), [](AstNode* varp) { // + return varp->user4u().to(); + }); VL_DO_DANGLING(logicp->unlinkDelete(m_dfg), logicp); } debugDump("synth-selected"); @@ -1856,12 +1864,17 @@ class AstToDfgSynthesize final { // If synthesized, delete the corresponding AstNode. It is now in Dfg. logicp->nodep()->unlinkFrBack()->deleteTree(); } else { - // Not synthesized. Logic stays in Ast. Mark source variables - //as read in module. Outputs already marked by revertTransivelyAndRemove. - logicp->foreachSource([](DfgVertex& src) { // - src.as()->setHasModRdRefs(); + // Not synthesized. Logic stays in Ast. Add Ast reference vertices. + // Outputs already marked by revertTransivelyAndRemove. + // AstVar/AstVarScope::user4p() -> corresponding DfgVertexVar* in the graph + const VNUser4InUse m_user4InUse; + logicp->foreachSource([](DfgVertex& src) { + src.as()->nodep()->user4p(&src); return false; }); + V3DfgPasses::addAstRefs(m_dfg, logicp->nodep(), [](AstNode* varp) { // + return varp->user4u().to(); + }); } // Delete this DfgLogic diff --git a/src/V3DfgVertices.h b/src/V3DfgVertices.h index e6ba29161..82719e7d8 100644 --- a/src/V3DfgVertices.h +++ b/src/V3DfgVertices.h @@ -121,14 +121,9 @@ public: bool hasDfgRefs() const { return nodep()->user1() >> 6; } // I.e.: (nodep()->user1() >> 5) > 1 // Variable referenced from Ast code in the same module/netlist - static bool hasModRdRefs(const AstNode* nodep) { return nodep->user1() & 0x04; } static bool hasModWrRefs(const AstNode* nodep) { return nodep->user1() & 0x08; } - static void setHasModRdRefs(AstNode* nodep) { nodep->user1(nodep->user1() | 0x04); } static void setHasModWrRefs(AstNode* nodep) { nodep->user1(nodep->user1() | 0x08); } - bool hasModRdRefs() const { return hasModRdRefs(nodep()); } bool hasModWrRefs() const { return hasModWrRefs(nodep()); } - bool hasModRefs() const { return hasModRdRefs() || hasModWrRefs(); } - void setHasModRdRefs() const { setHasModRdRefs(nodep()); } void setHasModWrRefs() const { setHasModWrRefs(nodep()); } // Variable referenced outside the containing module/netlist. @@ -149,7 +144,7 @@ public: // A DfgVarVertex is written in exactly one DfgGraph, and might be read in an arbitrary // number of other DfgGraphs. If it's driven in this DfgGraph, it's read in others. if (hasDfgRefs()) return srcp() || defaultp(); - return hasExtRdRefs() || hasModRdRefs(); + return hasExtRdRefs(); } // The value of this vertex might differ from what is defined by its drivers @@ -193,6 +188,48 @@ public: ASTGEN_MEMBERS_DfgVarPacked; }; +//------------------------------------------------------------------------------ +// Ast reference vertices + +class DfgVertexAst VL_NOT_FINAL : public DfgVertex { + // Represents a reference in the Ast + + AstNodeExpr* m_exprp; // The AstNodeExpr representing this reference + +public: + DfgVertexAst(DfgGraph& dfg, VDfgType type, AstNodeExpr* exprp) + : DfgVertex{dfg, type, exprp->fileline(), *DfgDataType::fromAst(exprp->dtypep())} + , m_exprp{exprp} {} + ASTGEN_MEMBERS_DfgVertexAst; + + AstNodeExpr* exprp() const { return m_exprp; } + void exprp(AstNodeExpr* exprp) { m_exprp = exprp; } +}; + +class DfgAstRd final : public DfgVertexAst { + friend class DfgVertex; + friend class DfgVisitor; + + const bool m_inSenItem; // Reference is in a sensitivity list + const bool m_inLoop; // Reference is in a loop + +public: + DfgAstRd(DfgGraph& dfg, AstNodeExpr* exprp, bool inSenItem, bool inLoop) + : DfgVertexAst{dfg, dfgType(), exprp} + , m_inSenItem{inSenItem} + , m_inLoop{inLoop} { + // Allocate sources + newInput(); + } + ASTGEN_MEMBERS_DfgAstRd; + + DfgVertex* srcp() const { return inputp(0); } + void srcp(DfgVertex* vtxp) { inputp(0, vtxp); } + std::string srcName(size_t) const override final { return ""; } + bool inSenItem() const { return m_inSenItem; } + bool inLoop() const { return m_inLoop; } +}; + //------------------------------------------------------------------------------ // Nullary vertices - 0 inputs diff --git a/test_regress/t/t_debug_width.out b/test_regress/t/t_debug_width.out index 34d60fbbf..0054baeda 100644 --- a/test_regress/t/t_debug_width.out +++ b/test_regress/t/t_debug_width.out @@ -1,4 +1,4 @@ -%Error: Internal Error: t/t_opt_const.v:12:8: ../V3Ast.cpp:#: widthMismatch detected 'lhsp()->widthMin() != rhsp()->widthMin()' @ ../V3AstNodes.cpp:#OUT:(G/wu32/1) LHS:(G/w32) RHS:(G/wu32/1) - 12 | module t( - | ^ +%Error: Internal Error: t/t_opt_const.v:142:26: ../V3Ast.cpp:#: widthMismatch detected 'lhsp()->widthMin() != rhsp()->widthMin()' @ ../V3AstNodes.cpp:#OUT:(G/wu32/5) LHS:(G/w32) RHS:(G/wu32/5) + 142 | bug3182 i_bug3182(.in(d[4:0]), .out(bug3182_out)); + | ^ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. diff --git a/test_regress/t/t_dfg_astrefs.out b/test_regress/t/t_dfg_astrefs.out new file mode 100644 index 000000000..06f548501 --- /dev/null +++ b/test_regress/t/t_dfg_astrefs.out @@ -0,0 +1,31 @@ + 5 i == '{ 0: 255, 1: 255, 2: 255, 3: 255, 4: 255, 5: 255, 6: 255, 7: 255} + 15 i == '{ 0: 255, 1: 254, 2: 253, 3: 252, 4: 251, 5: 250, 6: 249, 7: 248} + 25 i == '{ 0: 255, 1: 253, 2: 251, 3: 249, 4: 247, 5: 245, 6: 243, 7: 241} + 35 i == '{ 0: 255, 1: 252, 2: 249, 3: 246, 4: 243, 5: 240, 6: 237, 7: 234} + 45 i == '{ 0: 255, 1: 251, 2: 247, 3: 243, 4: 239, 5: 235, 6: 231, 7: 227} + 55 i == '{ 0: 255, 1: 250, 2: 245, 3: 240, 4: 235, 5: 230, 6: 225, 7: 220} + 65 i == '{ 0: 255, 1: 249, 2: 243, 3: 237, 4: 231, 5: 225, 6: 219, 7: 213} + 75 i == '{ 0: 255, 1: 248, 2: 241, 3: 234, 4: 227, 5: 220, 6: 213, 7: 206} + 85 i == '{ 0: 255, 1: 247, 2: 239, 3: 231, 4: 223, 5: 215, 6: 207, 7: 199} + 95 i == '{ 0: 255, 1: 246, 2: 237, 3: 228, 4: 219, 5: 210, 6: 201, 7: 192} + 105 i == '{ 0: 255, 1: 245, 2: 235, 3: 225, 4: 215, 5: 205, 6: 195, 7: 185} + 115 i == '{ 0: 255, 1: 244, 2: 233, 3: 222, 4: 211, 5: 200, 6: 189, 7: 178} + 125 i == '{ 0: 255, 1: 243, 2: 231, 3: 219, 4: 207, 5: 195, 6: 183, 7: 171} + 135 i == '{ 0: 255, 1: 242, 2: 229, 3: 216, 4: 203, 5: 190, 6: 177, 7: 164} + 145 i == '{ 0: 255, 1: 241, 2: 227, 3: 213, 4: 199, 5: 185, 6: 171, 7: 157} + 155 i == '{ 0: 255, 1: 240, 2: 225, 3: 210, 4: 195, 5: 180, 6: 165, 7: 150} + 165 i == '{ 0: 255, 1: 239, 2: 223, 3: 207, 4: 191, 5: 175, 6: 159, 7: 143} + 175 i == '{ 0: 255, 1: 238, 2: 221, 3: 204, 4: 187, 5: 170, 6: 153, 7: 136} + 185 i == '{ 0: 255, 1: 237, 2: 219, 3: 201, 4: 183, 5: 165, 6: 147, 7: 129} + 195 i == '{ 0: 255, 1: 236, 2: 217, 3: 198, 4: 179, 5: 160, 6: 141, 7: 122} + 205 i == '{ 0: 255, 1: 235, 2: 215, 3: 195, 4: 175, 5: 155, 6: 135, 7: 115} + 215 i == '{ 0: 255, 1: 234, 2: 213, 3: 192, 4: 171, 5: 150, 6: 129, 7: 108} + 225 i == '{ 0: 255, 1: 233, 2: 211, 3: 189, 4: 167, 5: 145, 6: 123, 7: 101} + 235 i == '{ 0: 255, 1: 232, 2: 209, 3: 186, 4: 163, 5: 140, 6: 117, 7: 94} + 245 i == '{ 0: 255, 1: 231, 2: 207, 3: 183, 4: 159, 5: 135, 6: 111, 7: 87} + 255 i == '{ 0: 255, 1: 230, 2: 205, 3: 180, 4: 155, 5: 130, 6: 105, 7: 80} + 265 i == '{ 0: 255, 1: 229, 2: 203, 3: 177, 4: 151, 5: 125, 6: 99, 7: 73} + 275 i == '{ 0: 255, 1: 228, 2: 201, 3: 174, 4: 147, 5: 120, 6: 93, 7: 66} + 285 i == '{ 0: 255, 1: 227, 2: 199, 3: 171, 4: 143, 5: 115, 6: 87, 7: 59} + 295 i == '{ 0: 255, 1: 226, 2: 197, 3: 168, 4: 139, 5: 110, 6: 81, 7: 52} +*-* All Finished *-* diff --git a/test_regress/t/t_dfg_astrefs.py b/test_regress/t/t_dfg_astrefs.py new file mode 100755 index 000000000..fe7150491 --- /dev/null +++ b/test_regress/t/t_dfg_astrefs.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('vlt_all') + +test.compile(verilator_flags2=["--binary"]) + +test.execute(expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_dfg_astrefs.v b/test_regress/t/t_dfg_astrefs.v new file mode 100644 index 000000000..845e117ac --- /dev/null +++ b/test_regress/t/t_dfg_astrefs.v @@ -0,0 +1,47 @@ +// 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 + +// verilog_format: off +`define stop $stop +`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d (%s !== %s)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0); +// verilog_format: on + +module t; + + logic clk = 0; + always #5 clk = ~clk; + + initial begin + #300; + $write("*-* All Finished *-*\n"); + $finish; + end + + localparam int N = 8; + + logic [N-1:0][7:0] i = '0; + logic [N-1:0][7:0] o; + + always @(posedge clk) begin + for (int n = 0; n < N; ++n) begin + i[n] <= i[n] + 8'(n); + end + end + + for (genvar n = 0; n < N; ++n) begin + assign o[n] = ~i[n]; + end + + always @(posedge clk) begin + $write("%05t i == '{", $time); + for (int n = 0; n < N; ++n) begin + if (n > 0) $write(", "); + $write("%2d: %4d", n, o[n]); + end + $write("}\n"); + end + +endmodule diff --git a/test_regress/t/t_dfg_inline_const_clock.py b/test_regress/t/t_dfg_inline_const_clock.py new file mode 100755 index 000000000..c87a1e49f --- /dev/null +++ b/test_regress/t/t_dfg_inline_const_clock.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('vlt') + +test.compile(verilator_flags2=["--binary"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_dfg_inline_const_clock.v b/test_regress/t/t_dfg_inline_const_clock.v new file mode 100644 index 000000000..5b1146427 --- /dev/null +++ b/test_regress/t/t_dfg_inline_const_clock.v @@ -0,0 +1,25 @@ +// 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 + +module t; + + bit clk = 1'b0; + always #5 clk = ~clk; + + initial begin + #300; + $write("*-* All Finished *-*\n"); + $finish; + end + + // Complicated way to write constant 0 that only Dfg can decipher + wire [1:0] constant = 2'b0 ^ (({2{clk}} & ~{2{clk}})); + + always @(posedge constant[0]) begin + $stop; + end + +endmodule diff --git a/test_regress/t/t_dfg_stats_patterns_post_inline.out b/test_regress/t/t_dfg_stats_patterns_post_inline.out index 993127bf2..37d092dca 100644 --- a/test_regress/t/t_dfg_stats_patterns_post_inline.out +++ b/test_regress/t/t_dfg_stats_patterns_post_inline.out @@ -1,6 +1,7 @@ DFG 'post inline' patterns with depth 1 9 (CONCAT _A:1 _B:a):b 8 (REDXOR _A:a):1 + 4 (ASTRD vA:a):a 3 (NOT vA:a)*:a 2 (AND _A:a _B:a):a 1 (AND _A:a _B:a)*:a diff --git a/test_regress/t/t_opt_dedupe_clk_gate.py b/test_regress/t/t_opt_dedupe_clk_gate.py index 1125e65e5..02778fcec 100755 --- a/test_regress/t/t_opt_dedupe_clk_gate.py +++ b/test_regress/t/t_opt_dedupe_clk_gate.py @@ -20,6 +20,6 @@ if test.vlt_all: out_filename, r'{"type":"VAR","name":"t.f0.clock_gate.clken_latched","addr":"[^"]*","loc":"\w,44:[^"]*","dtypep":"\(\w+\)",.*"origName":"clken_latched",.*"isLatched":true,.*"dtypeName":"logic"' ) - test.file_grep(test.stats, r'Optimizations, Gate sigs deduped\s+(\d+)', 4) + test.file_grep(test.stats, r'Optimizations, Gate sigs deduped\s+(\d+)', 2) test.passes()