From e53d6d9006a75c2632da0e25900d275f652b167a Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 4 Jun 2026 14:57:36 +0100 Subject: [PATCH] Improve procedural loop unrolling - Enable unrolling of nested loops when the inner loop updates the outer loop condition - Enable unrolling 'for' loops with break statements --- src/V3Unroll.cpp | 153 +++++++++++++++++++------ test_regress/t/t_unroll_pragma_none.py | 2 +- test_regress/t/t_unroll_stmt.out | 29 +++++ test_regress/t/t_unroll_stmt.py | 4 +- test_regress/t/t_unroll_stmt.v | 18 +++ 5 files changed, 169 insertions(+), 37 deletions(-) diff --git a/src/V3Unroll.cpp b/src/V3Unroll.cpp index d8dd4221e..ce677324b 100644 --- a/src/V3Unroll.cpp +++ b/src/V3Unroll.cpp @@ -64,16 +64,69 @@ struct UnrollStats final { Stat m_nUnrolledIters{"Unrolled iterations"}; }; +//###################################################################### +// Variable bindings +class UnrolllBindings final { + // NODE STATE + // AstVarScope::user1() int: index of the binding in m_bindings + const VNUser1InUse m_inuser1; + + // STATE + // Map from AstVarScope::user1() to current variable value, nullptr if not bound - idx 0 unused + std::vector m_curr{nullptr}; + // Stack of binding checkpoints + std::vector> m_checkpoints; + +public: + // CONSTRUCTOR + UnrolllBindings() = default; + ~UnrolllBindings() = default; + VL_UNCOPYABLE(UnrolllBindings); + VL_UNMOVABLE(UnrolllBindings); + + // METHODS + void clear() { + m_curr.resize(1); + m_checkpoints.clear(); + AstNode::user1ClearTree(); + } + + void checkpoint() { m_checkpoints.push_back(m_curr); } + + void revert() { + m_curr = std::move(m_checkpoints.back()); + m_checkpoints.pop_back(); + } + + void commit() { m_checkpoints.pop_back(); } + + void set(AstVarScope* vscp, AstConst* valp) { + if (!vscp->user1()) { + vscp->user1(m_curr.size()); + m_curr.push_back(nullptr); + } + UINFO(6, "Binding SET " << vscp->name() << " / " << vscp->user1() << " := " << valp); + m_curr[vscp->user1()] = valp; + } + + AstConst* get(AstVarScope* vscp) { + // It's possible after a revert that user1 is set, but the vector is shorter, pad up + if (static_cast(vscp->user1()) >= m_curr.size()) { + m_curr.resize(vscp->user1() + 1, nullptr); + } + AstConst* const valp = vscp->user1() ? m_curr[vscp->user1()] : nullptr; + UINFO(6, "Binding GET " << vscp->name() << " / " << vscp->user1() << " == " << valp); + return valp; + } +}; + //###################################################################### // Unroll one AstLoop class UnrollOneVisitor final : VNVisitor { - // NODE STATE - // AstVarScope::user1p() AstConst: Value of this AstVarScope - const VNUser1InUse m_user1InUse; - // STATE UnrollStats& m_stats; // Statistics tracking + UnrolllBindings& m_bindings; // Variable bindings AstLoop* const m_loopp; // The loop we are trying to unroll AstNode* m_stmtsp = nullptr; // Resulting unrolled statement size_t m_unrolledSize = 0; // Number of nodes in unrolled loop @@ -204,11 +257,11 @@ class UnrollOneVisitor final : VNVisitor { AstVarRef* const refp = VN_CAST(np, VarRef); if (!refp) return; // Ignore if the referenced variable has no binding - AstConst* const valp = VN_AS(refp->varScopep()->user1p(), Const); + AstConst* const valp = m_bindings.get(refp->varScopep()); if (!valp) return; // If writen, remove the binding if (refp->access().isWriteOrRW()) { - refp->varScopep()->user1p(nullptr); + m_bindings.set(refp->varScopep(), nullptr); return; } // Otherwise add it to the list of variables to substitute @@ -219,7 +272,7 @@ class UnrollOneVisitor final : VNVisitor { // Actually substitute the variables that still have bindings for (AstVarRef* const refp : toSubstitute) { // Pick up bound value - AstConst* const valp = VN_AS(refp->varScopep()->user1p(), Const); + AstConst* const valp = m_bindings.get(refp->varScopep()); // Binding might have been removed after adding to 'toSubstitute' if (!valp) continue; // Substitute it @@ -230,8 +283,9 @@ class UnrollOneVisitor final : VNVisitor { } // CONSTRUCTOR - UnrollOneVisitor(UnrollStats& stats, AstLoop* loopp) + UnrollOneVisitor(UnrollStats& stats, UnrolllBindings& bindings, AstLoop* loopp) : m_stats{stats} + , m_bindings{bindings} , m_loopp{loopp} { UASSERT_OBJ(!loopp->contsp(), loopp, "'contsp' only used before LinkJump"); // Do not unroll if we are told not to @@ -239,25 +293,9 @@ class UnrollOneVisitor final : VNVisitor { cantUnroll(loopp, m_stats.m_nPragmaDisabled); return; } - // Gather variable bindings from the preceding statements - for (AstNode *succp = loopp, *currp = loopp->backp(); currp->nextp() == succp; - succp = currp, currp = currp->backp()) { - AstAssign* const assignp = VN_CAST(currp, Assign); - if (!assignp) break; - AstConst* const valp = VN_CAST(V3Const::constifyEdit(assignp->rhsp()), Const); - if (!valp) break; - AstVarRef* const lhsp = VN_CAST(assignp->lhsp(), VarRef); - if (!lhsp) break; - // Don't bind if volatile - if (lhsp->varp()->isForced() || lhsp->varp()->isSigUserRWPublic()) continue; - // Don't overwrite a later binding - if (lhsp->varScopep()->user1p()) continue; - // Set up the binding - lhsp->varScopep()->user1p(valp); - } // Attempt to unroll the loop - const size_t iterLimit - = m_unrollFull ? v3Global.opt.unrollLimit() : v3Global.opt.unrollCount(); + const size_t iterLimit = m_unrollFull ? v3Global.opt.unrollLimit() // + : v3Global.opt.unrollCount(); size_t iterCount = 0; do { if (iterCount > iterLimit) { @@ -316,7 +354,7 @@ class UnrollOneVisitor final : VNVisitor { AstVarRef* const lhsp = VN_CAST(nodep->lhsp(), VarRef); AstConst* const valp = VN_CAST(V3Const::constifyEdit(nodep->rhsp()), Const); if (lhsp && valp && !lhsp->varp()->isForced() && !lhsp->varp()->isSigUserRWPublic()) { - lhsp->varScopep()->user1p(valp); + m_bindings.set(lhsp->varScopep(), valp); return; } // Otherwise just like a generic statement @@ -340,12 +378,32 @@ class UnrollOneVisitor final : VNVisitor { // Remove trailing dead code if (nodep->nextp()) pushDeletep(nodep->nextp()->unlinkFrBackWithNext()); } + void visit(AstLoop* nodep) override { + m_bindings.checkpoint(); + std::pair pair = UnrollOneVisitor::apply(m_stats, m_bindings, nodep); + + // If failed, revert the bindings and process the loop as a generic statement + if (!pair.second) { + m_bindings.revert(); + process(nodep); + return; + } + + // Commit the bindings and replace the loop with the unrolled code + m_bindings.commit(); + if (pair.first) { + nodep->replaceWith(pair.first); + } else { + nodep->unlinkFrBack(); + } + VL_DO_DANGLING(pushDeletep(nodep), nodep); + } public: - // Unroll the given loop. Returns the resulting statements and the number of - // iterations unrolled (0 if unrolling failed); - static std::pair apply(UnrollStats& stats, AstLoop* loopp) { - UnrollOneVisitor visitor{stats, loopp}; + // Unroll the given loop. Returns the resulting statements and a flag indicating success + static std::pair apply(UnrollStats& stats, UnrolllBindings& bindings, + AstLoop* loopp) { + UnrollOneVisitor visitor{stats, bindings, loopp}; // If successfully unrolled, return the resulting list of statements - might be empty if (visitor.m_ok) return {visitor.m_stmtsp, true}; // Otherwise delete intermediate results @@ -358,13 +416,40 @@ public: // Unroll all AstLoop statements class UnrollAllVisitor final : VNVisitor { - // STATE - Statistic tracking - UnrollStats m_stats; + // STATE + UnrollStats m_stats; // Statistic tracking + UnrolllBindings m_bindings; // Variable bindings // VISIT void visit(AstLoop* nodep) override { + // Gather variable bindings from the preceding statements + m_bindings.clear(); + for (AstNode *succp = nodep, *currp = nodep->backp(); true; + succp = currp, currp = currp->backp()) { + // If the current statement is in higher list, proceed carefully + if (currp->nextp() != succp) { + // Jump block is OK, there is only one way to get here + if (VN_IS(currp, JumpBlock)) continue; + // TODO: if? + // Otherwise we dont' know the control flow, give up + break; + } + AstAssign* const assignp = VN_CAST(currp, Assign); + if (!assignp) break; + AstConst* const valp = VN_CAST(V3Const::constifyEdit(assignp->rhsp()), Const); + if (!valp) break; + AstVarRef* const lhsp = VN_CAST(assignp->lhsp(), VarRef); + if (!lhsp) break; + // Don't bind if volatile + if (lhsp->varp()->isForced() || lhsp->varp()->isSigUserRWPublic()) continue; + // Don't overwrite a later binding + if (m_bindings.get(lhsp->varScopep())) continue; + // Set up the binding + m_bindings.set(lhsp->varScopep(), valp); + } + // Attempt to unroll this loop - const std::pair pair = UnrollOneVisitor::apply(m_stats, nodep); + const std::pair pair = UnrollOneVisitor::apply(m_stats, m_bindings, nodep); // If failed, carry on with nested loop if (!pair.second) { diff --git a/test_regress/t/t_unroll_pragma_none.py b/test_regress/t/t_unroll_pragma_none.py index 24144ca3f..ea66b1ba4 100755 --- a/test_regress/t/t_unroll_pragma_none.py +++ b/test_regress/t/t_unroll_pragma_none.py @@ -20,6 +20,6 @@ test.compile(verilator_flags2=['--unroll-count 4 --unroll-stmts 9999 --stats -DT 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) + r'Optimizations, Loop unrolling, Failed - reached --unroll-count\s+(\d+)', 7) test.passes() diff --git a/test_regress/t/t_unroll_stmt.out b/test_regress/t/t_unroll_stmt.out index 50d32c577..69c4f7630 100644 --- a/test_regress/t/t_unroll_stmt.out +++ b/test_regress/t/t_unroll_stmt.out @@ -31,4 +31,33 @@ loop_6 3 loop_6 4 loop_6 5 stopping loop_6 +loop_7 0 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 1 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 2 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 3 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 4 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_9 0 +loop_9 1 +loop_9 2 +loop_9 3 *-* All Finished *-* diff --git a/test_regress/t/t_unroll_stmt.py b/test_regress/t/t_unroll_stmt.py index d7b6c0bfe..6d15f7c2e 100755 --- a/test_regress/t/t_unroll_stmt.py +++ b/test_regress/t/t_unroll_stmt.py @@ -26,7 +26,7 @@ test.file_grep(test.stats, test.file_grep(test.stats, r'Optimizations, Loop unrolling, Failed - unknown loop condition\s+(\d+)', 0) test.file_grep(test.stats, r'Optimizations, Loop unrolling, Pragma unroll_disable\s+(\d+)', 0) -test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 6) -test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 40) +test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 13) +test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 77) test.passes() diff --git a/test_regress/t/t_unroll_stmt.v b/test_regress/t/t_unroll_stmt.v index 2b651df4b..a40495dfd 100644 --- a/test_regress/t/t_unroll_stmt.v +++ b/test_regress/t/t_unroll_stmt.v @@ -12,6 +12,8 @@ module t; return ++static_loop_cond < 8; endfunction + int nonConst3 = $c("3"); + initial begin // Basic loop for (int i = 0; i < 3; ++i) begin : loop_0 @@ -59,6 +61,22 @@ module t; end end end + // Outer condition updated in inner loop + begin + automatic int i = 0; + while (i < 5) begin : loop_7 + $display("loop_7 %0d", i); + for (int j = 0 ; j < 4; ++j) begin : loop_8 + $display("loop_8 %0d", j); + if (j == 3) ++i; + end + end + end + // Data dependent break + for (int i = 0; i < 5; ++i) begin : loop_9 + $display("loop_9 %0d", i); + if (i == nonConst3) break; + end // Done $write("*-* All Finished *-*\n"); $finish;