DFG: make variable inlining part of the peephole optimizer

This saves some traversals and prepares us to better handle cyclic DFGs.

src/V3DfgPasses.cpp

@@ -75,20 +75,6 @@ V3DfgOptimizationContext::~V3DfgOptimizationContext() {
             "Inconsistent statistics");
 }
 
-// 'Inline' DfgVar nodes with known drivers
-void V3DfgPasses::inlineVars(DfgGraph& dfg) {
-    dfg.forEachVertex([](DfgVertex& vtx) {
-        // For each DfgVar that has a known driver
-        if (DfgVar* const varVtxp = vtx.cast<DfgVar>()) {
-            if (varVtxp->isDrivenFullyByDfg()) {
-                // Make consumers of the DfgVar consume the driver directly
-                DfgVertex* const driverp = varVtxp->source(0);
-                varVtxp->forEachSinkEdge([=](DfgEdge& edge) { edge.relinkSource(driverp); });
-            }
-        }
-    });
-}
-
 // Common subexpression elimination
 void V3DfgPasses::cse(DfgGraph& dfg, V3DfgCseContext& ctx) {
     DfgVertex::HashCache hashCache;
@@ -183,16 +169,6 @@ void V3DfgPasses::optimize(DfgGraph& dfg, V3DfgOptimizationContext& ctx) {
     // There is absolutely nothing useful we can do with a graph of size 2 or less
     if (dfg.size() <= 2) return;
 
-    // We consider a DFG trivial if it contains no more than 1 non-variable, non-constant vertex,
-    // or if if it contains a DfgConcat, which can be introduced through assinment coalescing.
-    unsigned excitingVertices = 0;
-    const bool isTrivial = !dfg.findVertex<DfgVertex>([&](const DfgVertex& vtx) {  //
-        if (vtx.is<DfgVar>()) return false;
-        if (vtx.is<DfgConst>()) return false;
-        if (vtx.is<DfgConcat>()) return true;
-        return ++excitingVertices >= 2;
-    });
-
     int passNumber = 0;
 
     const auto apply = [&](int dumpLevel, const string name, std::function<void()> pass) {
@@ -206,23 +182,15 @@ void V3DfgPasses::optimize(DfgGraph& dfg, V3DfgOptimizationContext& ctx) {
         ++passNumber;
     };
 
-    if (!isTrivial) {
+    if (dumpDfg() >= 8) dfg.dumpDotAllVarConesPrefixed(ctx.prefix() + "input");
-        // Optimize non-trivial graph
+    apply(3, "input         ", [&]() {});
-        if (dumpDfg() >= 8) { dfg.dumpDotAllVarConesPrefixed(ctx.prefix() + "input"); }
+    apply(4, "cse           ", [&]() { cse(dfg, ctx.m_cseContext0); });
-        apply(3, "input         ", [&]() {});
+    if (v3Global.opt.fDfgPeephole()) {
-        apply(4, "inlineVars    ", [&]() { inlineVars(dfg); });
+        apply(4, "peephole      ", [&]() { peephole(dfg, ctx.m_peepholeContext); });
-        apply(4, "cse           ", [&]() { cse(dfg, ctx.m_cseContext0); });
+        // Without peephole no variables will be redundant, and we just did CSE, so skip these
-        if (v3Global.opt.fDfgPeephole()) {
-            apply(4, "peephole      ", [&]() { peephole(dfg, ctx.m_peepholeContext); });
-        }
         apply(4, "removeVars    ", [&]() { removeVars(dfg, ctx.m_removeVarsContext); });
         apply(4, "cse           ", [&]() { cse(dfg, ctx.m_cseContext1); });
-        apply(3, "optimized     ", [&]() { removeUnused(dfg); });
-        if (dumpDfg() >= 8) { dfg.dumpDotAllVarConesPrefixed(ctx.prefix() + "optimized"); }
-    } else {
-        // We can still eliminate redundancies from trivial graphs
-        apply(5, "trivial-input         ", [&]() {});
-        apply(6, "trivial-inlineVars    ", [&]() { inlineVars(dfg); });
-        apply(5, "trivial-optimized     ", [&]() { removeVars(dfg, ctx.m_removeVarsContext); });
     }
+    apply(3, "optimized     ", [&]() { removeUnused(dfg); });
+    if (dumpDfg() >= 8) dfg.dumpDotAllVarConesPrefixed(ctx.prefix() + "optimized");
 }

src/V3DfgPasses.h

@@ -100,8 +100,6 @@ AstModule* dfgToAst(DfgGraph&, V3DfgOptimizationContext&);
 // Intermediate/internal operations
 //===========================================================================
 
-// Inline variables
-void inlineVars(DfgGraph&);
 // Common subexpression elimination
 void cse(DfgGraph&, V3DfgCseContext&);
 // Peephole optimizations

src/V3DfgPeephole.cpp

@@ -1102,19 +1102,30 @@ class V3DfgPeephole final : public DfgVisitor {
         }
     }
 
+    void visit(DfgVar* vtxp) override {
+        // Inline variables fully driven by the logic represented by the DFG
+        if (vtxp->hasSinks() && vtxp->isDrivenFullyByDfg()) {
+            APPLYING(INLINE_VAR) {
+                // Make consumers of the DfgVar consume the driver directly
+                DfgVertex* const driverp = vtxp->source(0);
+                vtxp->forEachSinkEdge([=](DfgEdge& edge) { edge.relinkSource(driverp); });
+            }
+        }
+    }
+
 #undef APPLYING
 
     // Process one vertex. Return true if graph changed
     bool processVertex(DfgVertex& vtx) {
-        // Keep DfgVars in this pass, we will remove them later if they become redundant
+        // Keep DfgVertexLValue vertices in this pass. We will remove them later if they become
-        // Note: We want to keep the original variables for non-var vertices that drive multiple
+        // redundant. We want to keep the original variables for non-var vertices that drive
-        // sinks (otherwise we would need to introduce a temporary, it is better for debugging to
+        // multiple sinks (otherwise we would need to introduce a temporary, but it is better for
-        // keep the original variable name, if one is available), so we can't remove redundant
+        // debugging to keep the original variable name, if one is available), so we can't remove
-        // variables here.
+        // redundant variables here.
-        if (vtx.is<DfgVar>()) return false;
+        const bool keep = vtx.is<DfgVar>();
 
         // If it has no sinks (unused), we can remove it
-        if (!vtx.hasSinks()) {
+        if (!keep && !vtx.hasSinks()) {
             vtx.unlinkDelete(m_dfg);
             return true;
         }
@@ -1122,7 +1133,15 @@ class V3DfgPeephole final : public DfgVisitor {
         // Transform node
         m_changed = false;
         iterate(&vtx);
-        if (!vtx.hasSinks()) vtx.unlinkDelete(m_dfg);  // If it became unused, we can remove it
+
+        // If it became unused, we can remove it
+        if (!keep && !vtx.hasSinks()) {
+            UASSERT_OBJ(m_changed, &vtx, "'m_changed' must be set if node became unused");
+            vtx.unlinkDelete(m_dfg);
+            return true;
+        }
+
+        // Return the changed status
         return m_changed;
     }
 

src/V3DfgPeephole.h

@@ -82,7 +82,8 @@
     _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, REPLACE_COND_WITH_THEN_BRANCH_ZERO) \
     _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, REPLACE_COND_WITH_THEN_BRANCH_ONES) \
     _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, REPLACE_COND_WITH_ELSE_BRANCH_ZERO) \
-    _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, REPLACE_COND_WITH_ELSE_BRANCH_ONES)
+    _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, REPLACE_COND_WITH_ELSE_BRANCH_ONES) \
+    _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, INLINE_VAR)
 
 // clang-format on