Add --unroll-limit option (#6654) (#6668)

bin/verilator

@@ -517,6 +517,7 @@ detailed descriptions of these arguments.
      -U<var>                    Undefine preprocessor define
     --no-unlimited-stack        Don't disable stack size limit
     --unroll-count <loops>      Tune maximum loop iterations
+    --unroll-limit <loops>      Maximum loop iterations before assuming infinite loop
     --unroll-stmts <stmts>      Tune maximum loop body size
     --unused-regexp <regexp>    Tune UNUSED lint signals
      -V                         Verbose version and config

docs/guide/exe_verilator.rst

@@ -1808,18 +1808,34 @@ Summary:
 
 .. option:: --unroll-count <loops>
 
-   Rarely needed.  Specifies the maximum number of loop iterations that may
-   be unrolled.  See also :option:`BLKLOOPINIT` warning, and
-   :option:`/*verilator&32;unroll_disable*/` and
-   :option:`/*verilator&32;unroll_full*/` metacomments.
+   Rarely needed.  Specifies the maximum number of iterations for procedural
+   loops that may be unrolled. If the loop has more iterations, it will not be
+   unrolled at all. Does not effect generate loops.  See also
+   :option:`BLKLOOPINIT` warning, and :option:`/*verilator&32;unroll_disable*/`
+   and :option:`/*verilator&32;unroll_full*/` metacomments.
+
+   Defaults to 64. Setting to 0 disables all unrolling of procedural loops
+   except those marked with :option:`/*verilator&32;unroll_full*/`.
+
+.. option:: --unroll-limit <loops>
+
+   Rarely needed.  Specifies the maximum number of unrolled loop iterations
+   before assuming the loop is infinite and subsequently issuing an error.
+   This is a safety limit to make sure Verialtor terminates even in the
+   of a true infinite loop in the input (e.g.: due to a generate for loop
+   missing the increment statement)
+
+   Defaults to 16K, can be increased if the input has larger finite loops.
 
 .. option:: --unroll-stmts <statements>
 
-   Rarely needed.  Specifies the maximum number of statements in a loop for
-   that loop to be unrolled.  See also :option:`BLKLOOPINIT` warning, and
-   :option:`/*verilator&32;unroll_disable*/` and
+   Rarely needed.  Specifies the maximum number of statements in the unrolling
+   of a loop for that loop to be unrolled.  See also :option:`BLKLOOPINIT`
+   warning, and :option:`/*verilator&32;unroll_disable*/` and
    :option:`/*verilator&32;unroll_full*/` metacomments.
 
+   Defaults to 30000.
+
 .. option:: --unused-regexp <regexp>
 
    Rarely needed.  Specifies a simple regexp with \* and ? that, if a signal

src/V3Options.cpp

@@ -1110,16 +1110,6 @@ VTimescale V3Options::timeComputeUnit(const VTimescale& flag) const {
     }
 }
 
-int V3Options::unrollCountAdjusted(const VOptionBool& full, bool generate, bool simulate) {
-    int count = unrollCount();
-    // std::max to avoid rollover if unrollCount is e.g. std::numeric_limits<int>::max()
-    // With /*verilator unroll_full*/ still have a limit to avoid infinite loops
-    if (full.isSetTrue()) count = std::max(count, count * 1024);
-    if (generate) count = std::max(count, count * 16);
-    if (simulate) count = std::max(count, count * 16);
-    return count;
-}
-
 //######################################################################
 // V3 Options utilities
 
@@ -1793,6 +1783,7 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc,
     DECL_OPTION("-underline-zero", OnOff, &m_underlineZero).undocumented();  // Deprecated
     DECL_OPTION("-no-unlimited-stack", CbCall, []() {});  // Processed only in bin/verilator shell
     DECL_OPTION("-unroll-count", Set, &m_unrollCount).undocumented();  // Optimization tweak
+    DECL_OPTION("-unroll-limit", Set, &m_unrollLimit);
     DECL_OPTION("-unroll-stmts", Set, &m_unrollStmts).undocumented();  // Optimization tweak
     DECL_OPTION("-unused-regexp", Set, &m_unusedRegexp);
 

src/V3Options.h

@@ -347,6 +347,7 @@ private:
     int         m_traceMaxWidth = 4096; // main switch: --trace-max-width
     int         m_traceThreads = 0; // main switch: --trace-threads
     int         m_unrollCount = 64;  // main switch: --unroll-count
+    int         m_unrollLimit = 16384;  // main switch: --unroll-limit
     int         m_unrollStmts = 30000;  // main switch: --unroll-stmts
     int         m_verilateJobs = -1;  // main switch: --verilate-jobs
 
@@ -626,7 +627,7 @@ public:
     }
     bool useFstWriterThread() const { return traceThreads() && traceEnabledFst(); }
     int unrollCount() const { return m_unrollCount; }
-    int unrollCountAdjusted(const VOptionBool& full, bool generate, bool simulate);
+    int unrollLimit() const { return m_unrollLimit; }
     int unrollStmts() const { return m_unrollStmts; }
     int verilateJobs() const { return m_verilateJobs; }
 

src/V3Simulate.h

@@ -1086,20 +1086,20 @@ private:
         }
         if (!optimizable()) return;
 
-        int loops = 0;
+        size_t iterCount = 0;
+        const size_t iterLimit = v3Global.opt.unrollLimit();
         while (true) {
+            if (iterCount > iterLimit) {
+                clearOptimizable(nodep, "Loop simulation took too long; probably this is an "
+                                        "infinite loop, otherwise set '--unroll-limit' above "
+                                            + std::to_string(iterLimit));
+                break;
+            }
+            ++iterCount;
+
             UINFO(5, "    LOOP-ITER " << nodep);
             iterateAndNextConstNull(nodep->stmtsp());
             if (jumpingOver()) break;
-
-            // Prep for next loop
-            if (loops++ > v3Global.opt.unrollCountAdjusted(nodep->unroll(), m_params, true)) {
-                clearOptimizable(nodep, "Loop unrolling took too long; probably this is an"
-                                        "infinite loop, or use /*verilator unroll_full*/, or "
-                                        "set --unroll-count above "
-                                            + cvtToStr(loops));
-                break;
-            }
         }
 
         if (m_jumptargetp == nodep) {

src/V3Unroll.cpp

@@ -116,7 +116,7 @@ class UnrollOneVisitor final : VNVisitor {
             // Add statements to unrolled body
             while (AstNode* const nodep = m_wrapp->stmtsp()) {
                 // Check if we reached the size limit, unless full unrolling is requested
-                if (!m_loopp->unroll().isSetTrue()) {
+                if (!m_unrollFull) {
                     m_unrolledSize += nodep->nodeCount();
                     if (m_unrolledSize > static_cast<size_t>(v3Global.opt.unrollStmts())) {
                         cantUnroll(m_loopp, m_stats.m_nFailUnrollStmts);
@@ -256,14 +256,18 @@ class UnrollOneVisitor final : VNVisitor {
             lhsp->varScopep()->user1p(valp);
         }
         // Attempt to unroll the loop
-        const size_t iterLimit = v3Global.opt.unrollCountAdjusted(loopp->unroll(), false, false);
+        const size_t iterLimit
+            = m_unrollFull ? v3Global.opt.unrollLimit() : v3Global.opt.unrollCount();
         size_t iterCount = 0;
         do {
-            // Allow iterLimit + 1 iterations, which is consistent with the old behaviour
-            // where 'do' loops used to be unrolled at least once, and while/for loops are
-            // tested at the front on the last entry to the loop body
             if (iterCount > iterLimit) {
                 cantUnroll(m_loopp, m_stats.m_nFailUnrollCount);
+                if (m_unrollFull) {
+                    loopp->v3error("Unrolling procedural loop with '/* verilator unroll_full */' "
+                                   "took too long; probably this is an infinite loop, otherwise "
+                                   "set '--unroll-limit' above "
+                                   << iterLimit);
+                }
                 return;
             }
             ++iterCount;

src/V3UnrollGen.cpp

@@ -190,7 +190,8 @@ class UnrollGenVisitor final : public VNVisitor {
         AstNode* newbodysp = nullptr;
         if (stmtsp) {
             pushDeletep(stmtsp);  // Always cloned below.
-            int times = 0;
+            size_t iterCount = 0;
+            const size_t iterLimit = v3Global.opt.unrollLimit();
             while (true) {
                 UINFO(8, "      Looping " << loopValue);
                 V3Number res{nodep};
@@ -201,6 +202,13 @@ class UnrollGenVisitor final : public VNVisitor {
                 if (!res.isEqOne()) {
                     break;  // Done with the loop
                 } else {
+                    if (++iterCount > iterLimit) {
+                        nodep->v3error("Unrolling generate loop took too long; probably this is "
+                                       "an infinite loop, otherwise set '--unroll-limit' above "
+                                       << iterLimit);
+                        break;
+                    }
+
                     // Replace iterator values with constant
                     AstNode* oneloopp = stmtsp->cloneTree(true);
                     AstConst* varValuep = new AstConst{nodep->fileline(), loopValue};
@@ -222,16 +230,6 @@ class UnrollGenVisitor final : public VNVisitor {
                         newbodysp = oneloopp;
                     }
 
-                    const int limit = v3Global.opt.unrollCountAdjusted(VOptionBool{}, true, false);
-                    if (++times / 3 > limit) {
-                        nodep->v3error(
-                            "Loop unrolling took too long;"
-                            " probably this is an infinite loop, "
-                            " or use /*verilator unroll_full*/, or set --unroll-count above "
-                            << times);
-                        break;
-                    }
-
                     // loopValue += valInc
                     AstAssign* const incpass = VN_AS(incp, Assign);
                     V3Number newLoopValue{nodep};

test_regress/t/t_flag_unroll_limit_const.out

@@ -0,0 +1,9 @@
+%Error: t/t_flag_unroll_limit_const.v:17:16: Expecting expression to be constant, but can't determine constant for FUNCREF 'f'
+                                           : ... note: In instance 't'
+        t/t_flag_unroll_limit_const.v:9:3: ... Location of non-constant LOOP: Loop simulation took too long; probably this is an infinite loop, otherwise set '--unroll-limit' above 4
+        t/t_flag_unroll_limit_const.v:17:16: ... Called from 'f()' with parameters:
+           x = ?32?h5
+   17 |   output logic[f(5):0] o5    
+      |                ^
+        ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance.
+%Error: Exiting due to

test_regress/t/t_flag_unroll_limit_const.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('vlt')
+
+test.compile(verilator_flags2=['--unroll-count 0 --unroll-limit 4'],
+             fails=True,
+             expect_filename=test.golden_filename)
+
+test.passes()

test_regress/t/t_flag_unroll_limit_const.v

@@ -0,0 +1,20 @@
+// 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
+
+function automatic int f(int x);
+  int n;
+  for (int i = 0; i < x; ++i) begin
+    ++n;
+  end
+  return n;
+endfunction
+
+module t(
+  output logic[f(4):0] o4,  // Should simulate
+  output logic[f(5):0] o5   // Should NOT simulate
+);
+
+endmodule

test_regress/t/t_flag_unroll_limit_gen.out

@@ -0,0 +1,6 @@
+%Error: t/t_flag_unroll_limit_gen.v:13:3: Unrolling generate loop took too long; probably this is an infinite loop, otherwise set '--unroll-limit' above 4
+                                        : ... note: In instance 't'
+   13 |   for (genvar i = 0; i < 5; ++i) begin
+      |   ^~~
+        ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance.
+%Error: Exiting due to

test_regress/t/t_flag_unroll_limit_gen.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('vlt')
+
+test.compile(verilator_flags2=['--unroll-count 0 --unroll-limit 4'],
+             fails=True,
+             expect_filename=test.golden_filename)
+
+test.passes()

test_regress/t/t_flag_unroll_limit_gen.v

@@ -0,0 +1,17 @@
+// 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;
+
+  for (genvar i = 0; i < 4; ++i) begin
+    initial $display("Should unroll: %0d", i);
+  end
+
+  for (genvar i = 0; i < 5; ++i) begin
+    initial $display("Should NOT unroll: %0d", i);
+  end
+
+endmodule

test_regress/t/t_flag_unroll_limit_stmt.out

@@ -0,0 +1,5 @@
+%Error: t/t_flag_unroll_limit_stmt.v:16:5: Unrolling procedural loop with '/* verilator unroll_full */' took too long; probably this is an infinite loop, otherwise set '--unroll-limit' above 4
+   16 |     for (int i = 0; i < 5; ++i) begin
+      |     ^~~
+        ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance.
+%Error: Exiting due to

test_regress/t/t_flag_unroll_limit_stmt.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('vlt')
+
+test.compile(verilator_flags2=['--unroll-count 0 --unroll-limit 4'],
+             fails=True,
+             expect_filename=test.golden_filename)
+
+test.passes()

test_regress/t/t_flag_unroll_limit_stmt.v

@@ -0,0 +1,21 @@
+// 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;
+
+  initial begin
+    // verilator unroll_full
+    for (int i = 0; i < 4; ++i) begin
+      $display("Should unroll: %0d", i);
+    end
+
+    // verilator unroll_full
+    for (int i = 0; i < 5; ++i) begin
+      $display("Should NOT unroll: %0d", i);
+    end
+  end
+
+endmodule

test_regress/t/t_func_const_bad.out

@@ -20,7 +20,7 @@
       |                   ^~~~~~~~~~~~~~
 %Error: t/t_func_const_bad.v:36:19: Expecting expression to be constant, but can't determine constant for FUNCREF 'f_bad_infinite'
                                   : ... note: In instance 't'
-        t/t_func_const_bad.v:38:5: ... Location of non-constant LOOP: Loop unrolling took too long; probably this is aninfinite loop, or use /*verilator unroll_full*/, or set --unroll-count above 16386
+        t/t_func_const_bad.v:38:5: ... Location of non-constant LOOP: Loop simulation took too long; probably this is an infinite loop, otherwise set '--unroll-limit' above 16384
         t/t_func_const_bad.v:36:19: ... Called from 'f_bad_infinite()' with parameters:
            a = ?32?h3
    36 |   localparam B4 = f_bad_infinite(3);

test_regress/t/t_unroll_pragma.v

@@ -22,11 +22,11 @@ module t;
    end
 
    initial begin
-      // Test a loop smaller than --unroll-count
+      // Test a loop equal to --unroll-count - should unroll without pragma
       `PRAGMA
-      for (i = 0; i < 2; ++i) begin
+      for (i = 0; i < 4; ++i) begin
          `PRAGMA
-         for (j = 0; j < 2; ++j) begin
+         for (j = 0; j < 4; ++j) begin
             $c("small();");
          end
       end

test_regress/t/t_unroll_pragma_full.py

@@ -17,7 +17,7 @@ test.compile(verilator_flags2=['--unroll-count 4 --unroll-stmts 9999 --stats -DT
              make_top_shell=False,
              make_main=False)
 
-test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 9)
-test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 45)
+test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 11)
+test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 61)
 
 test.passes()

test_regress/t/t_unroll_pragma_none.py

@@ -17,8 +17,8 @@ test.compile(verilator_flags2=['--unroll-count 4 --unroll-stmts 9999 --stats -DT
              make_top_shell=False,
              make_main=False)
 
-test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 3)
-test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 9)
+test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 5)
+test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 25)
 test.file_grep(test.stats,
                r'Optimizations, Loop unrolling, Failed - reached --unroll-count\s+(\d+)', 2)