Optimize Dfg with relaxed live variable analysis (#7739)

Relax the live variable analysis performed by Dfg to bail on fewer
cases. This analysis was already conservative (meaning it might think
variables are live when they are not), which is good enough for Dfg use.
This change in particular enables synthesizing more complex logic
involving arrays, e.g. those introduce by V3Table creating lookup
tables.

src/V3AstNodeOther.h

@@ -987,6 +987,7 @@ public:
     bool maybePointedTo() const override VL_MT_SAFE { return true; }
     void cloneRelink() override { V3ERROR_NA; }  // Not cloneable
     AstModule* modp() const { return m_modp; }
+    AstScope* scopep() const { return m_scopep; }
 
     // Find a table (unpacked array) within the constant pool which is initialized with the
     // given value, or create one if one does not already exists. The returned VarScope *might*

src/V3CfgLiveVariables.cpp

@@ -51,28 +51,18 @@ class CfgLiveVariables final : VNVisitorConst {
 
     // METHODS
 
-    // This is to match DFG, but can be extended independently - eventually should handle all
-    static bool isSupportedPackedDType(const AstNodeDType* dtypep) {
-        dtypep = dtypep->skipRefp();
+    static bool isPacked(const AstVarScope* vscp) {
+        AstNodeDType* const dtypep = vscp->dtypep()->skipRefp();
         if (const AstBasicDType* const typep = VN_CAST(dtypep, BasicDType)) {
             return typep->keyword().isIntNumeric();
         }
-        if (const AstPackArrayDType* const typep = VN_CAST(dtypep, PackArrayDType)) {
-            return isSupportedPackedDType(typep->subDTypep());
-        }
+        if (VN_IS(dtypep, PackArrayDType)) return true;
         if (const AstNodeUOrStructDType* const typep = VN_CAST(dtypep, NodeUOrStructDType)) {
             return typep->packed();
         }
         return false;
     }
 
-    // Check and return if variable is incompatible
-    bool incompatible(AstVarScope* vscp) {
-        if (!isSupportedPackedDType(vscp->dtypep())) return true;
-        if (vscp->varp()->ignoreSchedWrite()) return true;
-        return false;
-    }
-
     void updateGen(AstNode* nodep) {
         UASSERT_OBJ(!m_abort, nodep, "Doing useless work");
         m_abort = nodep->exists([&](const AstVarRef* refp) {
@@ -80,8 +70,6 @@ class CfgLiveVariables final : VNVisitorConst {
             if (refp->access().isWriteOnly()) return false;
             // Grab referenced variable
             AstVarScope* const vscp = refp->varScopep();
-            // Bail if not a compatible type
-            if (incompatible(vscp)) return true;
             // Add to gen set, if not killed - assume whole variable read
             if (m_currp->m_kill.count(vscp)) return false;
             m_currp->m_gen.emplace(vscp);
@@ -96,11 +84,13 @@ class CfgLiveVariables final : VNVisitorConst {
             if (refp->access().isReadOnly()) return false;
             // Grab referenced variable
             AstVarScope* const vscp = refp->varScopep();
-            // Bail if not a compatible type
-            if (incompatible(vscp)) return true;
-            // If whole written, add to kill set
-            if (refp->nextp()) return false;
-            if (VN_IS(refp->abovep(), Sel)) return false;
+            // Safety case: bail on variables with unusual semantics
+            if (vscp->varp()->ignoreSchedWrite()) return true;
+            // If not a packed variable, assume it's not a whole update, don't add to kill set
+            if (!isPacked(vscp)) return false;
+            // If packed, and updated partially, don't add to kill set
+            if (refp->backp()->nextp() != refp && VN_IS(refp->abovep(), Sel)) return false;
+            // Whole variable is writen, add to kill set
             m_currp->m_kill.emplace(vscp);
             return false;
         });
@@ -198,7 +188,7 @@ public:
         CfgLiveVariables analysis{cfg};
         // If failed, return nullptr
         if (analysis.m_abort) return nullptr;
-        // Gather variables live in to the entry blockstd::unique_ptr<std::vector<AstVarScope*>>
+        // Gather variables live in to the entry block
         const std::unordered_set<AstVarScope*>& lin = analysis.m_blockState[cfg.enter()].m_liveIn;
         std::vector<AstVarScope*>* const resultp
             = new std::vector<AstVarScope*>{lin.begin(), lin.end()};

src/V3Dfg.cpp

@@ -906,17 +906,20 @@ AstScope* DfgVertex::scopep(ScopeCache& cache, bool tryResultVar) VL_MT_DISABLED
         }
     }
 
+    AstScope* const rootp = v3Global.rootp()->topScopep()->scopep();
+    AstScope* const constPoolp = v3Global.rootp()->constPoolp()->scopep();
+
     // Note: the recursive invocation can cause a re-hash but that will not invalidate references
     AstScope*& resultr = cache[this];
     if (!resultr) {
         // Mark to prevent infinite recursion on circular graphs - should never be called on such
         resultr = reinterpret_cast<AstScope*>(1);
-        // Find scope based on sources, falling back on the root scope
-        AstScope* const rootp = v3Global.rootp()->topScopep()->scopep();
+        // Find scope based on sources, falling back on the root scope,
+        // also make sure it's not the constant pool scope, which is special.
         AstScope* foundp = nullptr;
         foreachSource([&](DfgVertex& src) {
             AstScope* const scp = src.scopep(cache, true);
-            if (scp != rootp) {
+            if (scp != rootp && scp != constPoolp) {
                 foundp = scp;
                 return true;
             }

test_regress/t/t_case_huge.py

@@ -11,7 +11,8 @@ import vltest_bootstrap
 
 test.scenarios('simulator')
 
-test.compile(verilator_flags2=["--stats"])
+# This tests combining CFuncs, but Dfg would inline the submodule, disabling
+test.compile(verilator_flags2=["--stats", "-fno-dfg"])
 
 test.execute()
 

test_regress/t/t_dfg_synthesis.v

@@ -577,4 +577,13 @@ module t (
   end
   `signal(VIA_CRESET, via_creset);
 
+  logic [1:0] LUT [2] = '{0: 2'b11, 1: 2'b10};
+  logic [1:0] array_read;
+  always_comb begin
+    array_read = 2'd0;
+    array_read += LUT[rand_a[0]];
+    array_read += LUT[rand_a[1]];
+  end
+  `signal(ARRAY_READ, array_read);
+
 endmodule

test_regress/t/t_gate_inline_wide_noexclude_arraysel.py

@@ -11,7 +11,7 @@ import vltest_bootstrap
 
 test.scenarios('vlt')
 
-test.lint(verilator_flags2=['--stats', '--expand-limit 5'])
+test.lint(verilator_flags2=['--stats', '--expand-limit 5', '-fno-dfg'])
 
 test.file_grep(test.stats, r'Optimizations, Gate excluded wide expressions\s+(\d+)', 0)
 test.file_grep(test.stats, r'Optimizations, Gate sigs deleted\s+(\d+)', 1)