diff --git a/src/V3Dfg.h b/src/V3Dfg.h index d8770d6ce..8578a7c53 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -496,6 +496,9 @@ public: // Is this a DfgConst that is all ones inline bool isOnes() const; + // Should this vertex be inlined when rendering to Ast, or be stored to a temporary + inline bool inlined() const; + // Methods that allow DfgVertex to participate in error reporting/messaging void v3errorEnd(std::ostringstream& str) const { m_filelinep->v3errorEnd(str); } void v3errorEndFatal(std::ostringstream& str) const VL_ATTR_NORETURN { @@ -914,4 +917,13 @@ bool DfgVertex::isOnes() const { return false; } +bool DfgVertex::inlined() const { + // Inline vertices that drive only a single node, or are special + if (!hasMultipleSinks()) return true; + if (is()) return true; + if (is()) return true; + if (const DfgArraySel* const selp = cast()) return selp->bitp()->is(); + return false; +} + #endif diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index f87c443c8..32fb87b8f 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -221,19 +221,8 @@ class DfgToAstVisitor final : DfgVisitor { return resultp; } - bool inlineVertex(DfgVertex& vtx) { - // Inline vertices that drive only a single node, or are special - if (!vtx.hasMultipleSinks()) return true; - if (vtx.is()) return true; - if (vtx.is()) return true; - if (const DfgArraySel* const selp = vtx.cast()) { - return selp->bitp()->is(); - } - return false; - } - AstNodeExpr* convertSource(DfgVertex* vtxp) { - if (inlineVertex(*vtxp)) { + if (vtxp->inlined()) { // Inlined vertices are simply recursively converted UASSERT_OBJ(vtxp->hasSinks(), vtxp, "Must have one sink: " << vtxp->typeName()); return convertDfgVertexToAstNodeExpr(vtxp); @@ -406,7 +395,7 @@ class DfgToAstVisitor final : DfgVisitor { nextp = vtxp->verticesNext(); // If the vertex is known to be inlined, then there is nothing to do - if (inlineVertex(*vtxp)) continue; + if (vtxp->inlined()) continue; // Check if this uses a temporary, vs one of the vars rendered above AstVar* const resultVarp = getResultVar(vtxp); diff --git a/src/V3DfgPasses.cpp b/src/V3DfgPasses.cpp index 396a18027..26e1b3b6b 100644 --- a/src/V3DfgPasses.cpp +++ b/src/V3DfgPasses.cpp @@ -75,59 +75,66 @@ V3DfgOptimizationContext::~V3DfgOptimizationContext() { "Inconsistent statistics"); } -// Common subexpression elimination +// Common sub-expression elimination void V3DfgPasses::cse(DfgGraph& dfg, V3DfgCseContext& ctx) { - DfgVertex::EqualsCache equalsCache; - std::unordered_map> verticesWithEqualHashes; - verticesWithEqualHashes.reserve(dfg.size()); + // Remove common sub-expressions + { + // Used by DfgVertex::hash + const auto userDataInUse = dfg.userDataInUse(); - // Used by DfgVertex::hash - const auto userDataInUse = dfg.userDataInUse(); + DfgVertex::EqualsCache equalsCache; + std::unordered_map> verticesWithEqualHashes; + verticesWithEqualHashes.reserve(dfg.size()); - // Pre-hash variables for speed, these are all unique, so just set their hash to a unique value - uint32_t varHash = 0; - for (DfgVertexVar *vtxp = dfg.varVerticesBeginp(), *nextp; vtxp; vtxp = nextp) { - nextp = vtxp->verticesNext(); - vtxp->user() = 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 constants - // to equal values. - for (DfgConst *vtxp = dfg.constVerticesBeginp(), *nextp; vtxp; vtxp = nextp) { - nextp = vtxp->verticesNext(); - // Get rid of unused constants while we are at it - if (!vtxp->hasSinks()) { - vtxp->unlinkDelete(dfg); - continue; + // Pre-hash variables, these are all unique, so just set their hash to a unique value + uint32_t varHash = 0; + for (DfgVertexVar *vtxp = dfg.varVerticesBeginp(), *nextp; vtxp; vtxp = nextp) { + nextp = vtxp->verticesNext(); + vtxp->user() = V3Hash{++varHash}; } - vtxp->user() = vtxp->num().toHash(); - } - // Combine operation vertices - for (DfgVertex *vtxp = dfg.opVerticesBeginp(), *nextp; vtxp; vtxp = nextp) { - nextp = vtxp->verticesNext(); - // Get rid of unused operations while we are at it - if (!vtxp->hasSinks()) { - vtxp->unlinkDelete(dfg); - continue; - } - const V3Hash hash = vtxp->hash(); - if (VL_LIKELY(nextp)) VL_PREFETCH_RW(nextp); - std::vector& vec = verticesWithEqualHashes[hash]; - bool replaced = false; - for (DfgVertex* const candidatep : vec) { - if (candidatep->equals(*vtxp, equalsCache)) { - ++ctx.m_eliminated; - vtxp->replaceWith(candidatep); + // 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 + // constants to equal values. + for (DfgConst *vtxp = dfg.constVerticesBeginp(), *nextp; vtxp; vtxp = nextp) { + nextp = vtxp->verticesNext(); + if (VL_LIKELY(nextp)) VL_PREFETCH_RW(nextp); + // Delete unused constants while we are at it. + if (!vtxp->hasSinks()) { vtxp->unlinkDelete(dfg); - replaced = true; - break; + continue; } + vtxp->user() = vtxp->num().toHash() + varHash; + } + + // Combine operation vertices + for (DfgVertex *vtxp = dfg.opVerticesBeginp(), *nextp; vtxp; vtxp = nextp) { + nextp = vtxp->verticesNext(); + if (VL_LIKELY(nextp)) VL_PREFETCH_RW(nextp); + // Delete unused nodes while we are at it. + if (!vtxp->hasSinks()) { + vtxp->unlinkDelete(dfg); + continue; + } + const V3Hash hash = vtxp->hash(); + std::vector& vec = verticesWithEqualHashes[hash]; + bool replaced = false; + for (DfgVertex* const candidatep : vec) { + if (candidatep->equals(*vtxp, equalsCache)) { + ++ctx.m_eliminated; + vtxp->replaceWith(candidatep); + vtxp->unlinkDelete(dfg); + replaced = true; + break; + } + } + if (replaced) continue; + vec.push_back(vtxp); } - if (replaced) continue; - vec.push_back(vtxp); } + + // Prune unused nodes + removeUnused(dfg); } void V3DfgPasses::inlineVars(DfgGraph& dfg) { @@ -167,41 +174,46 @@ void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) { // Can't remove if it has consumers if (varp->hasSinks()) continue; - // Can't remove if read in the module and driven here (i.e.: it's an output of the DFG) - if (varp->hasModRefs() && varp->isDrivenByDfg()) continue; - - // Can't remove if only partially driven by the DFG - if (varp->isDrivenByDfg() && !varp->isDrivenFullyByDfg()) continue; - - // Can't remove if referenced externally, or other special reasons - if (varp->keep()) continue; - - // If the driver of this variable has multiple non-variable sinks, then we would need - // a temporary when rendering the graph. Instead of introducing a temporary, keep the - // first variable that is driven by that driver + // Otherwise if it has drivers if (varp->isDrivenByDfg()) { - DfgVertex* const driverp = varp->source(0); - unsigned nonVarSinks = 0; - const DfgVarPacked* firstSinkVarp = nullptr; - const bool keepFirst = driverp->findSink([&](const DfgVertex& sink) { - if (const DfgVarPacked* const sinkVarp = sink.cast()) { - if (!firstSinkVarp) firstSinkVarp = sinkVarp; - } else { - ++nonVarSinks; + // Can't remove if read in the module and driven here (i.e.: it's an output of the DFG) + if (varp->hasModRefs()) continue; + + // Can't remove if referenced externally, or other special reasons + if (varp->keep()) continue; + + // If the driver of this variable is not an inlined vertex, then we would need a + // temporary when rendering the graph. Instead of introducing a temporary, keep the + // first variable that is driven by that driver. Note that we still remove if the only + // sinks we have are variables, as we might be able to remove all of them (we can be + // sure the not inlined if we have at least 2 non-variable sinks). + if (varp->isDrivenFullyByDfg()) { + DfgVertex* const driverp = varp->source(0); + if (!driverp->inlined()) { + unsigned nonVarSinks = 0; + const DfgVarPacked* firstp = nullptr; + const bool found = driverp->findSink([&](const DfgVertex& sink) { + if (const DfgVarPacked* const sinkVarp = sink.cast()) { + if (!firstp) firstp = sinkVarp; + } else { + ++nonVarSinks; + } + // We can stop as soon as we found the first var, and 2 non-var sinks + return firstp && nonVarSinks >= 2; + }); + // Keep this DfgVarPacked if needed + if (found && firstp == varp) continue; } - // We can stop as soon as we found the first var, and 2 non-var sinks - return firstSinkVarp && nonVarSinks >= 2; - }); - // Keep this DfgVarPacked if needed - if (keepFirst && firstSinkVarp == varp) continue; + } } - // OK, we can delete this DfgVarPacked - ++ctx.m_removed; + // OK, we can delete this DfgVarPacked from the graph. - // If not referenced outside the DFG, then also delete the referenced AstVar, - // as it is now unused. - if (!varp->hasRefs()) varp->varp()->unlinkFrBack()->deleteTree(); + // If not referenced outside the DFG, then also delete the referenced AstVar (now unused). + if (!varp->hasRefs()) { + ++ctx.m_removed; + varp->varp()->unlinkFrBack()->deleteTree(); + } // Unlink and delete vertex varp->unlinkDelete(dfg); @@ -283,14 +295,14 @@ void V3DfgPasses::optimize(DfgGraph& dfg, V3DfgOptimizationContext& ctx) { }; if (dumpDfg() >= 8) dfg.dumpDotAllVarConesPrefixed(ctx.prefix() + "input"); - apply(3, "input ", [&]() {}); - apply(4, "cse ", [&]() { cse(dfg, ctx.m_cseContext0); }); - apply(4, "inlineVars ", [&]() { inlineVars(dfg); }); + apply(3, "input ", [&]() {}); + apply(4, "inlineVars ", [&]() { inlineVars(dfg); }); + apply(4, "cse0 ", [&]() { cse(dfg, ctx.m_cseContext0); }); if (v3Global.opt.fDfgPeephole()) { - apply(4, "peephole ", [&]() { peephole(dfg, ctx.m_peepholeContext); }); + apply(4, "peephole ", [&]() { peephole(dfg, ctx.m_peepholeContext); }); + // We just did CSE above, so without peephole there is no need to run it again these + apply(4, "cse1 ", [&]() { cse(dfg, ctx.m_cseContext1); }); } - apply(4, "cse ", [&]() { cse(dfg, ctx.m_cseContext1); }); - apply(4, "removeVars ", [&]() { removeVars(dfg, ctx.m_removeVarsContext); }); - apply(3, "optimized ", [&]() { removeUnused(dfg); }); + apply(4, "removeVars ", [&]() { removeVars(dfg, ctx.m_removeVarsContext); }); if (dumpDfg() >= 8) dfg.dumpDotAllVarConesPrefixed(ctx.prefix() + "optimized"); } diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index df78ea1d3..c17f3776c 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -1615,18 +1615,6 @@ class V3DfgPeephole final : public DfgVisitor { #undef APPLYING - // // Process one vertex. Return true if graph changed - void processVertex(DfgVertex* vtxp) { - // Check if unused and remove if so - if (!vtxp->hasSinks()) { - deleteVertex(vtxp); - return; - } - - // Transform node (might get deleted) - iterate(vtxp); - } - V3DfgPeephole(DfgGraph& dfg, V3DfgPeepholeContext& ctx) : m_dfg{dfg} , m_ctx{ctx} { @@ -1650,8 +1638,13 @@ class V3DfgPeephole final : public DfgVisitor { m_workListp = vtxp->getUser(); VL_PREFETCH_RW(m_workListp); vtxp->setUser(nullptr); - // Process the vertex - processVertex(vtxp); + // Remove unused vertices as we gp + if (!vtxp->hasSinks()) { + deleteVertex(vtxp); + continue; + } + // Transform node (might get deleted in the process) + iterate(vtxp); } }