Fix NBA shared flag reuse (#5848)

Fixes #5847

src/V3Delayed.cpp

@@ -155,6 +155,7 @@ class DelayedVisitor final : public VNVisitor {
             struct {  // Stuff needed for Scheme::FlagShared
                 AstActive* activep;  // The active block for the Pre/Post logic
                 AstAlwaysPost* postp;  // The post block for commiting results
+                AstVarScope* commitFlagp;  // The commit flag variable, for reuse
                 AstIf* commitIfp;  // The previous if statement for committing, for reuse
             } m_flagSharedKit;
             struct {  // Stuff needed for Scheme::FlagUnique
@@ -421,6 +422,9 @@ class DelayedVisitor final : public VNVisitor {
         AstAlwaysPost* const postp = new AstAlwaysPost{flp};
         activep->addStmtsp(postp);
         vscpInfo.flagSharedKit().postp = postp;
+        // Initialize
+        vscpInfo.flagSharedKit().commitFlagp = nullptr;
+        vscpInfo.flagSharedKit().commitIfp = nullptr;
     }
     void convertSchemeFlagShared(AstAssignDly* nodep, AstVarScope* vscp, VarScopeInfo& vscpInfo) {
         UASSERT_OBJ(vscpInfo.m_scheme == Scheme::FlagShared, vscp, "Inconsistent NBA scheme");
@@ -443,14 +447,16 @@ class DelayedVisitor final : public VNVisitor {
         const bool consecutive = nodep == m_nextDlyp;
         m_nextDlyp = VN_CAST(nodep->nextp(), AssignDly);
 
+        VarScopeInfo* const prevVscpInfop = consecutive ? &m_vscpInfo(m_prevVscp) : nullptr;
+
         // We can reuse the flag of the previous assignment if:
         const bool reuseTheFlag =
             // Consecutive NBAs
             consecutive
             // ... that use the same scheme
-            && m_vscpInfo(m_prevVscp).m_scheme == Scheme::FlagShared
+            && prevVscpInfop->m_scheme == Scheme::FlagShared
             // ... and share the same overall update domain
-            && m_vscpInfo(m_prevVscp).senTreep()->sameTree(vscpInfo.senTreep());
+            && prevVscpInfop->senTreep()->sameTree(vscpInfo.senTreep());
 
         if (!reuseTheFlag) {
             // Create new flag
@@ -466,10 +472,25 @@ class DelayedVisitor final : public VNVisitor {
             // Add the 'Post' scheduled commit
             AstIf* const ifp = new AstIf{flp, new AstVarRef{flp, flagVscp, VAccess::READ}};
             vscpInfo.flagSharedKit().postp->addStmtsp(ifp);
+            vscpInfo.flagSharedKit().commitFlagp = flagVscp;
             vscpInfo.flagSharedKit().commitIfp = ifp;
         } else {
-            // Reuse the commit block of the previous assignment
+            if (vscp != m_prevVscp) {
-            vscpInfo.flagSharedKit().commitIfp = m_vscpInfo(m_prevVscp).flagSharedKit().commitIfp;
+                // Different variable, ensure the commit block exists for this variable,
+                // can reuse existing one with the same flag, otherwise create a new one.
+                AstVarScope* const flagVscp = prevVscpInfop->flagSharedKit().commitFlagp;
+                UASSERT_OBJ(flagVscp, nodep, "Commit flag of previous assignment should exist");
+                if (vscpInfo.flagSharedKit().commitFlagp != flagVscp) {
+                    AstIf* const ifp = new AstIf{flp, new AstVarRef{flp, flagVscp, VAccess::READ}};
+                    vscpInfo.flagSharedKit().postp->addStmtsp(ifp);
+                    vscpInfo.flagSharedKit().commitFlagp = flagVscp;
+                    vscpInfo.flagSharedKit().commitIfp = ifp;
+                }
+            } else {
+                // Same variable, reuse the commit block of the previous assignment
+                vscpInfo.flagSharedKit().commitFlagp = prevVscpInfop->flagSharedKit().commitFlagp;
+                vscpInfo.flagSharedKit().commitIfp = prevVscpInfop->flagSharedKit().commitIfp;
+            }
             ++m_nSharedSetFlags;
         }
         // Commit the captured value to the captured destination

test_regress/t/t_nba_shared_flag_reuse.py

@@ -0,0 +1,18 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 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
+
+import vltest_bootstrap
+
+test.scenarios('simulator')
+
+test.compile(verilator_flags2=['--timing', '--binary'])
+
+test.execute()
+
+test.passes()

test_regress/t/t_nba_shared_flag_reuse.v

@@ -0,0 +1,72 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2025 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+`define stop $stop
+`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d:  got=%0x exp=%0x (%s !== %s)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0);
+
+module t;
+
+  reg clk = 1'b1;
+  reg reset = 1'b1;
+
+  reg aw_valid = 1'b0;
+  reg w_valid = 1'b0;
+
+  reg r_valid;
+
+  reg [31:0] addr [1:0];
+  reg [7:0] len [1:0];
+
+  always #5 clk = ~clk;
+
+  initial begin
+    #5; // Align with negedge clk
+
+    #20;
+    `checkh(addr[0], 32'h0000_0000);
+    reset = 1'b0;
+
+    #20;
+    `checkh(addr[0], 32'h0000_0000);
+    aw_valid = 1'b1;
+    w_valid = 1'b1;
+
+    #10;
+    `checkh(addr[0], 32'h4444_4444);
+    aw_valid = 1'b0;
+
+    #10;
+    `checkh(addr[0], 32'h2222_2222);
+    w_valid = 1'b0;
+
+    #10;
+    `checkh(addr[0], 32'h2222_2222);
+    $write("*-* All Finished *-*\n");
+    $finish;
+  end
+
+
+  always @(posedge clk) begin
+    if (reset) begin
+      r_valid <= 0;
+    end
+    else begin
+      if (r_valid) begin
+        addr[0] <= 32'h11111111;
+        len[0] <= len[0] - 1;
+      end
+      if (w_valid) begin
+        addr[0] <= 32'h22222222;
+      end
+      if (aw_valid) begin
+        addr[0] <= 32'h33333333;
+        len[0] <= 8'hff;
+        if (w_valid)
+          addr[0] <= 32'h44444444;
+      end
+    end
+  end
+endmodule