Fix ++/-- under statements (#4399)

src/V3LinkInc.cpp

@@ -61,13 +61,30 @@ private:
 
     // STATE
     AstNodeFTask* m_ftaskp = nullptr;  // Function or task we're inside
+    AstNodeModule* m_modp = nullptr;  // Module we're inside
     int m_modIncrementsNum = 0;  // Var name counter
     InsertMode m_insMode = IM_BEFORE;  // How to insert
     AstNode* m_insStmtp = nullptr;  // Where to insert statement
     bool m_unsupportedHere = false;  // Used to detect where it's not supported yet
 
     // METHODS
-    void insertBeforeStmt(AstNode* nodep, AstNode* newp) {
+    void insertOnTop(AstNode* newp) {
+        // Add the thing directly under the current TFunc/Module
+        AstNode* stmtsp = nullptr;
+        if (m_ftaskp) {
+            stmtsp = m_ftaskp->stmtsp();
+        } else if (m_modp) {
+            stmtsp = m_modp->stmtsp();
+        }
+        UASSERT(stmtsp, "Variable not under FTASK/MODULE");
+        newp->addNext(stmtsp->unlinkFrBackWithNext());
+        if (m_ftaskp) {
+            m_ftaskp->addStmtsp(newp);
+        } else if (m_modp) {
+            m_modp->addStmtsp(newp);
+        }
+    }
+    void insertNextToStmt(AstNode* nodep, AstNode* newp) {
         // Return node that must be visited, if any
         if (debug() >= 9) newp->dumpTree("-  newstmt: ");
         UASSERT_OBJ(m_insStmtp, nodep, "Function not underneath a statement");
@@ -88,7 +105,9 @@ private:
 
     // VISITORS
     void visit(AstNodeModule* nodep) override {
+        VL_RESTORER(m_modp);
         VL_RESTORER(m_modIncrementsNum);
+        m_modp = nodep;
         m_modIncrementsNum = 0;
         iterateChildren(nodep);
     }
@@ -248,11 +267,10 @@ private:
 
         AstConst* const constp = VN_AS(nodep->lhsp(), Const);
         UASSERT_OBJ(nodep, constp, "Expecting CONST");
-        const AstNode* const backp = nodep->backp();
         AstConst* const newconstp = constp->cloneTree(true);
 
         // Prepare a temporary variable
-        FileLine* const fl = backp->fileline();
+        FileLine* const fl = nodep->fileline();
         const string name = string{"__Vincrement"} + cvtToStr(++m_modIncrementsNum);
         AstVar* const varp = new AstVar{
             fl, VVarType::BLOCKTEMP, name, VFlagChildDType{},
@@ -260,7 +278,7 @@ private:
         if (m_ftaskp) varp->funcLocal(true);
 
         // Declare the variable
-        insertBeforeStmt(nodep, varp);
+        insertOnTop(varp);
 
         // Define what operation will we be doing
         AstNodeExpr* operp;
@@ -273,17 +291,20 @@ private:
         if (VN_IS(nodep, PreAdd) || VN_IS(nodep, PreSub)) {
             // PreAdd/PreSub operations
             // Immediately after declaration - increment it by one
-            varp->addNextHere(new AstAssign{fl, writep->cloneTree(true),
+            AstAssign* const assignp
-                                            new AstVarRef{fl, varp, VAccess::READ}});
+                = new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, operp};
+            insertNextToStmt(nodep, assignp);
             // Immediately after incrementing - assign it to the original variable
-            varp->addNextHere(new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, operp});
+            assignp->addNextHere(new AstAssign{fl, writep->cloneTree(true),
+                                               new AstVarRef{fl, varp, VAccess::READ}});
         } else {
             // PostAdd/PostSub operations
-            // assign the original variable to the temporary one
+            // Assign the original variable to the temporary one
-            varp->addNextHere(new AstAssign{fl, writep->cloneTree(true), operp});
+            AstAssign* const assignp = new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE},
+                                                     readp->cloneTree(true)};
+            insertNextToStmt(nodep, assignp);
             // Increment the original variable by one
-            varp->addNextHere(new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE},
+            assignp->addNextHere(new AstAssign{fl, writep->cloneTree(true), operp});
-                                            readp->cloneTree(true)});
         }
 
         // Replace the node with the temporary

test_regress/t/t_inc_relink.pl

@@ -0,0 +1,18 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2023 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(simulator => 1);
+
+compile(
+    verilator_flags2 => ["-Wno-WIDTHTRUNC"],
+    );
+
+ok(1);
+1;

test_regress/t/t_inc_relink.v

@@ -0,0 +1,37 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2023 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+
+// Test if temporary vars are relinked if not used directly under FTASK.
+
+package A;  // Create JUMPBLOCK; use n++ in it
+   task t;
+      automatic int n;
+      if ($random) return;
+      n = n++;
+   endtask
+endpackage
+
+package B;  // Create IF; use n++ in it
+   int n;
+   task t;
+      if ($random) n = n++;
+   endtask
+endpackage
+
+module C;  // Like above but in a module
+   int n = 0;
+
+   initial if ($random) n = n++;
+endmodule
+
+module t;  // Actually use those to test relinking
+   C c;
+
+   initial begin
+      A::t();
+      B::t();
+   end
+endmodule