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()