diff --git a/docs/guide/exe_verilator.rst b/docs/guide/exe_verilator.rst index 807216565..95a0c8636 100644 --- a/docs/guide/exe_verilator.rst +++ b/docs/guide/exe_verilator.rst @@ -715,6 +715,10 @@ Summary: .. option:: -fno-dfg-break-cycles + Deprecated and has no effect (ignored). + + In versions before 5.052: + Rarely needed. Disable breaking combinational cycles during DFG. .. option:: -fno-dfg-peephole diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index b729e841a..618aacf88 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -179,6 +179,40 @@ static void dumpDotVertex(std::ostream& os, const DfgVertex& vtx) { return; } + if (const DfgPrev* const prevVtxp = vtx.cast()) { + const AstVarScope* const vscp = prevVtxp->vscp(); + os << toDotId(vtx); + // Begin attributes + os << " ["; + // Begin 'label' + os << "label=\""; + // Name + os << vscp->prettyName(); + // Address + os << '\n' << cvtToHex(prevVtxp); + // Type and fanout + os << '\n'; + prevVtxp->dtype().astDtypep()->dumpSmall(os); + os << " / F" << prevVtxp->fanout(); + // End 'label' + os << '"'; + // Shape + if (prevVtxp->isPacked()) { + os << ", shape=box"; + } else if (prevVtxp->isArray()) { + os << ", shape=box3d"; + } else { + prevVtxp->v3fatalSrc("Unhandled variable type"); + } + // Color + const char* const colorp = "mediumorchid1"; // Purple + os << ", style=filled"; + os << ", fillcolor=\"" << colorp << "\""; + // End attributes + os << "]\n"; + return; + } + if (const DfgConst* const constVtxp = vtx.cast()) { const V3Number& num = constVtxp->num(); @@ -479,6 +513,10 @@ void DfgVertex::typeCheck(const DfgGraph& dfg) const { CHECK(!v.srcp() || v.srcp()->dtype() == v.dtype(), "'srcp' should match"); return; } + case VDfgType::Prev: { + CHECK(isPacked() || isArray(), "Should be Packed or Array type"); + return; + } case VDfgType::SpliceArray: case VDfgType::SplicePacked: { const DfgVertexSplice& v = *as(); diff --git a/src/V3Dfg.h b/src/V3Dfg.h index c66d2d1a6..21667922d 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -816,6 +816,7 @@ bool DfgVertex::isCheaperThanLoad() const { if (is()) return true; // Variables if (is()) return true; + if (is()) return true; // Array sels are just address computation, but the address itself can be expensive if (const DfgArraySel* aselp = cast()) { if (aselp->bitp()->is()) return false; diff --git a/src/V3DfgBreakCycles.cpp b/src/V3DfgBreakCycles.cpp index 9839beab7..b68b0c86f 100644 --- a/src/V3DfgBreakCycles.cpp +++ b/src/V3DfgBreakCycles.cpp @@ -108,6 +108,18 @@ public: }); } + // Update after replacing 'vtxp' with 'replacement' + void updateReplacement(const DfgVertex& vtx, DfgVertex& replacement) { + updateAcyclic(vtx); + if (!get(replacement)) { + replacement.foreachSink([&](DfgVertex& dst) { + if (!get(dst)) return false; + if (!stillCyclicFwd(dst)) updateAcyclic(dst); + return false; + }); + } + } + /* Check stored information is consistent with actual SCCs. Note we can't detect during updates if an SCC has split into multiple SCCs. @@ -1518,17 +1530,8 @@ class FixUp final { UASSERT_OBJ(!vtx.hasSinks() == !m_sccInfo.get(*replacementp), &vtx, "Replacement vertex SCC inconsistent"); - // If we broke the cycle through this vertex we can update the SccInfo - if (!vtx.hasSinks()) { - m_sccInfo.updateAcyclic(vtx); - if (!m_sccInfo.get(*replacementp)) { - replacementp->foreachSink([&](DfgVertex& dst) { - if (!m_sccInfo.get(dst)) return false; - if (!m_sccInfo.stillCyclicFwd(dst)) m_sccInfo.updateAcyclic(dst); - return false; - }); - } - } + // If we broke the cycle through this vertex, update the SccInfo + if (!vtx.hasSinks()) m_sccInfo.updateReplacement(vtx, *replacementp); } void main(DfgVertexVar& var) { @@ -1583,7 +1586,7 @@ public: } }; -bool breakCycles(DfgGraph& dfg, V3DfgBreakCyclesContext& ctx) { +void breakCycles(DfgGraph& dfg, V3DfgBreakCyclesContext& ctx) { // Shorthand for dumping graph at given dump level const auto dump = [&](int level, const DfgGraph& dfg, const std::string& name) { if (dumpDfgLevel() >= level) dfg.dumpDotFilePrefixed("breakCycles-" + name); @@ -1622,7 +1625,7 @@ bool breakCycles(DfgGraph& dfg, V3DfgBreakCyclesContext& ctx) { UINFO(7, "Graph became acyclic after " << nImprovements << " improvements"); dump(7, dfg, "result-acyclic"); ++ctx.m_nFixed; - return true; + return; } } while (nImprovements != prevNImprovements); @@ -1641,17 +1644,62 @@ bool breakCycles(DfgGraph& dfg, V3DfgBreakCyclesContext& ctx) { ++ctx.m_nImproved; } else { UINFO(7, "Graph NOT improved"); - dump(7, dfg, "result-original"); ++ctx.m_nUnchanged; } - return false; + + // Break any remaining cycles by inserting a DfgPrev for variables still in a cycle. + // Doing this unconditionally is both safe and sufficient: + // - Safe: a DfgPrev makes the variable's readers read its stored value instead + // of its in-graph driver, while the variable itself is still assigned. This + // merely reconstructs the cyclic variable-level dataflow, which the scheduler + // will resolve via its settle loop, so the computation is equivalent. + // - Sufficient: every cycle must pass through a variable, and in particular + // through a non-temporary one (synthesis doesn't create cycles within a single + // logic block). + // Also note that synthesized logic that depends on an in-graph update will read + // the synthesis temporary, not the real variable (which is what we are cutting), + // so the graph still holds all updates in a form visible to later optimizers. + + // Gather all non-temporary variables as candidate cut points, prefer those that + // are kept regardless. Breaking a cycle there is essentially free, as they must be + // materialized anyway, whereas cutting an internal variable forces it to survive + // via its DfgPrev when it could otherwise be inlined away later. + std::vector varps; + for (DfgVertexVar& vtx : dfg.varVertices()) { + if (!vtx.tmpForp()) varps.push_back(&vtx); + } + std::stable_partition(varps.begin(), varps.end(), [](const DfgVertexVar* varp) { + return varp->hasExtRefs() || varp->isVolatile(); + }); + + // Insert DfgPrev vertices until the graph becomes acyclic + SccInfo sccInfo{dfg}; + for (DfgVertexVar* const varp : varps) { + // Stop as soon as it becomes acyclic + if (!sccInfo.isCyclic()) break; + + // Ignore if variable is not part of a cycle + if (!sccInfo.get(*varp)) continue; + + // Insert a DfgPrev vertex for the variable + DfgPrev* const prevp = new DfgPrev{dfg, varp->vscp()}; + sccInfo.add(*prevp, 0); + varp->replaceWith(prevp); + ++ctx.m_nPrevInserted; + + // Update SCC info + sccInfo.updateReplacement(*varp, *prevp); + } + // Validate SccInfo if in debug mode + if (v3Global.opt.debugCheck()) sccInfo.validate(); + UASSERT(!sccInfo.isCyclic(), "Graph should become acyclic"); + + dump(7, dfg, "result-withprev"); } } //namespace V3DfgBreakCycles - -bool V3DfgPasses::breakCycles(DfgGraph& dfg, V3DfgContext& ctx) { - const bool res = V3DfgBreakCycles::breakCycles(dfg, ctx.m_breakCyclesContext); +void V3DfgPasses::breakCycles(DfgGraph& dfg, V3DfgContext& ctx) { + V3DfgBreakCycles::breakCycles(dfg, ctx.m_breakCyclesContext); if (v3Global.opt.debugCheck()) V3DfgPasses::typeCheck(dfg); V3DfgPasses::removeUnused(dfg); - return res; } diff --git a/src/V3DfgContext.h b/src/V3DfgContext.h index ee34df5f6..a3f637166 100644 --- a/src/V3DfgContext.h +++ b/src/V3DfgContext.h @@ -103,6 +103,7 @@ public: VDouble0 m_nImproved; // Number of graphs that were improved but still cyclic VDouble0 m_nUnchanged; // Number of graphs that were left unchanged VDouble0 m_nImprovements; // Number of changes made to graphs + VDouble0 m_nPrevInserted; // Number of Prev vertices inserted private: V3DfgBreakCyclesContext() @@ -112,6 +113,7 @@ private: addStat("improved", m_nImproved); addStat("left unchanged", m_nUnchanged); addStat("changes applied", m_nImprovements); + addStat("prev vertices inserted", m_nPrevInserted); } }; class V3DfgCseContext final : public V3DfgSubContext { diff --git a/src/V3DfgCse.cpp b/src/V3DfgCse.cpp index c1b3862e4..c9ca7f611 100644 --- a/src/V3DfgCse.cpp +++ b/src/V3DfgCse.cpp @@ -55,6 +55,7 @@ class V3DfgCse final { case VDfgType::CReset: case VDfgType::VarArray: case VDfgType::VarPacked: + case VDfgType::Prev: case VDfgType::AstRd: // LCOV_EXCL_STOP vtx.v3fatalSrc("Hash should have been pre-computed"); @@ -173,6 +174,7 @@ class V3DfgCse final { // Special vertices case VDfgType::Const: return a.as()->num().isCaseEq(b.as()->num()); case VDfgType::CReset: return false; + case VDfgType::Prev: return false; case VDfgType::VarArray: case VDfgType::VarPacked: // CSE does not combine variables @@ -307,9 +309,10 @@ class V3DfgCse final { 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}; - // Pre-hash CReset vertices, these are all unique + // Pre-hash CReset and Prev vertices, these are all unique for (const DfgVertex& vtx : dfg.opVertices()) { if (vtx.is()) m_hashCache[vtx] = V3Hash{++varHash}; + if (vtx.is()) m_hashCache[vtx] = V3Hash{++varHash}; } // Similarly pre-hash constants for speed. While we don't combine constants, we do want diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index b2af96461..90dda7cdf 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -224,6 +224,10 @@ class DfgToAstVisitor final : DfgVisitor { m_resultp = new AstVarRef{vtxp->fileline(), vtxp->vscp(), VAccess::READ}; } + void visit(DfgPrev* vtxp) override { + m_resultp = new AstVarRef{vtxp->fileline(), vtxp->vscp(), VAccess::READ}; + } + void visit(DfgConst* vtxp) override { // m_resultp = new AstConst{vtxp->fileline(), vtxp->num()}; } diff --git a/src/V3DfgOptimizer.cpp b/src/V3DfgOptimizer.cpp index f18672598..7abfd6788 100644 --- a/src/V3DfgOptimizer.cpp +++ b/src/V3DfgOptimizer.cpp @@ -39,7 +39,8 @@ class DataflowOptimize final { // - 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: Has READWRITE references - // - bit31-5: Reference count, how many DfgVertexVar represent this variable + // - bit5: Has DfgPrev instance + // - bit31-6: Reference count, how many DfgVertexVar represent this variable // // AstNode::user2/user3/user4 can be used by various DFG algorithms const VNUser1InUse m_user1InUse; @@ -116,65 +117,44 @@ class DataflowOptimize final { V3DfgPasses::synthesize(dfg, m_ctx); endOfStage("synthesize", dfg, {}); - // Extract the cyclic sub-graphs. We do this because a lot of the optimizations assume a - // DAG, and large, mostly acyclic graphs could not be optimized due to the presence of - // small cycles. - std::vector> cyclicComps = dfg.extractCyclicComponents("cyclic"); - endOfStage("extractCyclic", dfg, cyclicComps); - - // Attempt to convert cyclic components into acyclic ones - std::vector> madeAcyclicComponents; - if (v3Global.opt.fDfgBreakCycles()) { - for (auto it = cyclicComps.begin(); it != cyclicComps.end();) { - const bool madeAcyclic = V3DfgPasses::breakCycles(**it, m_ctx); - // If not made acyclic, keep it in 'cyclicComps' - if (!madeAcyclic) { - ++it; - continue; - } - // Otherwise move to 'madeAcyclicComponents' - madeAcyclicComponents.emplace_back(std::move(*it)); - it = cyclicComps.erase(it); - } + // Extract the cyclic sub-graphs, so breakCycles can operate on small graphs, + // make all of them acyclic, then merge them back to the main graph + { + std::vector> comps = dfg.extractCyclicComponents("cyclic"); + for (const auto& cp : comps) V3DfgPasses::breakCycles(*cp, m_ctx); + dfg.mergeGraphs(std::move(comps)); + endOfStage("breakCycles", dfg, {}); } - // Merge those that were made acyclic back to the graph, this enables optimizing more - dfg.mergeGraphs(std::move(madeAcyclicComponents)); - endOfStage("breakCycles", dfg, cyclicComps); - // Remove redundant selects - V3DfgPasses::removeSelects(dfg, m_ctx.m_removeSelectsContext); - for (std::unique_ptr& compp : cyclicComps) { - V3DfgPasses::removeSelects(*compp, m_ctx.m_removeSelectsContext); - } - endOfStage("removeSelects", dfg, cyclicComps); - - // Split the acyclic DFG into [weakly] connected components - std::vector> acyclicComps = dfg.splitIntoComponents("acyclic"); + // Split the now entirely acyclic DFG into [weakly] connected components + std::vector> comps = dfg.splitIntoComponents("acyclic"); UASSERT(dfg.size() == 0, "DfgGraph should have become empty"); - endOfStage("splitAcyclic", dfg, acyclicComps); + endOfStage("splitAcyclic", dfg, comps); - // Optimize each acyclic component - for (auto& cp : acyclicComps) V3DfgPasses::inlineVars(*cp); - endOfStage("inlineVars", dfg, acyclicComps); - for (auto& cp : acyclicComps) V3DfgPasses::cse(*cp, m_ctx.m_cseContext0); - endOfStage("cse0", dfg, acyclicComps); - for (auto& cp : acyclicComps) V3DfgPasses::binToOneHot(*cp, m_ctx.m_binToOneHotContext); - endOfStage("binToOneHot", dfg, acyclicComps); - for (auto& cp : acyclicComps) V3DfgPasses::peephole(*cp, m_ctx.m_peepholeContext); - endOfStage("peephole", dfg, acyclicComps); - // Accumulate patterns for reporting - if (v3Global.opt.dumpDfgPatterns()) { - V3DfgPasses::dumpPatterns(acyclicComps); - endOfStage("dumpPatterns"); + // Main pass pipeline - optimize each acyclic component + { + for (auto& cp : comps) V3DfgPasses::removeSelects(*cp, m_ctx.m_removeSelectsContext); + endOfStage("removeSelects", dfg, comps); + for (const auto& cp : comps) V3DfgPasses::inlineVars(*cp); + endOfStage("inlineVars", dfg, comps); + for (auto& cp : comps) V3DfgPasses::cse(*cp, m_ctx.m_cseContext0); + endOfStage("cse0", dfg, comps); + for (auto& cp : comps) V3DfgPasses::binToOneHot(*cp, m_ctx.m_binToOneHotContext); + endOfStage("binToOneHot", dfg, comps); + for (auto& cp : comps) V3DfgPasses::peephole(*cp, m_ctx.m_peepholeContext); + endOfStage("peephole", dfg, comps); + + // Accumulate patterns for reporting + if (v3Global.opt.dumpDfgPatterns()) V3DfgPasses::dumpPatterns(comps); + + for (auto& cp : comps) V3DfgPasses::pushDownSels(*cp, m_ctx.m_pushDownSelsContext); + endOfStage("pushDownSels", dfg, comps); + for (auto& cp : comps) V3DfgPasses::cse(*cp, m_ctx.m_cseContext1); + endOfStage("cse1", dfg, comps); } - for (auto& cp : acyclicComps) V3DfgPasses::pushDownSels(*cp, m_ctx.m_pushDownSelsContext); - endOfStage("pushDownSels", dfg, acyclicComps); - for (auto& cp : acyclicComps) V3DfgPasses::cse(*cp, m_ctx.m_cseContext1); - endOfStage("cse1", dfg, acyclicComps); - // Merge everything back under the main DFG - dfg.mergeGraphs(std::move(acyclicComps)); - dfg.mergeGraphs(std::move(cyclicComps)); + // Merge everything back under the main graph + dfg.mergeGraphs(std::move(comps)); endOfStage("optimized", dfg, {}); // Regularize the graph after merging it all back together so all diff --git a/src/V3DfgPasses.h b/src/V3DfgPasses.h index 3392f0703..798a3df90 100644 --- a/src/V3DfgPasses.h +++ b/src/V3DfgPasses.h @@ -43,10 +43,10 @@ void removeUnobservable(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; void synthesize(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; // Remove redundant selects void removeSelects(DfgGraph& dfg, V3DfgRemoveSelectsContext& ctx) VL_MT_DISABLED; -// Attempt to make the given cyclic graph into an acyclic, or "less cyclic" -// equivalent. Genuine combinational cycles can exist, so this might be -// unsuccessful. Returns true if the graph became acyclic, false otherwise. -bool breakCycles(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; +// Make the given cyclic graph into an acyclic one, by tracing drivers, and +// if that is unsuccessful, by inserting Prev vertices. The given graph will +// always become acyclic after this pass. +void breakCycles(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; // Construct binary to oneHot decoders void binToOneHot(DfgGraph&, V3DfgBinToOneHotContext&) VL_MT_DISABLED; // Common subexpression elimination diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index fa29339fe..fbf244550 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -266,6 +266,8 @@ class V3DfgPeephole final : public DfgVisitor { void deleteVertex(DfgVertex* vtxp) { UASSERT_OBJ(!m_vInfo[vtxp].m_workListIndex, vtxp, "Deleted Vertex is in work list"); UASSERT_OBJ(!vtxp->hasSinks(), vtxp, "Should not delete used vertex"); + const DfgVertexVar* const varp = vtxp->cast(); + UASSERT_OBJ(!varp || !varp->hasPrev(), vtxp, "Deleting variable consumed via DfgPrev"); // Invalidate cache entry m_cache.invalidate(vtxp); @@ -293,7 +295,6 @@ class V3DfgPeephole final : public DfgVisitor { // This pass only removes variables that are either not driven in this graph, // or are not observable outside the graph. If there is also no external write // to the variable and no references in other graph then delete the Ast var too. - const DfgVertexVar* const varp = vtxp->cast(); if (varp && !varp->isVolatile() && !varp->hasDfgRefs()) { m_ctx.m_deleteps.push_back(varp->vscp()); VL_DO_DANGLING(vtxp->unlinkDelete(m_dfg), vtxp); diff --git a/src/V3DfgRegularize.cpp b/src/V3DfgRegularize.cpp index f8e7b7935..a657159a3 100644 --- a/src/V3DfgRegularize.cpp +++ b/src/V3DfgRegularize.cpp @@ -56,16 +56,6 @@ class DfgRegularize final { } } - std::unordered_set gatherCyclicVariables() { - DfgUserMap vtx2Scc = m_dfg.makeUserMap(); - V3DfgPasses::colorStronglyConnectedComponents(m_dfg, vtx2Scc); - std::unordered_set circularVariables; - for (const DfgVertexVar& vtx : m_dfg.varVertices()) { - if (vtx2Scc[vtx]) circularVariables.emplace(&vtx); - } - return circularVariables; - } - static bool isUnused(const DfgVertex& vtx) { if (vtx.hasSinks()) return false; if (const DfgVertexVar* const varp = vtx.cast()) { @@ -73,6 +63,7 @@ class DfgRegularize final { UASSERT_OBJ(!varp->hasDfgRefs(), varp, "Should not have refs in other DfgGraph"); if (varp->hasModWrRefs()) return false; if (varp->hasExtRefs()) return false; + if (varp->hasPrev()) return false; } return true; } @@ -87,6 +78,9 @@ class DfgRegularize final { &aVtx, "Mismatched vertices"); UASSERT_OBJ(!aVtx.is(), &aVtx, "Should be an operation vertex"); + // Prev is just a variable reference + if (aVtx.is()) return false; + if (bVtx.hasMultipleSinks()) { // Add a temporary if it's cheaper to store and load from memory than recompute if (!aVtx.isCheaperThanLoad()) return true; @@ -119,10 +113,6 @@ class DfgRegularize final { } void eliminateVars() { - // Although we could eliminate some circular variables, doing so would - // make UNOPTFLAT traces fairly usesless, so we will not do so. - const std::unordered_set circularVariables = gatherCyclicVariables(); - // Worklist based algoritm DfgWorklist workList{m_dfg}; @@ -141,7 +131,7 @@ class DfgRegularize final { }); // Delete corresponsing Ast variable at the end if (const DfgVertexVar* const varp = vtx.cast()) { - m_ctx.m_deleteps.push_back(varp->vscp()); + if (!varp->hasPrev()) m_ctx.m_deleteps.push_back(varp->vscp()); } // Remove the unused vertex vtx.unlinkDelete(m_dfg); @@ -173,7 +163,7 @@ class DfgRegularize final { UASSERT_OBJ(!varp->hasDfgRefs(), varp, "Should not have refs in other DfgGraph"); // Do not eliminate circular variables - need to preserve UNOPTFLAT traces - if (circularVariables.count(varp)) return; + if (varp->hasPrev()) 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) diff --git a/src/V3DfgVertices.h b/src/V3DfgVertices.h index dd51a58a9..960d4aedd 100644 --- a/src/V3DfgVertices.h +++ b/src/V3DfgVertices.h @@ -62,8 +62,8 @@ protected: *DfgDataType::fromAst(vscp->varp()->dtypep())} , m_vscp{vscp} { // Increment reference count - m_vscp->user1(m_vscp->user1() + 0x20); - UASSERT_OBJ((m_vscp->user1() >> 5) > 0, m_vscp, "Reference count overflow"); + m_vscp->user1(m_vscp->user1() + 0x40); + UASSERT_OBJ((m_vscp->user1() >> 6) > 0, m_vscp, "Reference count overflow"); // Allocate sources newInput(); newInput(); @@ -72,8 +72,8 @@ protected: public: ~DfgVertexVar() { // Decrement reference count - m_vscp->user1(m_vscp->user1() - 0x20); - UASSERT_OBJ((m_vscp->user1() >> 5) >= 0, m_vscp, "Reference count underflow"); + m_vscp->user1(m_vscp->user1() - 0x40); + UASSERT_OBJ((m_vscp->user1() >> 6) >= 0, m_vscp, "Reference count underflow"); } ASTGEN_MEMBERS_DfgVertexVar; @@ -87,7 +87,6 @@ public: std::string srcName(size_t idx) const override final { return idx ? "defaultp" : "srcp"; } // The Ast variable this vertex representess - // AstVar* varp() const { return m_varp; } AstVarScope* vscp() const { return m_vscp; } // If this is a temporary, the Ast variable it stands for, or same as @@ -100,7 +99,7 @@ public: void driverFileLine(FileLine* flp) { m_driverFileLine = flp; } // Variable referenced from other DFG in the same module/netlist - bool hasDfgRefs() const { return m_vscp->user1() >> 6; } // I.e.: (nodep()->user1() >> 5) > 1 + bool hasDfgRefs() const { return m_vscp->user1() >> 7; } // I.e.: (nodep()->user1() >> 6) > 1 // Variable referenced from Ast code in the same module/netlist static bool hasModWrRefs(const AstVarScope* nodep) { return nodep->user1() & 0x08; } @@ -121,8 +120,15 @@ public: static bool hasRWRefs(const AstVarScope* nodep) { return nodep->user1() & 0x10; } static void setHasRWRefs(AstVarScope* nodep) { nodep->user1(nodep->user1() | 0x10); } - // True iff the value of this variable is read outside this DfgGraph + // There exists a DfgPrev vertex for this variable + static bool hasPrev(const AstVarScope* nodep) { return nodep->user1() & 0x20; } + bool hasPrev() const { return hasPrev(m_vscp); } + + // True iff this variable is consumed without an explicit sink: its value is read outside + // this DfgGraph, or a DfgPrev reads it within this graph. bool isObserved() const { + // A DfgPrev reads this variable + if (hasPrev()) return true; // 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(); @@ -162,6 +168,33 @@ public: ASTGEN_MEMBERS_DfgVarPacked; }; +class DfgPrev final : public DfgVertex { + // Previous value of variable, before any updates made in this graph. + // Used to break combinational cycles. + friend class DfgVertex; + friend class DfgVisitor; + + AstVarScope* const m_vscp; // The AstVarScope associated with this vertex (not owned) + +public: + DfgPrev(DfgGraph& dfg, AstVarScope* vscp) + : DfgVertex{dfg, dfgType(), vscp->varp()->fileline(), + *DfgDataType::fromAst(vscp->varp()->dtypep())} + , m_vscp{vscp} { + UASSERT_OBJ(!DfgVertexVar::hasPrev(vscp), vscp, "Variable already has a DfgPrev"); + m_vscp->user1(m_vscp->user1() | 0x20); // Mark having a DfgPrev + } + ~DfgPrev() { + m_vscp->user1(m_vscp->user1() & ~0x20); // Unmark having a DfgPrev + } + ASTGEN_MEMBERS_DfgPrev; + + // The Ast variable this vertex representess + AstVarScope* vscp() const { return m_vscp; } + + std::string srcName(size_t) const override final { return ""; } +}; + //------------------------------------------------------------------------------ // Ast reference vertices diff --git a/src/V3Options.cpp b/src/V3Options.cpp index f9502e00c..26b1087a4 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -1466,7 +1466,9 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, DECL_OPTION("-fdead-cells", FOnOff, &m_fDeadCells); DECL_OPTION("-fdedup", FOnOff, &m_fDedupe); DECL_OPTION("-fdfg", CbFOnOff, [this](bool flag) { m_fDfg = flag; }); - DECL_OPTION("-fdfg-break-cycles", FOnOff, &m_fDfgBreakCycles); + DECL_OPTION("-fdfg-break-cycles", CbFOnOff, [fl](bool) { + fl->v3warn(DEPRECATED, "Option '-fdfg-break-cycles' is deprecated and has no effect"); + }); DECL_OPTION("-fdfg-peephole", FOnOff, &m_fDfgPeephole); DECL_OPTION("-fdfg-peephole-", CbPartialMatch, [this](const char* optp) { // m_fDfgPeepholeDisabled.erase(optp); diff --git a/src/V3Options.h b/src/V3Options.h index 182379fe9..692e0272b 100644 --- a/src/V3Options.h +++ b/src/V3Options.h @@ -402,7 +402,6 @@ private: bool m_fConstBitOpTree; // main switch: -fno-const-bit-op-tree constant bit op tree bool m_fConstEager = true; // main switch: -fno-const-eagerly run V3Const during passes bool m_fDedupe; // main switch: -fno-dedupe: logic deduplication - bool m_fDfgBreakCycles = true; // main switch: -fno-dfg-break-cycles bool m_fDfgPeephole = true; // main switch: -fno-dfg-peephole bool m_fDfgPushDownSels = true; // main switch: -fno-dfg-push-down-sels bool m_fDfg; // main switch: -fno-dfg @@ -743,7 +742,6 @@ public: bool fConstEager() const { return m_fConstEager; } bool fDedupe() const { return m_fDedupe; } bool fDfg() const { return m_fDfg; } - bool fDfgBreakCycles() const { return m_fDfgBreakCycles; } bool fDfgPeephole() const { return m_fDfgPeephole; } bool fDfgPushDownSels() const { return m_fDfgPushDownSels; } bool fDfgSynthesizeAll() const { return m_fDfgSynthesizeAll; } diff --git a/test_regress/t/t_dfg_break_cycles.py b/test_regress/t/t_dfg_break_cycles.py index 217863f38..194fae92e 100755 --- a/test_regress/t/t_dfg_break_cycles.py +++ b/test_regress/t/t_dfg_break_cycles.py @@ -67,7 +67,7 @@ with open(rdFile, 'r', encoding="utf8") as rdFh, \ test.compile(verilator_flags2=[ "--stats", "--build", - "-fno-dfg" if test.name == "t_dfg_break_cycles" else "-fno-dfg-break-cycles", + "-fno-dfg", "-fno-gate", "+incdir+" + test.obj_dir, "-Mdir", test.obj_dir + "/obj_ref", @@ -76,9 +76,8 @@ test.compile(verilator_flags2=[ ]) # yapf:disable # Check we got the expected number of circular logic warnings -if test.name == "t_dfg_break_cycles": - test.file_grep(test.obj_dir + "/obj_ref/Vref__stats.txt", - r'Warnings, Suppressed UNOPTFLAT\s+(\d+)', nExpectedCycles) +test.file_grep(test.obj_dir + "/obj_ref/Vref__stats.txt", + r'Warnings, Suppressed UNOPTFLAT\s+(\d+)', nExpectedCycles) # Compile optimized - also builds executable test.compile(verilator_flags2=[ diff --git a/test_regress/t/t_dfg_break_cycles_off.py b/test_regress/t/t_dfg_break_cycles_off.py deleted file mode 100755 index bf6af753c..000000000 --- a/test_regress/t/t_dfg_break_cycles_off.py +++ /dev/null @@ -1,16 +0,0 @@ -#!/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: 2025 Wilson Snyder -# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 - -import vltest_bootstrap - -import runpy - -test.scenarios('vlt_all') - -runpy.run_path("t/t_dfg_break_cycles.py", globals()) diff --git a/test_regress/t/t_dfg_peephole.v b/test_regress/t/t_dfg_peephole.v index 3e393a297..06578e6af 100644 --- a/test_regress/t/t_dfg_peephole.v +++ b/test_regress/t/t_dfg_peephole.v @@ -11,7 +11,9 @@ module t ( rand_a, rand_b, srand_a, srand_b, arand_a, arand_b ); +// verilator lint_off UNOPTFLAT `include "portdecl.vh" // Boilerplate generated by t_dfg_peephole.py +// verilator lint_on UNOPTFLAT input rand_a; input rand_b; @@ -429,6 +431,17 @@ module t ( `signal(NARROW_CONCAT_B, narrow_concat[8:4]); `signal(NARROW_CONCAT_C, narrow_concat[5:4]); + // Cycle needing a DfgPrev (variable shift). Peephole must keep the internal + // 'break_lo' driver, as the DfgPrev reads its stored value. Shift forced non-zero + // for a unique fixed point. + // verilator lint_off UNOPTFLAT + wire [63:0] break_lo; + wire [63:0] break_hi; + assign break_hi = (break_lo >> (rand_b[2:0] | 3'b1)) | rand_a; + assign break_lo = break_hi & rand_b; + // verilator lint_on UNOPTFLAT + `signal(DFG_PREV_KEEPS_DRIVER, break_hi); + // Assigned at the end to avoid inlining by other passes assign const_a = 64'h0123456789abcdef; assign const_b = 64'h98badefc10325647; diff --git a/test_regress/t/t_flag_deprecated_bad.out b/test_regress/t/t_flag_deprecated_bad.out index b05ea6277..8529877ec 100644 --- a/test_regress/t/t_flag_deprecated_bad.out +++ b/test_regress/t/t_flag_deprecated_bad.out @@ -8,4 +8,5 @@ %Warning-DEPRECATED: Option '-fno-dfg-pre-inline' is deprecated and has no effect %Warning-DEPRECATED: Option '-fno-dfg-post-inline' is deprecated and has no effect %Warning-DEPRECATED: Option '-fno-dfg-scoped' is deprecated, use '-fno-dfg' instead. +%Warning-DEPRECATED: Option '-fdfg-break-cycles' is deprecated and has no effect %Error: Exiting due to diff --git a/test_regress/t/t_flag_deprecated_bad.py b/test_regress/t/t_flag_deprecated_bad.py index c9370a2f7..233ce25f8 100755 --- a/test_regress/t/t_flag_deprecated_bad.py +++ b/test_regress/t/t_flag_deprecated_bad.py @@ -12,7 +12,7 @@ import vltest_bootstrap test.scenarios('vlt') test.lint(verilator_flags2=[ - "--trace-fst-thread --trace-threads 2 --order-clock-delay --clk foo --no-clk bar -fno-dfg-pre-inline -fno-dfg-post-inline -fno-dfg-scoped" + "--trace-fst-thread --trace-threads 2 --order-clock-delay --clk foo --no-clk bar -fno-dfg-pre-inline -fno-dfg-post-inline -fno-dfg-scoped -fno-dfg-break-cycles" ], fails=True, expect_filename=test.golden_filename)