Fix bad assertion in DFG variable elimination

Fixes #4943

src/V3DfgOptimizer.cpp

@@ -240,7 +240,7 @@ void V3DfgOptimizer::optimize(AstNetlist* netlistp, const string& label) {
     UINFO(2, __FUNCTION__ << ": " << endl);
 
     // NODE STATE
-    // AstVar::user1 -> Used by V3DfgPasses::astToDfg
+    // AstVar::user1 -> Used by V3DfgPasses::astToDfg, V3DfgPasses::eliminateVars
     // AstVar::user2 -> bool: Flag indicating referenced by AstVarXRef (set just below)
     // AstVar::user3 -> bool: Flag indicating written by logic not representable as DFG
     //                        (set by V3DfgPasses::astToDfg)

src/V3DfgPasses.cpp

@@ -274,8 +274,10 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) {
         workListp = &vtx;
     };
 
-    // Variable replacements to apply in the module
+    // List of variables we are replacing
-    std::unordered_map<AstVar*, AstVar*> replacements;
+    std::vector<AstVar*> replacedVariables;
+    // AstVar::user1p() : AstVar* -> The replacement variables
+    const VNUser1InUse user1InUse;
 
     // Process the work list
     while (workListp != sentinelp) {
@@ -315,24 +317,16 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) {
             ++ctx.m_varsRemoved;
             varp->replaceWith(varp->source(0));
             varp->varp()->unlinkFrBack()->deleteTree();
-        } else if (DfgVarPacked* const canonp = varp->source(0)->cast<DfgVarPacked>()) {
+        } else if (DfgVarPacked* const driverp = varp->source(0)->cast<DfgVarPacked>()) {
-            // If it's driven from another canonical variable, it can be replaced by that.
+            // If it's driven from another variable, it can be replaced by that. However, we do not
-            // However, we don't want to propagate SystemC variables into the design
+            // want to propagate SystemC variables into the design.
-            if (canonp->varp()->isSc()) continue;
+            if (driverp->varp()->isSc()) continue;
-            // Note that if this is a duplicate variable, then the canonical variable must
+            // Mark it for replacement
-            // be either kept or have module references. We ensured this earlier when picking
-            // the canonical variable in the regularize pass. Additionally, it's possible
-            // neither of those holds, if an otherwise unreferenced variable drives another one.
-            // In that case it's true that it must not have a source, so it cannot itself be
-            // substituted. This condition can be relaxed if needed by supporting recursive
-            // substitution below.
-            UASSERT_OBJ(canonp->keep() || canonp->hasDfgRefs() || canonp->hasModRefs()
-                            || !canonp->isDrivenByDfg(),
-                        varp, "Canonical variable should be kept or have module refs");
             ++ctx.m_varsReplaced;
             UASSERT_OBJ(!varp->hasSinks(), varp, "Variable inlining should make this impossible");
-            const bool newEntry = replacements.emplace(varp->varp(), canonp->varp()).second;
+            UASSERT(!varp->varp()->user1p(), "Replacement already exists");
-            UASSERT_OBJ(newEntry, varp->varp(), "Replacement already exists");
+            replacedVariables.emplace_back(varp->varp());
+            varp->varp()->user1p(driverp->varp());
         } else {
             // Otherwise this *is* the canonical var
             continue;
@@ -345,19 +339,18 @@ void V3DfgPasses::eliminateVars(DfgGraph& dfg, V3DfgEliminateVarsContext& ctx) {
     }
 
     // Job done if no replacements possible
-    if (replacements.empty()) return;
+    if (replacedVariables.empty()) return;
 
     // Apply variable replacements in the module
     VNDeleter deleter;
     dfg.modulep()->foreach([&](AstVarRef* refp) {
-        const auto it = replacements.find(refp->varp());
+        AstVar* varp = refp->varp();
-        if (it == replacements.end()) return;
+        while (AstVar* const replacementp = VN_AS(varp->user1p(), Var)) varp = replacementp;
-        refp->replaceWith(new AstVarRef{refp->fileline(), it->second, refp->access()});
+        refp->varp(varp);
-        deleter.pushDeletep(refp);
     });
 
     // Remove the replaced variables
-    for (const auto& pair : replacements) pair.first->unlinkFrBack()->deleteTree();
+    for (AstVar* const varp : replacedVariables) varp->unlinkFrBack()->deleteTree();
 }
 
 void V3DfgPasses::optimize(DfgGraph& dfg, V3DfgOptimizationContext& ctx) {

test_regress/t/t_dfg_4943.pl

@@ -0,0 +1,16 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2024 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(vlt => 1);
+
+compile();
+
+ok(1);
+1;

test_regress/t/t_dfg_4943.v

@@ -0,0 +1,20 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+module top(input wire i, output wire o);
+
+  // Partially driven, and drives other var with non-DFG refs
+  wire logic [1:0] x;
+
+  assign x[0] = i;
+
+  assign o = |x;
+
+  wire logic [1:0] alt = x;
+
+  always @(alt) $display(alt);
+
+endmodule