Fix merging of impure assignments in gate optimization (#6629) (#6630)

src/V3Gate.cpp

@@ -1097,6 +1097,7 @@ public:
 class GateMergeAssignments final {
     GateGraph& m_graph;
     size_t m_statAssignMerged = 0;  // Statistic tracking
+    std::vector<GateLogicVertex*> m_toRemove;  // Logic vertices to delete
 
     // assemble two Sel into one if possible
     AstSel* merge(AstSel* prevSelp, AstSel* currSelp) {
@@ -1146,18 +1147,16 @@ class GateMergeAssignments final {
                 // replace preSel with newSel
                 prevSelp->replaceWith(newSelp);
                 VL_DO_DANGLING(prevSelp->deleteTree(), prevSelp);
-                // create new rhs for pre assignment
+                // Update RHS of the prev assignment, reusing existing parts (might be impure).
-                AstNode* const newRhsp = new AstConcat{prevAssignp->rhsp()->fileline(),
+                prevAssignp->rhsp(new AstConcat{prevAssignp->rhsp()->fileline(),
-                                                       prevAssignp->rhsp()->cloneTreePure(false),
+                                                prevAssignp->rhsp()->unlinkFrBack(),
-                                                       assignp->rhsp()->cloneTreePure(false)};
+                                                assignp->rhsp()->unlinkFrBack()});
-                AstNode* const prevRhsp = prevAssignp->rhsp();
-                prevRhsp->replaceWith(newRhsp);
-                VL_DO_DANGLING(prevRhsp->deleteTree(), prevRhsp);
                 // Why do we care about the type of an assignment?
                 prevAssignp->dtypeChgWidthSigned(prevAssignp->width() + assignp->width(),
                                                  prevAssignp->width() + assignp->width(),
                                                  VSigning::SIGNED);
-                // Don't need to delete assignp, will be handled
+                // We will delete the current assignment
+                m_toRemove.emplace_back(lVtxp);
 
                 // Update the graph
                 while (V3GraphEdge* const iedgep = lVtxp->inEdges().frontp()) {
@@ -1182,6 +1181,12 @@ class GateMergeAssignments final {
         for (V3GraphVertex& vtx : graph.vertices()) {
             if (GateVarVertex* const vVtxp = vtx.cast<GateVarVertex>()) process(vVtxp);
         }
+        // Delete merged assignments
+        for (GateLogicVertex* const lVtxp : m_toRemove) {
+            AstNode* const nodep = lVtxp->nodep();
+            VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
+            VL_DO_DANGLING(lVtxp->unlinkDelete(&m_graph), lVtxp);
+        }
     }
 
     ~GateMergeAssignments() {

test_regress/t/t_cover_line.out

@@ -443,6 +443,11 @@
 -000001  point: comment=block hier=top.t.cond1
            string s;
         
+           struct packed {
+               logic unsigned [15:0] a;
+               logic unsigned [15:0] b;
+           } pstruct;
+        
  000021    function logic func_side_effect;
 +000021  point: comment=block hier=top.t.cond1
  000021       $display("SIDE EFFECT");
@@ -535,5 +540,17 @@
  000011       else k = 0;
 +000011  point: comment=else hier=top.t.cond1
            end
+        
+~000010    assign pstruct.a = cyc == 1 ? 16'd2 : 16'd3;
+-000001  point: comment=cond_then hier=top.t.cond1
++000010  point: comment=cond_else hier=top.t.cond1
+           assign pstruct.b = 16'd0;
+        
+ 000010    always @(posedge clk) begin
++000010  point: comment=block hier=top.t.cond1
+%000009      if (cyc == 2) $display("%08x", pstruct);
+-000001  point: comment=if hier=top.t.cond1
+-000009  point: comment=else hier=top.t.cond1
+           end
         endmodule
         

test_regress/t/t_cover_line.v

@@ -315,6 +315,11 @@ module cond(input logic clk, input int cyc);
    Getter1 getter1 = new;
    string s;
 
+   struct packed {
+       logic unsigned [15:0] a;
+       logic unsigned [15:0] b;
+   } pstruct;
+
    function logic func_side_effect;
       $display("SIDE EFFECT");
       return 1;
@@ -361,4 +366,11 @@ module cond(input logic clk, input int cyc);
       if (k ? 1 : 0) k = 1;
       else k = 0;
    end
+
+   assign pstruct.a = cyc == 1 ? 16'd2 : 16'd3;
+   assign pstruct.b = 16'd0;
+
+   always @(posedge clk) begin
+     if (cyc == 2) $display("%08x", pstruct);
+   end
 endmodule

test_regress/t/t_cover_line_cc.info.out

@@ -149,65 +149,72 @@ DA:304,1
 DA:305,20
 DA:306,20
 DA:315,1
-DA:318,21
+DA:323,21
-DA:319,21
+DA:324,21
-DA:320,21
+DA:325,21
-DA:323,10
+DA:328,10
-DA:325,10
+DA:330,10
-DA:328,31
+DA:333,31
-BRDA:328,0,0,0
+BRDA:333,0,0,0
-BRDA:328,0,1,31
+BRDA:333,0,1,31
-DA:329,28
+DA:334,28
-BRDA:329,0,0,3
+BRDA:334,0,0,3
-BRDA:329,0,1,28
+BRDA:334,0,1,28
-DA:330,21
+DA:335,21
-BRDA:330,0,0,21
+BRDA:335,0,0,21
-BRDA:330,0,1,0
+BRDA:335,0,1,0
-DA:331,10
+DA:336,10
-DA:332,10
+DA:337,10
-BRDA:332,0,0,10
+BRDA:337,0,0,10
-BRDA:332,0,1,3
+BRDA:337,0,1,3
-BRDA:332,0,2,7
+BRDA:337,0,2,7
-DA:333,10
+DA:338,10
-BRDA:333,0,0,10
+BRDA:338,0,0,10
-BRDA:333,0,1,0
-BRDA:333,0,2,10
-DA:335,19
-BRDA:335,0,0,12
-BRDA:335,0,1,19
-BRDA:335,0,2,7
-BRDA:335,0,3,5
-DA:338,11
-BRDA:338,0,0,11
 BRDA:338,0,1,0
-DA:344,11
+BRDA:338,0,2,10
-BRDA:344,0,0,10
+DA:340,19
-BRDA:344,0,1,11
+BRDA:340,0,0,12
-DA:347,11
+BRDA:340,0,1,19
-DA:348,10
+BRDA:340,0,2,7
-BRDA:348,0,0,0
+BRDA:340,0,3,5
-BRDA:348,0,1,1
+DA:343,11
-BRDA:348,0,2,1
+BRDA:343,0,0,11
-BRDA:348,0,3,10
+BRDA:343,0,1,0
-DA:349,10
+DA:349,11
-DA:351,11
+BRDA:349,0,0,10
-BRDA:351,0,0,11
+BRDA:349,0,1,11
-BRDA:351,0,1,1
+DA:352,11
-BRDA:351,0,2,10
+DA:353,10
-DA:354,55
+BRDA:353,0,0,0
-BRDA:354,0,0,11
+BRDA:353,0,1,1
-BRDA:354,0,1,55
+BRDA:353,0,2,1
-DA:355,55
+BRDA:353,0,3,10
-DA:357,44
+DA:354,10
-BRDA:357,0,0,11
+DA:356,11
-BRDA:357,0,1,11
+BRDA:356,0,0,11
-BRDA:357,0,2,33
+BRDA:356,0,1,1
-BRDA:357,0,3,44
+BRDA:356,0,2,10
-DA:358,44
+DA:359,55
-DA:361,11
+BRDA:359,0,0,11
-BRDA:361,0,0,0
+BRDA:359,0,1,55
-BRDA:361,0,1,11
+DA:360,55
-DA:362,11
+DA:362,44
-BRF:79
+BRDA:362,0,0,11
-BRH:32
+BRDA:362,0,1,11
+BRDA:362,0,2,33
+BRDA:362,0,3,44
+DA:363,44
+DA:366,11
+BRDA:366,0,0,0
+BRDA:366,0,1,11
+DA:367,11
+DA:370,10
+BRDA:370,0,0,1
+BRDA:370,0,1,10
+DA:373,10
+DA:374,9
+BRDA:374,0,0,1
+BRDA:374,0,1,9
+BRF:83
+BRH:33
 end_of_record