From e71d4e7fe576cdabe3b04047c26fd5916449584d Mon Sep 17 00:00:00 2001 From: Jonathan Drolet Date: Mon, 21 Sep 2026 23:17:39 -0400 Subject: [PATCH] Fix force read update sensitivity quadratic (#8431) (#8433) --- src/V3Force.cpp | 4 +- test_regress/t/t_forceable_unpacked_large.py | 20 ------ test_regress/t/t_forceable_unpacked_large.v | 66 -------------------- test_regress/t/t_forceable_unpacked_scale.py | 49 +++++++++++++++ test_regress/t/t_forceable_unpacked_scale.v | 17 +++++ 5 files changed, 69 insertions(+), 87 deletions(-) delete mode 100755 test_regress/t/t_forceable_unpacked_large.py delete mode 100644 test_regress/t/t_forceable_unpacked_large.v create mode 100755 test_regress/t/t_forceable_unpacked_scale.py create mode 100644 test_regress/t/t_forceable_unpacked_scale.v diff --git a/src/V3Force.cpp b/src/V3Force.cpp index dba84ff58..76bcdc97b 100644 --- a/src/V3Force.cpp +++ b/src/V3Force.cpp @@ -792,7 +792,9 @@ public: = new AstSenItem{flp, VEdgeType::ET_CHANGED, origSenRefp}; if (!itemsp) varp->v3fatalSrc("force-rd-update missing force-enable sen item"); itemsp->addNext(origItemp); - for (ForceInfo* const finfop : forceps) addSenItem(finfop->m_rhsVarVscp); + for (ForceInfo* const finfop : forceps) { + if (!finfop->m_isExternal) addSenItem(finfop->m_rhsVarVscp); + } AstActive* const activep = new AstActive{flp, "force-rd-update", new AstSenTree{flp, itemsp}}; diff --git a/test_regress/t/t_forceable_unpacked_large.py b/test_regress/t/t_forceable_unpacked_large.py deleted file mode 100755 index e74a64262..000000000 --- a/test_regress/t/t_forceable_unpacked_large.py +++ /dev/null @@ -1,20 +0,0 @@ -#!/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.skip("Too slow; Issue #8431") - -test.compile(verilator_flags2=["--binary"]) - -test.execute() - -test.passes() diff --git a/test_regress/t/t_forceable_unpacked_large.v b/test_regress/t/t_forceable_unpacked_large.v deleted file mode 100644 index b24615dc3..000000000 --- a/test_regress/t/t_forceable_unpacked_large.v +++ /dev/null @@ -1,66 +0,0 @@ -// 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 - -// Checks that a whole-array self-force ("force mem = mem;") on a large -// unpacked array compiles quickly rather than taking O(elements^2). - -`define stop $stop -`define checkh(g,e) do if ((g) !== (e)) begin $write("%%Error: %s:%0d: got=%x exp=%x\n", `__FILE__,`__LINE__, (g),(e)); `stop; end while(0) - -module t; - localparam int ArraySize = 1024; - localparam int MatSize = 64; - - logic [7:0] mem[0:ArraySize-1] /*verilator forceable*/; - logic [7:0] mat[0:MatSize-1][0:MatSize-1] /*verilator forceable*/; - logic go; - logic go2; - - initial begin - go = 1'b0; - go2 = 1'b0; - for (int i = 0; i < ArraySize; ++i) mem[i] = 8'h00; - for (int i = 0; i < MatSize; ++i) - for (int j = 0; j < MatSize; ++j) mat[i][j] = 8'h00; - end - - // The idiom under test: a self-referential whole-array force. - always @(posedge go) force mem = mem; - always @(posedge go2) force mat = mat; - - initial begin - #1; - for (int i = 0; i < ArraySize; ++i) mem[i] = 8'(i); - for (int i = 0; i < MatSize; ++i) - for (int j = 0; j < MatSize; ++j) mat[i][j] = 8'(i * MatSize + j); - #1; - `checkh(mem[0], 8'd0); - `checkh(mem[1], 8'd1); - `checkh(mem[255], 8'd255); - `checkh(mem[ArraySize-1], 8'(ArraySize - 1)); - `checkh(mat[0][0], 8'd0); - `checkh(mat[0][1], 8'd1); - `checkh(mat[MatSize-1][MatSize-1], 8'(MatSize * MatSize - 1)); - - go = 1'b1; - go2 = 1'b1; - #1; - for (int i = 0; i < ArraySize; ++i) mem[i] = 8'(255 - i); - for (int i = 0; i < MatSize; ++i) - for (int j = 0; j < MatSize; ++j) mat[i][j] = 8'(255 - (i * MatSize + j)); - #1; - `checkh(mem[0], 8'd255); - `checkh(mem[1], 8'd254); - `checkh(mem[255], 8'd0); - `checkh(mem[ArraySize-1], 8'(255 - (ArraySize - 1))); - `checkh(mat[0][0], 8'd255); - `checkh(mat[0][1], 8'd254); - `checkh(mat[MatSize-1][MatSize-1], 8'(255 - (MatSize * MatSize - 1))); - - $write("*-* All Finished *-*\n"); - $finish; - end -endmodule diff --git a/test_regress/t/t_forceable_unpacked_scale.py b/test_regress/t/t_forceable_unpacked_scale.py new file mode 100755 index 000000000..a97ee9765 --- /dev/null +++ b/test_regress/t/t_forceable_unpacked_scale.py @@ -0,0 +1,49 @@ +#!/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 re + +import vltest_bootstrap + +test.scenarios('vlt') + +SIZE_SMALL = 512 +SIZE_LARGE = SIZE_SMALL * 4 +# Linear scaling gives 4x, quadratic gives 16x +MAX_RATIO = 8 +# Ignore the ratio when scheduling is fast enough that timer noise dominates +MIN_SECONDS = 0.1 +# Peak memory limit at the small size (about 50 MB when fine, over 2000 MB when quadratic) +MAX_MB = 500 + + +def compile_stats(size): + test.compile(verilator_flags2=["--stats", "--no-debug-check", "-GArraySize=" + str(size)], + make_main=False, + verilator_make_gmake=False) + stats = test.file_contents(test.stats) + times = re.findall(r'Stage, Elapsed time \(sec\), \d+_sched\S*\s+(\S+)', stats) + peak = re.search(r'Peak Memory Usage \(MB\)\s+(\S+)', stats) + if not times or not peak: + test.error("Scheduling time or peak memory not found in " + test.stats) + return sum(float(t) for t in times), float(peak.group(1)) + + +small, small_mb = compile_stats(SIZE_SMALL) +# Do not build the large size if memory already blew up +if small_mb > MAX_MB: + test.error(f"Peak memory with {SIZE_SMALL} forced array elements was {small_mb:.0f} MB " + + f"(over {MAX_MB} MB)") +large, _ = compile_stats(SIZE_LARGE) +if large > MIN_SECONDS and large > small * MAX_RATIO: + test.error("Scheduling time scaled superlinearly with forced array size: " + + f"{SIZE_SMALL} elements took {small:.3f}s, " + + f"{SIZE_LARGE} elements took {large:.3f}s (over {MAX_RATIO}x)") + +test.passes() diff --git a/test_regress/t/t_forceable_unpacked_scale.v b/test_regress/t/t_forceable_unpacked_scale.v new file mode 100644 index 000000000..192f2f12f --- /dev/null +++ b/test_regress/t/t_forceable_unpacked_scale.v @@ -0,0 +1,17 @@ +// 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 + +module t ( + input logic go +); + parameter int ArraySize = 512; + + logic [7:0] mem[0:ArraySize-1] /*verilator forceable*/; + logic [7:0] mat[0:ArraySize/8-1][0:7] /*verilator forceable*/; + + always @(posedge go) force mem = mem; + always @(posedge go) force mat = mat; +endmodule