From d2edab458e8e6c7222eca3de35a3f9ec60aa0afc Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 5 Aug 2025 10:24:54 +0100 Subject: [PATCH] Refactor Dfg variable flags (#6259) Store all flags in a DfgVertexVar relating to the underlying AstVar/AstVarScope stored via AstNode::user1(). user2/user3/user4 are then usable by DFG algorithms as needed. --- src/V3Dfg.cpp | 27 ++++++------ src/V3DfgAstToDfg.cpp | 85 +++++++++++--------------------------- src/V3DfgDecomposition.cpp | 5 +-- src/V3DfgDfgToAst.cpp | 14 +++---- src/V3DfgOptimizer.cpp | 25 ++++++----- src/V3DfgPasses.cpp | 27 ++++++------ src/V3DfgVertices.h | 47 +++++++++++---------- src/astgen | 8 ++-- 8 files changed, 102 insertions(+), 136 deletions(-) diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index e57f5095c..5f56a80d4 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -77,10 +77,6 @@ std::unique_ptr DfgGraph::clone() const { break; } } - - if (vp->hasDfgRefs()) cp->setHasDfgRefs(); - if (vp->hasModRefs()) cp->setHasModRefs(); - if (vp->hasExtRefs()) cp->setHasExtRefs(); } // Clone operation vertices for (const DfgVertex& vtx : m_opVertices) { @@ -262,8 +258,6 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { os << ", shape=box, style=filled, fillcolor=darkorange1"; // Orange } else if (varVtxp->hasDfgRefs()) { os << ", shape=box, style=filled, fillcolor=gold2"; // Yellow - } else if (varVtxp->keep()) { - os << ", shape=box, style=filled, fillcolor=grey"; } else { os << ", shape=box"; } @@ -290,8 +284,6 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { os << ", shape=box3d, style=filled, fillcolor=darkorange1"; // Orange } else if (arrVtxp->hasDfgRefs()) { os << ", shape=box3d, style=filled, fillcolor=gold2"; // Yellow - } else if (arrVtxp->keep()) { - os << ", shape=box3d, style=filled, fillcolor=grey"; } else { os << ", shape=box3d"; } @@ -597,16 +589,23 @@ DfgVertexVar* DfgVertex::getResultVar() { resp = varp; return; } + // Prefer those variables that must be kept anyway - const bool keepOld = resp->keep() || resp->hasDfgRefs(); - const bool keepNew = varp->keep() || varp->hasDfgRefs(); - if (keepOld != keepNew) { - if (!keepOld) resp = varp; + if (resp->hasExtRefs() != varp->hasExtRefs()) { + if (!resp->hasExtRefs()) resp = varp; + return; + } + if (resp->hasModWrRefs() != varp->hasModWrRefs()) { + if (!resp->hasModWrRefs()) resp = varp; + return; + } + if (resp->hasDfgRefs() != varp->hasDfgRefs()) { + if (!resp->hasDfgRefs()) resp = varp; return; } // Prefer those that already have module references - if (resp->hasModRefs() != varp->hasModRefs()) { - if (!resp->hasModRefs()) resp = varp; + if (resp->hasModRdRefs() != varp->hasModRdRefs()) { + if (!resp->hasModRdRefs()) resp = varp; return; } // Prefer the earlier one in source order diff --git a/src/V3DfgAstToDfg.cpp b/src/V3DfgAstToDfg.cpp index ce9604523..760efd9b3 100644 --- a/src/V3DfgAstToDfg.cpp +++ b/src/V3DfgAstToDfg.cpp @@ -87,8 +87,8 @@ template class AstToDfgVisitor final : public VNVisitor { // NODE STATE - // AstNode::user1p // DfgVertex for this AstNode - const VNUser1InUse m_user1InUse; + // AstNode::user2p // DfgVertex for this AstNode + const VNUser2InUse m_user2InUse; // TYPES using RootType = std::conditional_t; @@ -124,15 +124,12 @@ class AstToDfgVisitor final : public VNVisitor { } void markReferenced(AstNode* nodep) { - nodep->foreach([this](const AstVarRef* refp) { - // No need to (and in fact cannot) mark variables if: - if (!DfgVertex::isSupportedDType(refp->varp()->dtypep())) return; // unsupported type - if (refp->varp()->isSc()) return; // SystemC + nodep->foreach([](const AstVarRef* refp) { VariableType* const tgtp = getTarget(refp); - // Mark vertex as having a module reference outside current DFG - getNet(tgtp)->setHasModRefs(); - // Mark variable as written from non-DFG logic - if (refp->access().isWriteOrRW()) tgtp->user3(true); + // 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); }); } @@ -144,27 +141,27 @@ class AstToDfgVisitor final : public VNVisitor { } DfgVertexVar* getNet(VariableType* vp) { - if (!vp->user1p()) { + if (!vp->user2p()) { // vp DfgVertexVar vertices are not added to m_uncommittedVertices, because we - // want to hold onto them via AstVar::user1p, and the AstVar might be referenced via + // want to hold onto them via AstVar::user2p, and the AstVar might be referenced via // multiple AstVarRef instances, so we will never revert a DfgVertexVar once // created. We will delete unconnected variable vertices at the end. if (VN_IS(vp->dtypep()->skipRefp(), UnpackArrayDType)) { DfgVarArray* const vtxp = new DfgVarArray{*m_dfgp, vp}; - vp->user1p(); + vp->user2p(); m_varArrayps.push_back(vtxp); - vp->user1p(vtxp); + vp->user2p(vtxp); } else { DfgVarPacked* const vtxp = new DfgVarPacked{*m_dfgp, vp}; m_varPackedps.push_back(vtxp); - vp->user1p(vtxp); + vp->user2p(vtxp); } } - return vp->user1u().template to(); + return vp->user2u().template to(); } DfgVertex* getVertex(AstNode* nodep) { - DfgVertex* vtxp = nodep->user1u().to(); + DfgVertex* vtxp = nodep->user2u().to(); UASSERT_OBJ(vtxp, nodep, "Missing Dfg vertex"); return vtxp; } @@ -713,42 +710,6 @@ class AstToDfgVisitor final : public VNVisitor { void visit(AstCell* nodep) override { markReferenced(nodep); } void visit(AstNodeProcedure* nodep) override { markReferenced(nodep); } - void visit(AstVar* nodep) override { - if VL_CONSTEXPR_CXX17 (T_Scoped) { - return; - } else { - if (nodep->isSc()) return; - // No need to (and in fact cannot) handle variables with unsupported dtypes - if (!DfgVertex::isSupportedDType(nodep->dtypep())) return; - - // Mark variables with external references - if (nodep->isIO() // Ports - || nodep->user2() // Target of a hierarchical reference - || nodep->isForced() // Forced - ) { - getNet(reinterpret_cast(nodep))->setHasExtRefs(); - } - } - } - - void visit(AstVarScope* nodep) override { - if VL_CONSTEXPR_CXX17 (!T_Scoped) { - return; - } else { - if (nodep->varp()->isSc()) return; - // No need to (and in fact cannot) handle variables with unsupported dtypes - if (!DfgVertex::isSupportedDType(nodep->dtypep())) return; - - // Mark variables with external references - if (nodep->varp()->isIO() // Ports - || nodep->user2() // Target of a hierarchical reference - || nodep->varp()->isForced() // Forced - ) { - getNet(reinterpret_cast(nodep))->setHasExtRefs(); - } - } - } - void visit(AstAssignW* nodep) override { ++m_ctx.m_inputEquations; @@ -820,7 +781,7 @@ class AstToDfgVisitor final : public VNVisitor { } void visit(AstVarRef* nodep) override { - UASSERT_OBJ(!nodep->user1p(), nodep, "Already has Dfg vertex"); + UASSERT_OBJ(!nodep->user2p(), nodep, "Already has Dfg vertex"); if (unhandled(nodep)) return; if (nodep->access().isRW() // Cannot represent read-write references @@ -853,19 +814,19 @@ class AstToDfgVisitor final : public VNVisitor { return; } - nodep->user1p(getNet(getTarget(nodep))); + nodep->user2p(getNet(getTarget(nodep))); } void visit(AstConst* nodep) override { - UASSERT_OBJ(!nodep->user1p(), nodep, "Already has Dfg vertex"); + UASSERT_OBJ(!nodep->user2p(), nodep, "Already has Dfg vertex"); if (unhandled(nodep)) return; DfgVertex* const vtxp = new DfgConst{*m_dfgp, nodep->fileline(), nodep->num()}; m_uncommittedVertices.push_back(vtxp); - nodep->user1p(vtxp); + nodep->user2p(vtxp); } void visit(AstSel* nodep) override { - UASSERT_OBJ(!nodep->user1p(), nodep, "Already has Dfg vertex"); + UASSERT_OBJ(!nodep->user2p(), nodep, "Already has Dfg vertex"); if (unhandled(nodep)) return; iterate(nodep->fromp()); @@ -875,19 +836,19 @@ class AstToDfgVisitor final : public VNVisitor { DfgVertex* vtxp = nullptr; if (AstConst* const constp = VN_CAST(nodep->lsbp(), Const)) { DfgSel* const selp = new DfgSel{*m_dfgp, flp, DfgVertex::dtypeFor(nodep)}; - selp->fromp(nodep->fromp()->user1u().to()); + selp->fromp(nodep->fromp()->user2u().to()); selp->lsb(constp->toUInt()); vtxp = selp; } else { iterate(nodep->lsbp()); if (m_foundUnhandled) return; DfgMux* const muxp = new DfgMux{*m_dfgp, flp, DfgVertex::dtypeFor(nodep)}; - muxp->fromp(nodep->fromp()->user1u().to()); - muxp->lsbp(nodep->lsbp()->user1u().to()); + muxp->fromp(nodep->fromp()->user2u().to()); + muxp->lsbp(nodep->lsbp()->user2u().to()); vtxp = muxp; } m_uncommittedVertices.push_back(vtxp); - nodep->user1p(vtxp); + nodep->user2p(vtxp); } // The rest of the 'visit' methods are generated by 'astgen' diff --git a/src/V3DfgDecomposition.cpp b/src/V3DfgDecomposition.cpp index 38f31288e..8a1e96095 100644 --- a/src/V3DfgDecomposition.cpp +++ b/src/V3DfgDecomposition.cpp @@ -330,13 +330,10 @@ class ExtractCyclicComponents final { } } UASSERT_OBJ(clonep, &vtx, "Unhandled 'DfgVertexVar' sub-type"); - if (vtx.hasModRefs()) clonep->setHasModRefs(); - if (vtx.hasExtRefs()) clonep->setHasExtRefs(); VertexState& cloneStatep = allocState(*clonep); cloneStatep.component = component; - // We need to mark both the original and the clone as having references in other DFGs + // Mark variable as having references in other DFGs vtx.setHasDfgRefs(); - clonep->setHasDfgRefs(); } return *clonep; } diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index 7b1cab73a..6a557e088 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -129,8 +129,8 @@ template class DfgToAstVisitor final : DfgVisitor { // NODE STATE - // AstScope::user1p // The combinational AstActive under this scope - const VNUser1InUse m_user1InUse; + // AstScope::user2p // The combinational AstActive under this scope + const VNUser2InUse m_user2InUse; // TYPES using VariableType = std::conditional_t; @@ -152,28 +152,28 @@ class DfgToAstVisitor final : DfgVisitor { } static AstActive* getCombActive(AstScope* scopep) { - if (!scopep->user1p()) { + if (!scopep->user2p()) { // Try to find the existing combinational AstActive for (AstNode* nodep = scopep->blocksp(); nodep; nodep = nodep->nextp()) { AstActive* const activep = VN_CAST(nodep, Active); if (!activep) continue; if (activep->hasCombo()) { - scopep->user1p(activep); + scopep->user2p(activep); break; } } // If there isn't one, create a new one - if (!scopep->user1p()) { + if (!scopep->user2p()) { FileLine* const flp = scopep->fileline(); AstSenTree* const senTreep = new AstSenTree{flp, new AstSenItem{flp, AstSenItem::Combo{}}}; AstActive* const activep = new AstActive{flp, "", senTreep}; activep->sensesStorep(senTreep); scopep->addBlocksp(activep); - scopep->user1p(activep); + scopep->user2p(activep); } } - return VN_AS(scopep->user1p(), Active); + return VN_AS(scopep->user2p(), Active); } AstNodeExpr* convertDfgVertexToAstNodeExpr(DfgVertex* vtxp) { diff --git a/src/V3DfgOptimizer.cpp b/src/V3DfgOptimizer.cpp index 2db9a8265..dcf2a76ab 100644 --- a/src/V3DfgOptimizer.cpp +++ b/src/V3DfgOptimizer.cpp @@ -296,16 +296,15 @@ void V3DfgOptimizer::optimize(AstNetlist* netlistp, const string& label) { UINFO(2, __FUNCTION__ << ":"); // NODE STATE - // AstVar::user1 -> Used by: - // - V3DfgPasses::astToDfg, - // - V3DfgPasses::eliminateVars, - // - V3DfgPasses::dfgToAst, - // AstVar::user2 -> bool: Flag indicating referenced by AstVarXRef (set just below) - // AstVar::user3, AstVarScope::user3 - // -> bool: Flag indicating written by logic not representable as DFG - // (set by V3DfgPasses::astToDfg) - const VNUser2InUse user2InUse; - const VNUser3InUse user3InUse; + // AstVar::user1, AstVarScope::user1 -> int, used as a bit-field + // - bit0: Read via AstVarXRef (hierarchical reference) + // - bit1: Written via AstVarXRef (hierarchical reference) + // - bit2: Read by logic in same module/netlist not represented in DFG + // - bit3: Written by logic in same module/netlist not represented in DFG + // - bit4: Referenced by other DFG in same module/netlist + // + // AstNode::user2/user3/user4 can be used by various DFG algorithms + const VNUser1InUse user1InUse; V3DfgContext ctx{label}; @@ -313,7 +312,11 @@ void V3DfgOptimizer::optimize(AstNetlist* netlistp, const string& label) { // Pre V3Scope application. Run on each module separately. // Mark cross-referenced variables - netlistp->foreach([](const AstVarXRef* xrefp) { xrefp->varp()->user2(true); }); + netlistp->foreach([](const AstVarXRef* xrefp) { + AstVar* const tgtp = xrefp->varp(); + if (xrefp->access().isReadOrRW()) DfgVertexVar::setHasRdXRefs(tgtp); + if (xrefp->access().isWriteOrRW()) DfgVertexVar::setHasWrXRefs(tgtp); + }); // Run the optimization phase for (AstNode* nodep = netlistp->modulesp(); nodep; nodep = nodep->nextp()) { diff --git a/src/V3DfgPasses.cpp b/src/V3DfgPasses.cpp index 9a9058090..664ba9420 100644 --- a/src/V3DfgPasses.cpp +++ b/src/V3DfgPasses.cpp @@ -294,7 +294,7 @@ void V3DfgPasses::binToOneHot(DfgGraph& dfg, V3DfgBinToOneHotContext& ctx) { varp->varp()->isInternal(true); varp->srcp(srcp); } - varp->setHasModRefs(); + varp->setHasModRdRefs(); return varp; }(); // The previous index variable - we don't need a vertex for this @@ -314,7 +314,7 @@ void V3DfgPasses::binToOneHot(DfgGraph& dfg, V3DfgBinToOneHotContext& ctx) { = dfg.makeNewVar(flp, name, tabDTypep, nullptr)->as(); varp->varp()->isInternal(true); varp->varp()->noReset(true); - varp->setHasModRefs(); + varp->setHasModWrRefs(); return varp; }(); @@ -417,9 +417,9 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) { // List of variables (AstVar or AstVarScope) we are replacing std::vector replacedVariables; - // AstVar::user1p() : AstVar* -> The replacement variables - // AstVarScope::user1p() : AstVarScope* -> The replacement variables - const VNUser1InUse user1InUse; + // AstVar::user2p() : AstVar* -> The replacement variables + // AstVarScope::user2p() : AstVarScope* -> The replacement variables + const VNUser2InUse user2InUse; // Process the work list while (workListp != sentinelp) { @@ -448,10 +448,11 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) { // Can't remove if it has external drivers if (!varp->isDrivenFullyByDfg()) continue; - // Can't remove if must be kept (including external, non module references) - if (varp->keep()) continue; - - // Can't remove if referenced in other DFGs of the same module (otherwise might rm twice) + // Can't remove if referenced external to the module/netlist + if (varp->hasExtRefs()) continue; + // Can't remove if written in the module + if (varp->hasModWrRefs()) continue; + // Can't remove if referenced in other DFGs of the same module if (varp->hasDfgRefs()) continue; // If it has multiple sinks, it can't be eliminated @@ -469,9 +470,9 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) { UASSERT_OBJ(!varp->hasSinks(), varp, "Variable inlining should make this impossible"); // Grab the AstVar/AstVarScope AstNode* const nodep = varp->nodep(); - UASSERT_OBJ(!nodep->user1p(), nodep, "Replacement already exists"); + UASSERT_OBJ(!nodep->user2p(), nodep, "Replacement already exists"); replacedVariables.emplace_back(nodep); - nodep->user1p(driverp->nodep()); + nodep->user2p(driverp->nodep()); } else { // Otherwise this *is* the canonical var continue; @@ -490,13 +491,13 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) { if (AstModule* const modp = dfg.modulep()) { modp->foreach([&](AstVarRef* refp) { AstVar* varp = refp->varp(); - while (AstVar* const replacep = VN_AS(varp->user1p(), Var)) varp = replacep; + while (AstVar* const replacep = VN_AS(varp->user2p(), Var)) varp = replacep; refp->varp(varp); }); } else { v3Global.rootp()->foreach([&](AstVarRef* refp) { AstVarScope* vscp = refp->varScopep(); - while (AstVarScope* const replacep = VN_AS(vscp->user1p(), VarScope)) vscp = replacep; + while (AstVarScope* const replacep = VN_AS(vscp->user2p(), VarScope)) vscp = replacep; refp->varScopep(vscp); refp->varp(vscp->varp()); }); diff --git a/src/V3DfgVertices.h b/src/V3DfgVertices.h index 11a04e5a1..6ecc63e6a 100644 --- a/src/V3DfgVertices.h +++ b/src/V3DfgVertices.h @@ -41,9 +41,6 @@ class DfgVertexVar VL_NOT_FINAL : public DfgVertexUnary { AstVar* const m_varp; // The AstVar associated with this vertex (not owned by this vertex) AstVarScope* const m_varScopep; // The AstVarScope associated with this vertex (not owned) - bool m_hasDfgRefs = false; // This AstVar is referenced in a different DFG of the module - bool m_hasModRefs = false; // This AstVar is referenced outside the DFG, but in the module - bool m_hasExtRefs = false; // This AstVar is referenced from outside the module // Location of driver of this variable. Only used for converting back to Ast. Might be nullptr. FileLine* m_driverFileLine = nullptr; @@ -62,13 +59,6 @@ public: AstNode* nodep() const { return m_varScopep ? static_cast(m_varScopep) : static_cast(m_varp); } - bool hasDfgRefs() const { return m_hasDfgRefs; } - void setHasDfgRefs() { m_hasDfgRefs = true; } - bool hasModRefs() const { return m_hasModRefs; } - void setHasModRefs() { m_hasModRefs = true; } - bool hasExtRefs() const { return m_hasExtRefs; } - void setHasExtRefs() { m_hasExtRefs = true; } - bool hasNonLocalRefs() const { return hasDfgRefs() || hasModRefs() || hasExtRefs(); } FileLine* driverFileLine() const { return m_driverFileLine; } void driverFileLine(FileLine* flp) { m_driverFileLine = flp; } @@ -77,17 +67,32 @@ public: return srcp() && !srcp()->is() && !varp()->isForced(); } - // Variable cannot be removed, even if redundant in the DfgGraph (might be used externally) - bool keep() const { - // Keep if referenced outside this module - if (hasExtRefs()) return true; - // Keep if traced - if (v3Global.opt.trace() && varp()->isTrace()) return true; - // Keep if public - if (varp()->isSigPublic()) return true; - // Keep if written in non-DFG code - if (nodep()->user3()) return true; - // Otherwise it can be removed + // Variable referenced via an AstVarXRef (hierarchical reference) + bool hasXRefs() const { return nodep()->user1() & 0x03; } + static void setHasRdXRefs(AstNode* nodep) { nodep->user1(nodep->user1() | 0x01); } + static void setHasWrXRefs(AstNode* nodep) { nodep->user1(nodep->user1() | 0x02); } + + // Variable referenced from Ast code in the same module/netlist + bool hasModRdRefs() const { return nodep()->user1() & 0x04; } + bool hasModWrRefs() const { return nodep()->user1() & 0x08; } + bool hasModRefs() const { return nodep()->user1() & 0x0c; } + static void setHasModRdRefs(AstNode* nodep) { nodep->user1(nodep->user1() | 0x04); } + static void setHasModWrRefs(AstNode* nodep) { nodep->user1(nodep->user1() | 0x08); } + void setHasModRdRefs() const { setHasModRdRefs(nodep()); } + void setHasModWrRefs() const { setHasModWrRefs(nodep()); } + + // Variable referenced from other DFG in the same module/netlist + bool hasDfgRefs() const { return nodep()->user1() & 0x10; } + void setHasDfgRefs() { nodep()->user1(nodep()->user1() | 0x10); } + + // Variable referenced outside the containing module/netlist. + bool hasExtRefs() const { + if (m_varp->isIO()) return true; // Ports + if (m_varp->isTrace()) return true; // Traced + if (m_varp->isForced()) return true; // Forced + if (hasXRefs()) return true; // Target of a hierarchical reference + if (m_varp->isPrimaryIO()) return true; // Top level ports + if (m_varp->isSigPublic()) return true; // Public return false; } }; diff --git a/src/astgen b/src/astgen index 03493426a..a6fd71e1c 100755 --- a/src/astgen +++ b/src/astgen @@ -1250,13 +1250,13 @@ def write_dfg_ast_to_dfg(filename): continue fh.write("void visit(Ast{t}* nodep) override {{\n".format(t=node.name)) - fh.write(' UASSERT_OBJ(!nodep->user1p(), nodep, "Already has Dfg vertex");\n\n') + fh.write(' UASSERT_OBJ(!nodep->user2p(), nodep, "Already has Dfg vertex");\n\n') fh.write(" if (unhandled(nodep)) return;\n\n") for i in range(node.arity): fh.write(" iterate(nodep->op{j}p());\n".format(j=i + 1)) fh.write(" if (m_foundUnhandled) return;\n") fh.write( - ' UASSERT_OBJ(nodep->op{j}p()->user1p(), nodep, "Child {j} missing Dfg vertex");\n' + ' UASSERT_OBJ(nodep->op{j}p()->user2p(), nodep, "Child {j} missing Dfg vertex");\n' .format(j=i + 1)) fh.write("\n") fh.write(" Dfg{t}* const vtxp = makeVertex(nodep, *m_dfgp);\n".format( @@ -1268,11 +1268,11 @@ def write_dfg_ast_to_dfg(filename): fh.write(" }\n\n") for i in range(node.arity): fh.write( - " vtxp->relinkSource<{i}>(nodep->op{j}p()->user1u().to());\n". + " vtxp->relinkSource<{i}>(nodep->op{j}p()->user2u().to());\n". format(i=i, j=i + 1)) fh.write("\n") fh.write(" m_uncommittedVertices.push_back(vtxp);\n") - fh.write(" nodep->user1p(vtxp);\n") + fh.write(" nodep->user2p(vtxp);\n") fh.write("}\n")