Skip to content

Commit

Permalink
Fix V3Simulate constant reuse
Browse files Browse the repository at this point in the history
Use a generational allocator for reusing AstConst across
V3Simulate::clear(), instead of using user1 (which is also used
to store values of nodes).

Also fix invalid lookup on array initializer

Fixes verilator#5707
  • Loading branch information
gezalore committed Jan 3, 2025
1 parent 951f5ea commit 36cc52f
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 38 deletions.
73 changes: 35 additions & 38 deletions src/V3Simulate.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
// Cleared on each always/assignw
const VNUser1InUse m_inuser1;

// AstVar/AstVarScope::user1p() -> See AuxAstVar via m_varAux
// AstConst::user1() -> bool. This AstConst (allocated by this class) is in use
// AstNode::user1p() -> See AuxAstVar via m_varAux

enum VarUsage : uint8_t { VU_NONE = 0, VU_LV = 1, VU_RV = 2, VU_LVDLY = 4 };

Expand All @@ -96,6 +95,32 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {

AstUser1Allocator<AstNode, AuxVariable> m_varAux;

// We want to re-use allocated constants across calls to clear(), but we want to be able
// to 'clear()' fast, so we use a generation number based allocator.
struct ConstAllocator final {
size_t m_generation = 0;
size_t m_nextFree = 0;
std::deque<AstConst*> m_constps;
AstConst* allocate(size_t currentGeneration, AstNode* nodep) {
if (m_generation != currentGeneration) {
m_generation = currentGeneration;
m_nextFree = 0;
}
UASSERT_OBJ(m_nextFree <= m_constps.size(), nodep, "Should only allocate at end");
if (m_nextFree == m_constps.size()) {
m_constps.push_back(
new AstConst{nodep->fileline(), AstConst::DTyped{}, nodep->dtypep()});
}
AstConst* const constp = m_constps[m_nextFree++];
constp->num().nodep(nodep);
return constp;
}

~ConstAllocator() {
for (AstConst* const constp : m_constps) VL_DO_DANGLING(delete constp, constp);
}
};

// STATE
// Major mode
bool m_checkOnly; ///< Checking only (no simulation) mode
Expand All @@ -113,8 +138,9 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
int m_dataCount; ///< Bytes of data
AstJumpGo* m_jumpp = nullptr; ///< Jump label we're branching from
// Simulating:
std::unordered_map<const AstNodeDType*, std::deque<AstConst*>>
m_constps; ///< Lists of all AstConst* allocated per dtype
// Allocators for constants of various data types
std::unordered_map<const AstNodeDType*, ConstAllocator> m_constps;
size_t m_constGeneration = 0;
std::vector<SimStackNode*> m_callStack; ///< Call stack for verbose error messages

// Cleanup
Expand Down Expand Up @@ -223,37 +249,8 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
// Simulation METHODS
private:
AstConst* allocConst(AstNode* nodep) {
// Save time - kept a list of allocated but unused values
// It would be more efficient to do this by size, but the extra accounting
// slows things down more than we gain.
AstConst* constp;
// Grab free list corresponding to this dtype
std::deque<AstConst*>& freeList = m_constps[nodep->dtypep()];
bool allocNewConst = true;
if (!freeList.empty()) {
constp = freeList.front();
if (!constp->user1()) {
// Front of free list is free, reuse it (otherwise allocate new node)
allocNewConst = false; // No need to allocate
// Mark the AstConst node as used, and move it to the back of the free list. This
// ensures that when all AstConst instances within the list are used, then the
// front of the list will be marked as used, in which case the enclosing 'if' will
// fail and we fall back to allocation.
constp->user1(1);
freeList.pop_front();
freeList.push_back(constp);
// configure const
constp->num().nodep(nodep);
}
}
if (allocNewConst) {
// Need to allocate new constant
constp = new AstConst{nodep->fileline(), AstConst::DTyped{}, nodep->dtypep()};
// Mark as in use, add to free list for later reuse
constp->user1(1);
freeList.push_back(constp);
}
return constp;
// Allocate a constant with this dtype. Reuse them across a 'clear()' call for efficiency.
return m_constps[nodep->dtypep()].allocate(m_constGeneration, nodep);
}

public:
Expand Down Expand Up @@ -850,6 +847,8 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
if (!itemp) {
clearOptimizable(nodep, "Array initialization has too few elements, need element "
+ cvtToStr(offset));
} else if (AstConst* const constp = VN_CAST(itemp, Const)) {
setValue(nodep, constp);
} else {
setValue(nodep, fetchValue(itemp));
}
Expand Down Expand Up @@ -1256,6 +1255,7 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {

AstNode::user1ClearTree();
m_varAux.clear();
++m_constGeneration;
}
void mainTableCheck(AstNode* nodep) {
setMode(true /*scoped*/, true /*checking*/, false /*params*/);
Expand All @@ -1274,9 +1274,6 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
mainGuts(nodep);
}
~SimulateVisitor() override {
for (const auto& pair : m_constps) {
for (AstConst* const constp : pair.second) delete constp;
}
m_constps.clear();
for (AstNode* ip : m_reclaimValuesp) delete ip;
m_reclaimValuesp.clear();
Expand Down
18 changes: 18 additions & 0 deletions test_regress/t/t_simulate_array.py
Original file line number Diff line number Diff line change
@@ -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('simulator')

test.compile()

test.execute(check_finished=True)

test.passes()
24 changes: 24 additions & 0 deletions test_regress/t/t_simulate_array.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// 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 integer fun;
integer array[0:0];
begin
array[0] = 10;
fun = array[0];
end
endfunction

module test ();
begin
localparam something = fun();
initial begin
if (something !== 10) $stop;
$write("*-* All Finished *-*\n");
$finish;
end
end
endmodule

0 comments on commit 36cc52f

Please sign in to comment.