Internals: Do not emit temporaries for atomic assignments. (#6217)

Added test for a particularly convoluted case requiring fixup in
V3Premit. To help with statistics stability, also prevent V3Premit from
introducing temporaries for assignment where the RHS reads the LHS, but
the assignment is known to be atomic (by emitted C++ semantics).

Also rename `createWideTemp` to `createTemp`, as it is used for non-wide
expressions as well.

src/V3Premit.cpp

@@ -46,6 +46,7 @@ class PremitVisitor final : public VNVisitor {
 
     // STATE - across all visitors
     VDouble0 m_extractedToConstPool;  // Statistic tracking
+    VDouble0 m_temporaryVarsCreated;  // Statistic tracking
 
     // STATE - for current visit position (use VL_RESTORER)
     AstCFunc* m_cfuncp = nullptr;  // Current block
@@ -62,12 +63,12 @@ class PremitVisitor final : public VNVisitor {
         if (!nodep->isWide()) return;  // Not wide
         if (m_assignLhs) return;  // This is an lvalue!
         UASSERT_OBJ(!VN_IS(nodep->firstAbovep(), ArraySel), nodep, "Should have been ignored");
-        createWideTemp(nodep);
+        createTemp(nodep);
     }
 
-    AstVar* createWideTemp(AstNodeExpr* nodep) {
+    AstVar* createTemp(AstNodeExpr* nodep) {
         UASSERT_OBJ(m_stmtp, nodep, "Attempting to create temporary with no insertion point");
-        UINFO(4, "createWideTemp: " << nodep);
+        UINFO(4, "createTemp: " << nodep);
 
         VNRelinker relinker;
         nodep->unlinkFrBack(&relinker);
@@ -93,6 +94,7 @@ class PremitVisitor final : public VNVisitor {
             const std::string name = "__Vtemp_" + std::to_string(++m_tmpVarCnt);
             varp = new AstVar{flp, VVarType::STMTTEMP, name, nodep->dtypep()};
             m_cfuncp->addInitsp(varp);
+            ++m_temporaryVarsCreated;
 
             // Put assignment before the referencing statement
             assignp = new AstAssign{flp, new AstVarRef{flp, varp, VAccess::WRITE}, nodep};
@@ -221,9 +223,10 @@ class PremitVisitor final : public VNVisitor {
             }
         }
 
-        if (rhsReadsLhs(nodep)) {
-            // Need to do this even if not wide, as e.g. a select may be on a wide operator
-            createWideTemp(nodep->rhsp());
+        // If the RHS reads the LHS, we need a temporary unless the update is atomic
+        const bool isAtomic = VN_IS(nodep->lhsp(), VarRef) && !nodep->lhsp()->isWide();
+        if (!isAtomic && rhsReadsLhs(nodep)) {
+            createTemp(nodep->rhsp());
         } else {
             iterateAndNextNull(nodep->rhsp());
         }
@@ -290,7 +293,7 @@ class PremitVisitor final : public VNVisitor {
     void visit(AstCvtPackedToArray* nodep) override {
         iterateChildren(nodep);
         checkNode(nodep);
-        if (!VN_IS(nodep->backp(), NodeAssign)) createWideTemp(nodep);
+        if (!VN_IS(nodep->backp(), NodeAssign)) createTemp(nodep);
     }
     void visit(AstCvtUnpackedToQueue* nodep) override {
         iterateChildren(nodep);
@@ -330,7 +333,7 @@ class PremitVisitor final : public VNVisitor {
             && !VN_IS(nodep->condp(), VarRef)) {
             // We're going to need the expression several times in the expanded code,
             // so might as well make it a common expression
-            createWideTemp(nodep->condp());
+            createTemp(nodep->condp());
             VIsCached::clearCacheTree();
         }
         checkNode(nodep);
@@ -342,7 +345,7 @@ class PremitVisitor final : public VNVisitor {
         for (AstNodeExpr *expp = nodep->exprsp(), *nextp; expp; expp = nextp) {
             nextp = VN_AS(expp->nextp(), NodeExpr);
             if (expp->isString() && !VN_IS(expp, VarRef)) {
-                AstVar* const varp = createWideTemp(expp);
+                AstVar* const varp = createTemp(expp);
                 // Do not remove VarRefs to this in V3Const
                 varp->noSubst(true);
             }
@@ -360,6 +363,8 @@ public:
     ~PremitVisitor() override {
         V3Stats::addStat("Optimizations, Prelim extracted value to ConstPool",
                          m_extractedToConstPool);
+        V3Stats::addStat("Optimizations, Prelim temporary variables created",
+                         m_temporaryVarsCreated);
     }
 };
 

test_regress/t/t_wide_temp_while_cond.cpp

@@ -0,0 +1,12 @@
+// -*- mode: C++; c-file-style: "cc-mode" -*-
+//
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// Copyright 2025 by Geza Lore. 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
+//*************************************************************************
+
+extern "C" int identity(int x) { return x; }

test_regress/t/t_wide_temp_while_cond.py

@@ -0,0 +1,22 @@
+#!/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=["--exe", "--main", "--timing", "--stats", "t/t_wide_temp_while_cond.cpp"])
+
+if test.vlt or test.vltmt:
+    test.file_grep(test.stats, r'Optimizations, Prelim temporary variables created\s+(\d+)', 3)
+
+test.execute()
+
+test.passes()

test_regress/t/t_wide_temp_while_cond.v

@@ -0,0 +1,26 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2025 by Geza Lore.
+// SPDX-License-Identifier: CC0-1.0
+
+import "DPI-C" pure function int identity(input int value);
+
+module t;
+   initial begin
+      int n = 0;
+      logic [127:0] val = 128'b1;
+      logic [15:0] one = 16'b1;
+
+      // This condition involves multiple wide temporaries, and an over-width
+      // shift, all of which requires V3Premit to fix up.
+      while (|((val[ 7'(one >> identity(32)) +: 96] << n) >> n)) begin
+        ++n;
+      end
+
+      $display("n=%0d", n);
+      if (n != 96) $stop;
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule