Fix inlined expression with assignment under LHS of NBA (#5736) (#5740)

src/V3Delayed.cpp

@@ -208,6 +208,7 @@ class DelayedVisitor final : public VNVisitor {
     // NODE STATE
     //  AstVar::user1()         -> bool.  Set true if already issued MULTIDRIVEN warning
     //  AstVarRef::user1()      -> bool.  Set true if target of NBA
+    //  AstAssignDly::user1()   -> bool.  Set true if already visited
     //  AstNodeModule::user1p() -> std::unorded_map<std::string, AstVar*> temp map via m_varMap
     //  AstVarScope::user1p()   -> VarScopeInfo via m_vscpInfo
     //  AstVarScope::user2p()   -> AstVarRef*: First write reference to the Variable
@@ -909,6 +910,9 @@ class DelayedVisitor final : public VNVisitor {
         VL_DO_DANGLING(nodep->deleteTree(), nodep);
     }
     void visit(AstAssignDly* nodep) override {
+        // Prevent double processing due to AstExprStmt being moved before this node
+        if (nodep->user1SetOnce()) return;
+
         if (m_cfuncp) {
             if (!v3Global.rootp()->nbaEventp()) {
                 nodep->v3warn(
@@ -922,21 +926,31 @@ class DelayedVisitor final : public VNVisitor {
         UASSERT_OBJ(m_inSuspendableOrFork || m_activep->hasClocked(), nodep,
                     "<= assignment in non-clocked block, should have been converted in V3Active");
 
-        // Grab the reference to the target of the NBA
+        // Grab the reference to the target of the NBA, also lift ExprStmt statements on the LHS
         VL_RESTORER(m_currNbaLhsRefp);
         UASSERT_OBJ(!m_currNbaLhsRefp, nodep, "NBAs should not nest");
-        nodep->lhsp()->foreach([&](AstVarRef* nodep) {
-            // Ignore reads (e.g.: '_[*here*] <= _')
-            if (nodep->access().isReadOnly()) return;
-            // A RW ref on the LHS (e.g.: '_[preInc(*here*)] <= _') is asking for trouble at this
-            // point. These should be lowered in an earlier pass into sequenced temporaries.
-            UASSERT_OBJ(!nodep->access().isRW(), nodep, "RW ref on LHS of NBA");
-            // Multiple target variables
-            // (e.g.: '{*here*, *and here*} <= _',or '*here*[*and here* = _] <= _').
-            // These should be lowered in an earlier pass into sequenced statements.
-            UASSERT_OBJ(!m_currNbaLhsRefp, nodep, "Multiple Write refs on LHS of NBA");
-            // Hold on to it
-            m_currNbaLhsRefp = nodep;
+        nodep->lhsp()->foreach([&](AstNode* currp) {
+            if (AstExprStmt* const exprp = VN_CAST(currp, ExprStmt)) {
+                // Move statements before the NBA
+                nodep->addHereThisAsNext(exprp->stmtsp()->unlinkFrBackWithNext());
+                // Replace with result
+                currp->replaceWith(exprp->resultp()->unlinkFrBack());
+                // Get rid of the AstExprStmt
+                VL_DO_DANGLING(pushDeletep(currp), currp);
+            } else if (AstVarRef* const refp = VN_CAST(currp, VarRef)) {
+                // Ignore reads (e.g.: '_[*here*] <= _')
+                if (refp->access().isReadOnly()) return;
+                // A RW ref on the LHS (e.g.: '_[preInc(*here*)] <= _') is asking for trouble at
+                // this point. These should be lowered in an earlier pass into sequenced
+                // temporaries.
+                UASSERT_OBJ(!refp->access().isRW(), refp, "RW ref on LHS of NBA");
+                // Multiple target variables
+                // (e.g.: '{*here*, *and here*} <= _',or '*here*[*and here* = _] <= _').
+                // These should be lowered in an earlier pass into sequenced statements.
+                UASSERT_OBJ(!m_currNbaLhsRefp, refp, "Multiple Write refs on LHS of NBA");
+                // Hold on to it
+                m_currNbaLhsRefp = refp;
+            }
         });
         // The target variable of the NBA (there can only be one per NBA at this point)
         AstVarScope* const vscp = m_currNbaLhsRefp->varScopep();

test_regress/t/t_exprstmt_on_lhs_of_nba.py

@@ -0,0 +1,16 @@
+#!/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('vlt')
+
+test.compile()
+
+test.passes()

test_regress/t/t_exprstmt_on_lhs_of_nba.v

@@ -0,0 +1,104 @@
+// 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
+
+module t(/*AUTOARG*/
+   // Outputs
+   data_o,
+   // Inputs
+   clk, rst_i, write_valid_i, write_front_i, read_valid_i, data_i
+   );
+
+   localparam NR_ELEMENTS = 16;
+   localparam DATAW = 32;
+
+   input clk;
+   input rst_i;
+   input write_valid_i;
+   input write_front_i;
+   input read_valid_i;
+   input [31:0] data_i;
+   output [31:0] data_o;
+
+   reg [31:0] FIFOContent [NR_ELEMENTS-1:0];
+
+   typedef logic [$clog2(NR_ELEMENTS)-1:0] FIFOPointer_t;
+
+   // verilator lint_off WIDTH
+   localparam FIFOPointer_t MAX_PTR_VAL = NR_ELEMENTS-1;
+   // verilator lint_on WIDTH
+   localparam FIFOPointer_t MIN_PTR_VAL = 0;
+   localparam FIFOPointer_t PTR_INC = 1;
+   FIFOPointer_t write_pointer;
+   FIFOPointer_t read_pointer;
+
+   function FIFOPointer_t nextPointer(input FIFOPointer_t val);
+      if ($clog2(NR_ELEMENTS) == $clog2(NR_ELEMENTS+1)
+          && val == MAX_PTR_VAL)
+        nextPointer = MIN_PTR_VAL; // explicit wrap if NR_ELEMENTS is not a power of 2
+      else
+        nextPointer = val + PTR_INC;
+   endfunction
+
+   function FIFOPointer_t prevPointer(input FIFOPointer_t val);
+      if ($clog2(NR_ELEMENTS) == $clog2(NR_ELEMENTS+1)
+          && val == MIN_PTR_VAL)
+        prevPointer = MAX_PTR_VAL; // explicit wrap if NR_ELEMENTS is not a power of 2
+      else
+        prevPointer = val - PTR_INC;
+   endfunction
+
+   reg [$clog2(NR_ELEMENTS)-1:0] level;
+   reg is_empty;
+
+   always @(posedge clk) begin
+      if (write_valid_i)
+        FIFOContent[write_front_i ? (read_valid_i ? read_pointer : prevPointer(read_pointer)) : write_pointer] <= data_i;
+   end
+
+   assign data_o = FIFOContent[read_pointer];
+
+   always @(posedge clk) begin
+      if (rst_i) begin
+         is_empty <= 1;
+      end
+      else if (write_valid_i) begin
+         is_empty <= 0;
+      end
+      else if (read_valid_i && write_pointer == nextPointer(read_pointer)) begin
+         is_empty <= 1;
+      end
+   end
+
+   always @(posedge clk) begin
+      if (rst_i) begin
+         level <= 0;
+      end
+      else begin
+         level <= level + (write_valid_i ? 1 : 0) - (read_valid_i ? 1 : 0);
+      end
+   end
+
+   always @(posedge clk) begin
+      if (rst_i) begin
+         write_pointer <= 0;
+      end
+      else if (write_valid_i && !write_front_i) begin
+         write_pointer <= nextPointer(write_pointer);
+      end
+   end
+
+   always @(posedge clk) begin
+      if (rst_i) begin
+         read_pointer <= 0;
+      end
+      else if (read_valid_i) begin
+         if (!(write_valid_i && write_front_i))read_pointer <= nextPointer(read_pointer);
+      end
+      else if (write_valid_i && write_front_i) begin
+         read_pointer <= prevPointer(read_pointer);
+      end
+   end
+endmodule