From ba3937734f520809e0d0cdd477fd367bc871e3a9 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 23 Apr 2026 16:57:43 +0100 Subject: [PATCH] Optimize mix of Concat/Extend assignments (#7479) Generalize concatenation splitting/balancing in V3FuncOpt to also handle AstExtend (which is a Concat with a 0 LHS). --- src/V3FuncOpt.cpp | 93 +++++++++++++++++++------ test_regress/t/t_opt_balance_cats.py | 4 +- test_regress/t/t_opt_balance_cats.v | 53 +++++++++----- test_regress/t/t_opt_balance_cats_sc.py | 4 +- test_regress/t/t_opt_const.py | 2 +- 5 files changed, 112 insertions(+), 44 deletions(-) diff --git a/src/V3FuncOpt.cpp b/src/V3FuncOpt.cpp index 002299db3..4e4423161 100644 --- a/src/V3FuncOpt.cpp +++ b/src/V3FuncOpt.cpp @@ -14,7 +14,7 @@ // //************************************************************************* // -// - Split assignments to wide locations with Concat on the RHS +// - Split assignments to wide locations with Concat/Extend on the RHS // at word boundaries: // foo = {l, r}; // becomes (recursively): @@ -60,6 +60,12 @@ class BalanceConcatTree final { gatherTermsRecursive(catp->lhsp(), terms); return; } + if (AstExtend* const extp = VN_CAST(exprp, Extend)) { + // Recursive case: gather sub terms, right to left + gatherTermsRecursive(extp->lhsp(), terms); + terms.emplace_back(extp); + return; + } // Base case: different operation terms.emplace_back(exprp); @@ -68,10 +74,18 @@ class BalanceConcatTree final { // Gather terms in the tree rooted at the given node. // Results are right to left, that is, index 0 in the returned vector // is the rightmost term, index size()-1 is the leftmost term. - static std::vector gatherTerms(AstConcat* rootp) { + // If a term is an AstExtend, it represents the extension part only. + static std::vector gatherTerms(AstNodeExpr* rootp) { std::vector terms; - gatherTermsRecursive(rootp->rhsp(), terms); - gatherTermsRecursive(rootp->lhsp(), terms); + if (AstConcat* const catp = VN_CAST(rootp, Concat)) { + gatherTermsRecursive(catp->rhsp(), terms); + gatherTermsRecursive(catp->lhsp(), terms); + } else if (AstExtend* const extp = VN_CAST(rootp, Extend)) { + gatherTermsRecursive(extp->lhsp(), terms); + terms.emplace_back(extp); + } else { + rootp->v3fatalSrc("Unexpected node type"); + } return terms; } @@ -108,7 +122,7 @@ class BalanceConcatTree final { } // Returns replacement node, or nullptr if no change - static AstConcat* balance(AstConcat* const rootp) { + static AstConcat* balance(AstNodeExpr* const rootp) { UINFO(9, "balanceConcat " << rootp); // Gather all input vertices of the tree const std::vector exprps = gatherTerms(rootp); @@ -125,8 +139,15 @@ class BalanceConcatTree final { terms[0].offset = 0; terms[exprps.size()].exprp = nullptr; for (size_t i = 0; i < exprps.size(); ++i) { - terms[i].exprp = exprps[i]->unlinkFrBack(); - terms[i + 1].offset = terms[i].offset + exprps[i]->width(); + AstNodeExpr* const exprp = [&]() -> AstNodeExpr* { + if (AstExtend* const extp = VN_CAST(exprps[i], Extend)) { + const int width = extp->width() - extp->lhsp()->width(); + return new AstConst{extp->fileline(), AstConst::WidthedValue{}, width, 0}; + } + return exprps[i]->cloneTreePure(false); + }(); + terms[i].exprp = exprp; + terms[i + 1].offset = terms[i].offset + exprp->width(); } // Round 1: try to create terms ending on VL_EDATASIZE boundaries. @@ -166,7 +187,15 @@ class BalanceConcatTree final { } public: - static AstConcat* apply(AstConcat* rootp) { return balance(rootp); } + static AstNodeExpr* apply(AstNodeExpr* nodep) { + if (!v3Global.opt.fFuncBalanceCat()) return nullptr; + if (nodep->user1()) return nullptr; // Created by us, don't try to balance again + if (VN_IS(nodep->backp(), Concat)) return nullptr; // Not root of tree + if (VN_IS(nodep->backp(), Extend)) return nullptr; // Not root of tree + AstNodeExpr* const exprp = balance(nodep); + if (exprp) exprp->user1(true); // Must not attempt again. + return exprp; + } }; struct FuncOptStats final { @@ -230,9 +259,9 @@ class FuncOptVisitor final : public VNVisitor { // Returns true if 'nodep' was deleted bool splitConcat(AstNodeAssign* nodep) { UINFO(9, "splitConcat " << nodep); - // Only care about concatenations on the right - AstConcat* const rhsp = VN_CAST(nodep->rhsp(), Concat); - if (!rhsp) return false; + AstNodeExpr* const rhsp = nodep->rhsp(); + // Only care about concatenations an zero extend on the RHS + if (!VN_IS(rhsp, Concat) && !VN_IS(rhsp, Extend)) return false; // Will need the LHS AstNodeExpr* lhsp = nodep->lhsp(); UASSERT_OBJ(lhsp->width() == rhsp->width(), nodep, "Inconsistent assignment"); @@ -268,8 +297,19 @@ class FuncOptVisitor final : public VNVisitor { UINFO(5, "splitConcat optimizing " << nodep); ++m_stats.m_concatSplits; // The 2 parts and their offsets - AstNodeExpr* const rrp = rhsp->rhsp()->unlinkFrBack(); - AstNodeExpr* const rlp = rhsp->lhsp()->unlinkFrBack(); + AstNodeExpr* const rrp = [rhsp]() -> AstNodeExpr* { + if (AstConcat* const catp = VN_CAST(rhsp, Concat)) { + return catp->rhsp()->unlinkFrBack(); + } + return VN_AS(rhsp, Extend)->lhsp()->unlinkFrBack(); + }(); + AstNodeExpr* const rlp = [rhsp, rrp]() -> AstNodeExpr* { + if (AstConcat* const catp = VN_CAST(rhsp, Concat)) { + return catp->lhsp()->unlinkFrBack(); + } + const int lWidth = rhsp->width() - rrp->width(); + return new AstConst{rhsp->fileline(), AstConst::WidthedValue{}, lWidth, 0}; + }(); const int rLsb = lsb; const int lLsb = lsb + rrp->width(); // Insert the 2 assignment right after the original. They will be visited next. @@ -301,16 +341,23 @@ class FuncOptVisitor final : public VNVisitor { } void visit(AstConcat* nodep) override { - if (v3Global.opt.fFuncBalanceCat() && !nodep->user1() && !VN_IS(nodep->backp(), Concat)) { - if (AstConcat* const newp = BalanceConcatTree::apply(nodep)) { - UINFO(5, "balanceConcat optimizing " << nodep); - ++m_stats.m_balancedConcats; - nodep->replaceWith(newp); - VL_DO_DANGLING(pushDeletep(nodep), nodep); - newp->user1(true); // Must not attempt again. - // Return here. The new node will be iterated next. - return; - } + if (AstNodeExpr* const newp = BalanceConcatTree::apply(nodep)) { + UINFO(5, "balanceConcat optimizing " << nodep); + ++m_stats.m_balancedConcats; + nodep->replaceWith(newp); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + return; // The new node will be iterated next + } + iterateChildren(nodep); + } + + void visit(AstExtend* nodep) override { + if (AstNodeExpr* const newp = BalanceConcatTree::apply(nodep)) { + UINFO(5, "balanceConcat optimizing " << nodep); + ++m_stats.m_balancedConcats; + nodep->replaceWith(newp); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + return; // The new node will be iterated next } iterateChildren(nodep); } diff --git a/test_regress/t/t_opt_balance_cats.py b/test_regress/t/t_opt_balance_cats.py index cca6c4f1a..577c19985 100755 --- a/test_regress/t/t_opt_balance_cats.py +++ b/test_regress/t/t_opt_balance_cats.py @@ -14,7 +14,7 @@ test.scenarios('vlt') test.compile( verilator_flags2=["--stats", "--build", "--gate-stmts", "10000", "--expand-limit", "128"]) -test.file_grep(test.stats, r'Optimizations, FuncOpt concat trees balanced\s+(\d+)', 1) -test.file_grep(test.stats, r'Optimizations, FuncOpt concat splits\s+(\d+)', 62) +test.file_grep(test.stats, r'Optimizations, FuncOpt concat trees balanced\s+(\d+)', 2) +test.file_grep(test.stats, r'Optimizations, FuncOpt concat splits\s+(\d+)', 67) test.passes() diff --git a/test_regress/t/t_opt_balance_cats.v b/test_regress/t/t_opt_balance_cats.v index 154a9266f..2131b6a70 100644 --- a/test_regress/t/t_opt_balance_cats.v +++ b/test_regress/t/t_opt_balance_cats.v @@ -4,30 +4,51 @@ // SPDX-FileCopyrightText: 2024 Wilson Snyder // SPDX-License-Identifier: CC0-1.0 -module t(i, o); +module t( + clk, + i0, o0, + i1, o1 +); localparam N = 2000; // Deliberately not multiple of 32 - input i; - wire [N-1:0] i; + input clk; + wire clk; - output o; - wire [N-1:0] o; + // Case 1: concatenations only + + input i0; + wire [N-1:0] i0; + + output o0; + wire [N-1:0] o0; for (genvar n = 0 ; n + 31 < N ; n += 32) begin - assign o[n+ 0 +: 1] = i[(N-1-n)- 0 -: 1]; - assign o[n+ 1 +: 1] = i[(N-1-n)- 1 -: 1]; - assign o[n+ 2 +: 2] = i[(N-1-n)- 2 -: 2]; - assign o[n+ 4 +: 4] = i[(N-1-n)- 4 -: 4]; - assign o[n+ 8 +: 8] = i[(N-1-n)- 8 -: 8]; - assign o[n+16 +: 8] = i[(N-1-n)-16 -: 8]; - assign o[n+24 +: 4] = i[(N-1-n)-24 -: 4]; - assign o[n+28 +: 2] = i[(N-1-n)-28 -: 2]; - assign o[n+30 +: 1] = i[(N-1-n)-30 -: 1]; - assign o[n+31 +: 1] = i[(N-1-n)-31 -: 1]; + assign o0[n+ 0 +: 1] = i0[(N-1-n)- 0 -: 1]; + assign o0[n+ 1 +: 1] = i0[(N-1-n)- 1 -: 1]; + assign o0[n+ 2 +: 2] = i0[(N-1-n)- 2 -: 2]; + assign o0[n+ 4 +: 4] = i0[(N-1-n)- 4 -: 4]; + assign o0[n+ 8 +: 8] = i0[(N-1-n)- 8 -: 8]; + assign o0[n+16 +: 8] = i0[(N-1-n)-16 -: 8]; + assign o0[n+24 +: 4] = i0[(N-1-n)-24 -: 4]; + assign o0[n+28 +: 2] = i0[(N-1-n)-28 -: 2]; + assign o0[n+30 +: 1] = i0[(N-1-n)-30 -: 1]; + assign o0[n+31 +: 1] = i0[(N-1-n)-31 -: 1]; end for (genvar n = N / 32 * 32; n < N ; ++n) begin - assign o[n] = i[N-1-n]; + assign o0[n] = i0[N-1-n]; + end + + // Case 2: mixed concatenations and zero extension + + input i1; + logic [N-1:0] i1; + + output o1; + logic [N-1:0] o1; + + always @(posedge clk) begin + o1 = N'({i1[0 +: N/4], (N/2 - 128)'(i1[N/2 +: N/8]), 128'({i1[0 +: 10], 22'(i1[10 +: 10])})}); end endmodule diff --git a/test_regress/t/t_opt_balance_cats_sc.py b/test_regress/t/t_opt_balance_cats_sc.py index 59667fd06..ff851ed2a 100755 --- a/test_regress/t/t_opt_balance_cats_sc.py +++ b/test_regress/t/t_opt_balance_cats_sc.py @@ -17,7 +17,7 @@ test.compile(verilator_flags2=[ "--stats", "--build", "--gate-stmts", "10000", "--expand-limit", "128", "--sc" ]) -test.file_grep(test.stats, r'Optimizations, FuncOpt concat trees balanced\s+(\d+)', 1) -test.file_grep(test.stats, r'Optimizations, FuncOpt concat splits\s+(\d+)', 0) +test.file_grep(test.stats, r'Optimizations, FuncOpt concat trees balanced\s+(\d+)', 3) +test.file_grep(test.stats, r'Optimizations, FuncOpt concat splits\s+(\d+)', 5) test.passes() diff --git a/test_regress/t/t_opt_const.py b/test_regress/t/t_opt_const.py index 90b3b54f6..9a49ab61d 100755 --- a/test_regress/t/t_opt_const.py +++ b/test_regress/t/t_opt_const.py @@ -16,7 +16,7 @@ test.compile(verilator_flags2=["-Wno-UNOPTTHREADS", "-fno-dfg", "--stats", test. test.execute() if test.vlt: - test.file_grep(test.stats, r'Optimizations, Const bit op reduction\s+(\d+)', 49) + test.file_grep(test.stats, r'Optimizations, Const bit op reduction\s+(\d+)', 48) test.file_grep(test.stats, r'SplitVar, packed variables split automatically\s+(\d+)', 1) test.passes()