luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix exponential expansion of nested selects (#8137) (#8151) 

Fixes #8137
This commit is contained in:
Geza Lore 2026-08-19 11:00:47 +02:00 committed by GitHub
parent 8c2b7410f0
commit 7523a0cd8e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 258 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/V3Ast.h
View File

@ -1261,6 +1261,14 @@ public:
this->foreach([&count](const AstNode*) { ++count; }); this->foreach([&count](const AstNode*) { ++count; });
return count; return count;
} }
// Return true if and only if the tree rooted at this node has more than 'limit' nodes.
// Traversal terminates as soon as the result is known, so unlike comparing 'nodeCount',
// this is cheap on a large tree.
bool isLargerThan(int limit) const {
int count = 0;
return this->exists([&count, limit](const AstNode*) { return ++count > limit; });
}
}; };
// Forward declarations of specializations defined in V3Ast.cpp // Forward declarations of specializations defined in V3Ast.cpp

193
src/V3Expand.cpp
View File

@ -489,83 +489,162 @@ class ExpandVisitor final : public VNVisitor {
void visit(AstSel* nodep) override { void visit(AstSel* nodep) override {
if (nodep->user1SetOnce()) return; // Process once if (nodep->user1SetOnce()) return; // Process once
iterateChildren(nodep); iterateChildren(nodep);
// Remember, Sel's may have non-integer rhs, so need to optimize for that!
UASSERT_OBJ(nodep->widthMin() == nodep->widthConst(), nodep, "Width mismatch"); const uint32_t width = static_cast<uint32_t>(nodep->widthConst());
if (VN_IS(nodep->backp(), NodeAssign) UASSERT_OBJ(nodep->widthMin() == static_cast<int>(width), nodep, "Width mismatch");
&& nodep == VN_AS(nodep->backp(), NodeAssign)->lhsp()) {
// Sel is an LHS assignment select // Skip if Sel is an LHS assignment select
} else if (nodep->isWide()) { if (AstNodeAssign* const assignp = VN_CAST(nodep->backp(), NodeAssign)) {
// See under ASSIGN(WIDE) if (nodep == assignp->lhsp()) return;
} else if (VN_IS(nodep->fromp()->dtypep(), StreamDType) }
|| VN_IS(nodep->fromp()->dtypep(), QueueDType)) {
//sel stream or queue // Skip if wide: See under ASSIGN(WIDE)
} else if (nodep->fromp()->isWide()) { if (nodep->isWide()) return;
// Skip Sel from stream or queue
if (VN_IS(nodep->fromp()->dtypep(), StreamDType)) return;
if (VN_IS(nodep->fromp()->dtypep(), QueueDType)) return;
// Result must be non-wide after skipping all the above
UASSERT_OBJ(width <= 64, nodep, "Inconsistent result width");
if (nodep->fromp()->isWide()) { // Long/Quad from Wide
if (isImpure(nodep)) return; if (isImpure(nodep)) return;
UINFO(8, " SEL(wide) " << nodep); UINFO(8, " SEL(wide) " << nodep);
UASSERT_OBJ(nodep->widthConst() <= 64, nodep, "Inconsistent width");
// Selection amounts
// Check for constant shifts & save some constification work later.
// Grab lowest bit(s)
FileLine* const nfl = nodep->fileline(); FileLine* const nfl = nodep->fileline();
FileLine* const lfl = nodep->lsbp()->fileline(); FileLine* const lfl = nodep->lsbp()->fileline();
FileLine* const ffl = nodep->fromp()->fileline(); FileLine* const ffl = nodep->fromp()->fileline();
AstNodeExpr* lowwordp = newWordSelBit(ffl, nodep->fromp(), nodep->lsbp()); AstNodeExpr* const fromp = nodep->fromp();
if (nodep->isQuad() && !lowwordp->isQuad()) {
lowwordp = new AstCCast{nfl, lowwordp, nodep}; // To extract an up-to-64-bit value from a wide source, up to 3 words might be needed
// from the source operand. Each word might need to be shifted by a different amount.
// All word indices and shift amounts are derived from the LSB expression, so it might
// be duplicated several times. The problem arises when the LSB expression itself is
// a similar select, which results in an exponential size expansion.
//
// To avoid this, if the required expressions derived from the LSB expression are
// large, and the LSB expression is required multiple times for the expansion, then
// the LSB expression is evaluated into a temporary variable. This bounds the size
// of the expansion on each Sel instance, hence the total expansion of nested Sels
// is bounded linearly with the number of nested Sels (instead of exponentially).
// Return simplified 'exprp' if it simplifies to a small tree, otherwise nullptr
const auto tryFold = [&](AstNodeExpr* exprp) -> AstNodeExpr* {
// Size limit for inlining expressions (any arbitrary value results in
// a bounded expansion. Picked a value to allow common simple forms.)
static constexpr int EXPAND_SEL_LSB_LIMIT = 8;
// Simplify
exprp = V3Const::constifyEditCpp(exprp);
// Accept if not larger than the limit
if (!exprp->isLargerThan(EXPAND_SEL_LSB_LIMIT)) return exprp;
// Delete if rejected
VL_DO_DANGLING(exprp->deleteTree(), exprp);
return nullptr;
};
// Return the index of the word holding the bit 'offset' bits above the LSB
const auto wordIdx = [&](uint32_t offset) -> AstNodeExpr* {
AstNodeExpr* const msbp = new AstAdd{lfl, new AstConst{lfl, offset},
nodep->lsbp()->cloneTreePure(false)};
AstNodeExpr* const idxp = newWordIndex(msbp);
if (!msbp->backp()) VL_DO_DANGLING(msbp->deleteTree(), msbp);
return idxp;
};
// Bit index of the selected LSB within its word
AstNodeExpr* lBitp = tryFold(newSelBitBit(nodep->lsbp()));
const bool aligned = lBitp && lBitp->isZero();
// Which of the other 2 words the expansion might need
const bool mNeeded = width > 1 && !aligned;
const bool hNeeded = width > VL_EDATASIZE;
UASSERT_OBJ(!hNeeded || nodep->isQuad(), nodep, "Width mismatch");
// Word indices
const uint32_t mMsbOffset = std::min<uint32_t>(width, VL_EDATASIZE) - 1;
const uint32_t hMsbOffset = width - 1;
AstNodeExpr* lIdxp = tryFold(newWordIndex(nodep->lsbp()));
AstNodeExpr* mIdxp = mNeeded ? tryFold(wordIdx(mMsbOffset)) : nullptr;
AstNodeExpr* hIdxp = hNeeded ? tryFold(wordIdx(hMsbOffset)) : nullptr;
// If the LSB expression is needed multiple times, evaluate it into a temporary
const int nCopies = !lBitp + !lIdxp + (mNeeded && !mIdxp) + (hNeeded && !hIdxp);
if (nCopies > 1 && m_funcp && m_stmtp && !isImpure(m_stmtp)) {
++m_nTmps; // Use fresh set of temporaries
AstNodeExpr* const lsbp = nodep->lsbp()->unlinkFrBack();
AstVar* const tmpp = addLocalTmp(m_stmtp, "ExpandSel_Lsb", lsbp);
nodep->lsbp(new AstVarRef{lfl, tmpp, VAccess::READ});
} }
AstNodeExpr* const lowp
= new AstShiftR{nfl, lowwordp, newSelBitBit(nodep->lsbp()), nodep->width()}; // Rebuild remaining expressions derived from LSB if needed, now using the temporary
// If > 1 bit, we might be crossing the word boundary if (!lBitp) lBitp = newSelBitBit(nodep->lsbp());
AstNodeExpr* midp = nullptr; if (!lIdxp) lIdxp = newWordIndex(nodep->lsbp());
if (nodep->widthConst() > 1) { if (!mIdxp) mIdxp = mNeeded ? wordIdx(mMsbOffset) : nullptr;
const uint32_t midMsbOffset if (!hIdxp) hIdxp = hNeeded ? wordIdx(hMsbOffset) : nullptr;
= std::min<uint32_t>(nodep->widthConst(), VL_EDATASIZE) - 1;
AstNodeExpr* const midMsbp = new AstAdd{lfl, new AstConst{lfl, midMsbOffset}, // Return word 'idxp' of 'fromp', without consuming 'idxp'
nodep->lsbp()->cloneTreePure(true)}; const auto wordSel = [&](AstNodeExpr* idxp) -> AstNodeExpr* {
AstNodeExpr* midwordp = newWordSelBit(ffl, nodep->fromp(), midMsbp, 0); AstNodeExpr* const clonep = idxp->cloneTreePure(false);
if (!midMsbp->backp()) VL_DO_DANGLING(midMsbp->deleteTree(), midMsbp); AstNodeExpr* const wordp = newWordSelWord(ffl, fromp, clonep);
if (nodep->isQuad() && !midwordp->isQuad()) { if (!clonep->backp()) VL_DO_DANGLING(clonep->deleteTree(), clonep);
midwordp = new AstCCast{nfl, midwordp, nodep}; return wordp;
} };
AstNodeExpr* const midshiftp = new AstSub{lfl, new AstConst{lfl, VL_EDATASIZE},
newSelBitBit(nodep->lsbp())}; // Construct term containing the bits of the low word - always needed
AstNodeExpr* const lTermp = [&]() -> AstNodeExpr* {
AstNodeExpr* lWordp = wordSel(lIdxp);
if (nodep->isQuad()) lWordp = new AstCCast{nfl, lWordp, nodep};
return new AstShiftR{nfl, lWordp, lBitp->cloneTreePure(false), nodep->width()};
}();
// Construct term containing the bits of the middle word - if needed
AstNodeExpr* const mTermp = [&]() -> AstNodeExpr* {
if (!mNeeded) return nullptr;
AstNodeExpr* mWordp = wordSel(mIdxp);
if (nodep->isQuad()) mWordp = new AstCCast{nfl, mWordp, nodep};
AstNodeExpr* const mShiftp = new AstSub{lfl, new AstConst{lfl, VL_EDATASIZE},
lBitp->cloneTreePure(false)};
// If we're selecting bit zero, then all 32 bits in the mid word // If we're selecting bit zero, then all 32 bits in the mid word
// get shifted << by 32 bits, so ignore them. // get shifted << by 32 bits, so ignore them.
const V3Number zero{nodep, longOrQuadWidth(nodep)}; const V3Number zero{nodep, longOrQuadWidth(nodep)};
midp = new AstCond{ return new AstCond{
nfl, nfl,
// lsb % VL_EDATASIZE == 0 ? // lsb % VL_EDATASIZE == 0 ?
new AstEq{nfl, new AstConst{nfl, 0}, newSelBitBit(nodep->lsbp())}, new AstEq{nfl, new AstConst{nfl, 0}, lBitp->cloneTreePure(false)},
// 0 : // 0 :
new AstConst{nfl, zero}, new AstConst{nfl, zero},
// midword >> (VL_EDATASIZE - (lbs % VL_EDATASIZE)) // midword >> (VL_EDATASIZE - (lbs % VL_EDATASIZE))
new AstShiftL{nfl, midwordp, midshiftp, nodep->width()}}; new AstShiftL{nfl, mWordp, mShiftp, nodep->width()}};
} }();
// If > 32 bits, we might be crossing the second word boundary
AstNodeExpr* hip = nullptr; // Construct term containing the bits of the high word - if needed
if (nodep->widthConst() > VL_EDATASIZE) { AstNodeExpr* const hTermp = [&]() -> AstNodeExpr* {
const uint32_t hiMsbOffset = nodep->widthConst() - 1; if (!hNeeded) return nullptr;
AstNodeExpr* const hiMsbp = new AstAdd{lfl, new AstConst{lfl, hiMsbOffset},
nodep->lsbp()->cloneTreePure(true)}; AstNodeExpr* hWordp = wordSel(hIdxp);
AstNodeExpr* hiwordp = newWordSelBit(ffl, nodep->fromp(), hiMsbp); hWordp = new AstCCast{nfl, hWordp, nodep};
if (!hiMsbp->backp()) VL_DO_DANGLING(hiMsbp->deleteTree(), hiMsbp); AstNodeExpr* const hShiftp = new AstCond{
if (nodep->isQuad() && !hiwordp->isQuad()) {
hiwordp = new AstCCast{nfl, hiwordp, nodep};
}
AstNodeExpr* const hishiftp = new AstCond{
nfl, nfl,
// lsb % VL_EDATASIZE == 0 ? // lsb % VL_EDATASIZE == 0 ?
new AstEq{nfl, new AstConst{nfl, 0}, newSelBitBit(nodep->lsbp())}, new AstEq{nfl, new AstConst{nfl, 0}, lBitp->cloneTreePure(false)},
// VL_EDATASIZE : // VL_EDATASIZE :
new AstConst{lfl, VL_EDATASIZE}, new AstConst{lfl, VL_EDATASIZE},
// 64 - (lbs % VL_EDATASIZE) // 64 - (lbs % VL_EDATASIZE)
new AstSub{lfl, new AstConst{lfl, 64}, newSelBitBit(nodep->lsbp())}}; new AstSub{lfl, new AstConst{lfl, 64}, lBitp->cloneTreePure(false)}};
hip = new AstShiftL{nfl, hiwordp, hishiftp, nodep->width()}; return new AstShiftL{nfl, hWordp, hShiftp, nodep->width()};
} }();
AstNodeExpr* newp = lowp; // Delete parts not captured during construction
if (midp) newp = new AstOr{nfl, midp, newp}; VL_DO_DANGLING(lBitp->deleteTree(), lBitp);
if (hip) newp = new AstOr{nfl, hip, newp}; VL_DO_DANGLING(lIdxp->deleteTree(), lIdxp);
if (mIdxp) VL_DO_DANGLING(mIdxp->deleteTree(), mIdxp);
if (hIdxp) VL_DO_DANGLING(hIdxp->deleteTree(), hIdxp);
// Or reduce the terms
AstNodeExpr* newp = lTermp;
if (mTermp) newp = new AstOr{nfl, mTermp, newp};
if (hTermp) newp = new AstOr{nfl, hTermp, newp};
newp->dtypeFrom(nodep); newp->dtypeFrom(nodep);
VL_DO_DANGLING(replaceWithDelete(nodep, newp), nodep); VL_DO_DANGLING(replaceWithDelete(nodep, newp), nodep);
} else { // Long/Quad from Long/Quad } else { // Long/Quad from Long/Quad

23
test_regress/t/t_opt_expand_sel_wide_nested.py Executable file
View File

@ -0,0 +1,23 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# 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-FileCopyrightText: 2026 Wilson Snyder
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('vlt')
test.compile(verilator_flags2=["--binary", "--stats"])
memUsageMB = int(test.file_grep(test.stats, r'Peak Memory Usage \(MB\) +(\d+)')[0])
if memUsageMB > 128 and not test.have_dev_asan:
test.error("Consumed over 128MB memory")
test.execute()
test.passes()

91
test_regress/t/t_opt_expand_sel_wide_nested.v Normal file
View File

@ -0,0 +1,91 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain.
// SPDX-FileCopyrightText: 2026 Wilson Snyder
// SPDX-License-Identifier: CC0-1.0
// verilog_format: off
`define stop $stop
`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0x exp=%0x (%s !== %s)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0);
// verilog_format: on
module t;
//--------------------------------------------------------------------
// Stimulus/test driver
logic clk = 0;
always #5 clk = ~clk;
int cyc = 0;
logic [31:0] rng = 32'h1234_5678;
function automatic logic [31:0] xorshift(input logic [31:0] x);
logic [31:0] r;
r = x ^ (x << 13);
r = r ^ (r >> 17);
r = r ^ (r << 5);
return r;
endfunction
always @(posedge clk) begin
cyc <= cyc + 1;
rng <= xorshift(rng);
if (cyc == 500) begin
$write("*-* All Finished *-*\n");
$finish;
end
end
//--------------------------------------------------------------------
// Deeply nested variable offset selects from a wide value.
logic [15:0][31:0] lut = '0;
logic [31:0] arr[16] = '{default: 32'h0}; // Same as 'lut', but unpacked
logic [3:0] start = 4'h0;
// Explicitly nested so test doesn't depend on unrolling/dfg, or other opts
wire [3:0] chainLut
= 4'(lut[4'(lut[4'(lut[4'(lut[4'(lut[4'(lut[4'(lut[4'(lut[start])])])])])])])]);
wire [3:0] chainArr
= 4'(arr[4'(arr[4'(arr[4'(arr[4'(arr[4'(arr[4'(arr[4'(arr[start])])])])])])])]);
// Check exponential expansion
always @(posedge clk) begin
`checkh(chainLut, chainArr);
// Shift a new entry into 'lut' and into its reference array in step
lut <= {lut[14:0], rng};
for (int i = 15; i > 0; --i) arr[i] <= arr[i-1];
arr[0] <= rng;
start <= rng[3:0];
end
//--------------------------------------------------------------------
// Check access boundaries
logic [511:0] data = 512'h0;
logic [8:0] lsb = 9'h0;
wire [0:0] sel01 = data[lsb+:01];
wire [3:0] sel04 = data[lsb+:04];
wire [30:0] sel31 = data[lsb+:31];
wire [31:0] sel32 = data[lsb+:32];
wire [32:0] sel33 = data[lsb+:33];
wire [63:0] sel64 = data[lsb+:64];
always @(posedge clk) begin
`checkh(sel01, 1'(data >> lsb));
`checkh(sel04, 4'(data >> lsb));
`checkh(sel31, 31'(data >> lsb));
`checkh(sel32, 32'(data >> lsb));
`checkh(sel33, 33'(data >> lsb));
`checkh(sel64, 64'(data >> lsb));
// Sweep every offset, so all word alignments are covered. Stop at 448,
// so that even the widest select stays in range and the reference is
// defined.
lsb <= 9'(cyc % 449);
data <= {data[479:0], ~rng};
end
endmodule