diff --git a/bin/verilator b/bin/verilator index 8da3c0ddc..67650dcd1 100755 --- a/bin/verilator +++ b/bin/verilator @@ -517,6 +517,7 @@ detailed descriptions of these arguments. -U Undefine preprocessor define --no-unlimited-stack Don't disable stack size limit --unroll-count Tune maximum loop iterations + --unroll-limit Maximum loop iterations before assuming infinite loop --unroll-stmts Tune maximum loop body size --unused-regexp Tune UNUSED lint signals -V Verbose version and config diff --git a/docs/guide/exe_verilator.rst b/docs/guide/exe_verilator.rst index 4df0a1e1a..884055f20 100644 --- a/docs/guide/exe_verilator.rst +++ b/docs/guide/exe_verilator.rst @@ -1808,18 +1808,34 @@ Summary: .. option:: --unroll-count - 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 + + 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 - 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 Rarely needed. Specifies a simple regexp with \* and ? that, if a signal diff --git a/src/V3Options.cpp b/src/V3Options.cpp index b22fb7c80..8d7aeeab3 100644 --- a/src/V3Options.cpp +++ b/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::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); diff --git a/src/V3Options.h b/src/V3Options.h index 8bd742a48..db1506dba 100644 --- a/src/V3Options.h +++ b/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; } diff --git a/src/V3Simulate.h b/src/V3Simulate.h index fd19fe21d..b48eedb22 100644 --- a/src/V3Simulate.h +++ b/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) { diff --git a/src/V3Unroll.cpp b/src/V3Unroll.cpp index c3fb046ed..1011e70f4 100644 --- a/src/V3Unroll.cpp +++ b/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(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; diff --git a/src/V3UnrollGen.cpp b/src/V3UnrollGen.cpp index be0d19f9a..88477afe6 100644 --- a/src/V3UnrollGen.cpp +++ b/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}; diff --git a/test_regress/t/t_flag_unroll_limit_const.out b/test_regress/t/t_flag_unroll_limit_const.out new file mode 100644 index 000000000..86f4a5bed --- /dev/null +++ b/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 diff --git a/test_regress/t/t_flag_unroll_limit_const.py b/test_regress/t/t_flag_unroll_limit_const.py new file mode 100755 index 000000000..ce5bb61c7 --- /dev/null +++ b/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() diff --git a/test_regress/t/t_flag_unroll_limit_const.v b/test_regress/t/t_flag_unroll_limit_const.v new file mode 100644 index 000000000..56a70124a --- /dev/null +++ b/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 diff --git a/test_regress/t/t_flag_unroll_limit_gen.out b/test_regress/t/t_flag_unroll_limit_gen.out new file mode 100644 index 000000000..38b0ca018 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_flag_unroll_limit_gen.py b/test_regress/t/t_flag_unroll_limit_gen.py new file mode 100755 index 000000000..ce5bb61c7 --- /dev/null +++ b/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() diff --git a/test_regress/t/t_flag_unroll_limit_gen.v b/test_regress/t/t_flag_unroll_limit_gen.v new file mode 100644 index 000000000..04bb741d2 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_flag_unroll_limit_stmt.out b/test_regress/t/t_flag_unroll_limit_stmt.out new file mode 100644 index 000000000..f91501437 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_flag_unroll_limit_stmt.py b/test_regress/t/t_flag_unroll_limit_stmt.py new file mode 100755 index 000000000..ce5bb61c7 --- /dev/null +++ b/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() diff --git a/test_regress/t/t_flag_unroll_limit_stmt.v b/test_regress/t/t_flag_unroll_limit_stmt.v new file mode 100644 index 000000000..1eec5e37f --- /dev/null +++ b/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 diff --git a/test_regress/t/t_func_const_bad.out b/test_regress/t/t_func_const_bad.out index d1d52f71d..eafb65ea3 100644 --- a/test_regress/t/t_func_const_bad.out +++ b/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); diff --git a/test_regress/t/t_unroll_pragma.v b/test_regress/t/t_unroll_pragma.v index ac8381298..52bd062b0 100644 --- a/test_regress/t/t_unroll_pragma.v +++ b/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 diff --git a/test_regress/t/t_unroll_pragma_full.py b/test_regress/t/t_unroll_pragma_full.py index 2c836b347..653d6e8dd 100755 --- a/test_regress/t/t_unroll_pragma_full.py +++ b/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() diff --git a/test_regress/t/t_unroll_pragma_none.py b/test_regress/t/t_unroll_pragma_none.py index d1393ef61..6ba506a65 100755 --- a/test_regress/t/t_unroll_pragma_none.py +++ b/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)