From 28b9216f8a490c77d0c0113dd9682bfab231bca4 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 27 Mar 2024 20:07:46 -0400 Subject: [PATCH] Fix tracing class parameters (#5014). --- Changes | 1 + src/V3Class.cpp | 2 +- src/V3TraceDecl.cpp | 29 ++++++++++++++++++++++++++--- src/V3Width.cpp | 6 +++--- test_regress/t/t_trace_class.out | 12 ++++++++++++ test_regress/t/t_trace_class.pl | 25 +++++++++++++++++++++++++ test_regress/t/t_trace_class.v | 32 ++++++++++++++++++++++++++++++++ 7 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 test_regress/t/t_trace_class.out create mode 100755 test_regress/t/t_trace_class.pl create mode 100644 test_regress/t/t_trace_class.v diff --git a/Changes b/Changes index 08a34a3f0..0714788eb 100644 --- a/Changes +++ b/Changes @@ -48,6 +48,7 @@ Verilator 5.023 devel * Fix inout ports of unpacked struct type (#5000). [Ryszard Rozak, Antmicro Ltd.] * Fix `unique {}` constraints missing semicolon (#5001). * Fix preprocessor to respect strings in joins (#5007). +* Fix tracing class parameters (#5014). * Fix $readmem with missing newline (#5019). [Josse Van Delm] * Fix internal error on missing pattern key (#5023). diff --git a/src/V3Class.cpp b/src/V3Class.cpp index fbbfb892b..5d187a5c0 100644 --- a/src/V3Class.cpp +++ b/src/V3Class.cpp @@ -158,7 +158,7 @@ class ClassVisitor final : public VNVisitor { // have a pointer to it yet m_toScopeMoves.emplace_back(nodep, m_packageScopep); } - if (!m_ftaskp && nodep->lifetime().isStatic()) { + if (!m_ftaskp && (nodep->lifetime().isStatic() || nodep->isParam())) { m_toPackageMoves.emplace_back(nodep, m_classPackagep); // We're really moving the VarScope but we might not // have a pointer to it yet diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 4bde0eeaa..d46d97e9b 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -81,8 +81,7 @@ public: // Emit Prefix adjustments to unwind the path back to its original state void unwind() { - unsigned toPop = m_stack.size() - 1; - while (toPop--) m_emit(new AstTracePopPrefix{m_flp}); + for (unsigned toPop = m_stack.size(); --toPop;) m_emit(new AstTracePopPrefix{m_flp}); } }; @@ -98,6 +97,7 @@ class TraceDeclVisitor final : public VNVisitor { std::vector m_topFuncps; // Top level trace initialization functions std::vector m_subFuncps; // Trace sub functions for this scope + std::set m_declUncalledps; // Declarations not called int m_topFuncSize = 0; // Size of the top function currently being built int m_subFuncSize = 0; // Size of the sub function currently being built const int m_funcSizeLimit // Maximum size of a function @@ -232,9 +232,10 @@ class TraceDeclVisitor final : public VNVisitor { } else if (const AstBasicDType* const bdtypep = m_traValuep->dtypep()->basicp()) { bitRange = bdtypep->nrange(); } - auto* const newp + AstTraceDecl* const newp = new AstTraceDecl{m_traVscp->fileline(), m_traName, m_traVscp->varp(), m_traValuep->cloneTree(false), bitRange, arrayRange}; + m_declUncalledps.emplace(newp); addToSubFunc(newp); } @@ -289,6 +290,7 @@ class TraceDeclVisitor final : public VNVisitor { void fixupPlaceholders() { // Fix up cell initialization placehodlers + UINFO(9, "fixupPlaceholders()\n"); for (const auto& item : m_cellInitPlaceholders) { const AstScope* const parentp = std::get<0>(item); const AstCell* const cellp = std::get<1>(item); @@ -325,8 +327,27 @@ class TraceDeclVisitor final : public VNVisitor { } } + void checkCalls(const AstCFunc* funcp) { + if (!v3Global.opt.debugCheck()) return; + checkCallsRecurse(funcp); + if (!m_declUncalledps.empty()) { + for (auto tracep : m_declUncalledps) UINFO(0, "-nodep " << tracep << "\n"); + (*(m_declUncalledps.begin()))->v3fatalSrc("Created TraceDecl which is never called"); + } + } + void checkCallsRecurse(const AstCFunc* funcp) { + funcp->foreach([this](const AstNode* nodep) { + if (const AstTraceDecl* const declp = VN_CAST(nodep, TraceDecl)) { + m_declUncalledps.erase(declp); + } else if (const AstCCall* const ccallp = VN_CAST(nodep, CCall)) { + checkCallsRecurse(ccallp->funcp()); + } + }); + } + // VISITORS void visit(AstScope* nodep) override { + UINFO(9, "visit " << nodep << "\n"); UASSERT_OBJ(!m_currScopep, nodep, "Should not nest"); UASSERT_OBJ(m_subFuncps.empty(), nodep, "Should not nest"); UASSERT_OBJ(m_entries.empty(), nodep, "Should not nest"); @@ -678,6 +699,8 @@ public: // Set name of top level function AstCFunc* const topFuncp = m_topFuncps.front(); topFuncp->name("trace_init_top"); + + checkCalls(topFuncp); } ~TraceDeclVisitor() override { V3Stats::addStat("Tracing, Traced signals", m_statSigs); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 64742726a..7419fcac7 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -2795,9 +2795,9 @@ class WidthVisitor final : public VNVisitor { if (AstNode* const foundp = m_memberMap.findMember(classp, nodep->name())) { if (AstVar* const varp = VN_CAST(foundp, Var)) { if (!varp->didWidth()) userIterate(varp, nullptr); - if (varp->lifetime().isStatic()) { - // Static fiels are moved outside the class, so they shouldn't be accessed - // by member select on a class object + if (varp->lifetime().isStatic() || varp->isParam()) { + // Static members are moved outside the class, so they shouldn't be + // accessed by member select on a class object AstVarRef* const varRefp = new AstVarRef{nodep->fileline(), varp, nodep->access()}; varRefp->classOrPackagep(classp); diff --git a/test_regress/t/t_trace_class.out b/test_regress/t/t_trace_class.out new file mode 100644 index 000000000..41b74f075 --- /dev/null +++ b/test_regress/t/t_trace_class.out @@ -0,0 +1,12 @@ +$version Generated by VerilatedVcd $end +$timescale 1ps $end + $scope module TOP $end + $scope module $unit::Cls__P0__Vclpkg $end + $var wire 32 # PARAM [31:0] $end + $upscope $end + $upscope $end +$enddefinitions $end + + +#0 +b00000000000000000000000000000000 # diff --git a/test_regress/t/t_trace_class.pl b/test_regress/t/t_trace_class.pl new file mode 100755 index 000000000..7d0046bfc --- /dev/null +++ b/test_regress/t/t_trace_class.pl @@ -0,0 +1,25 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 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 + +scenarios(vlt => 1); + +compile( + # verilator_flags2 => ['--binary --trace'], + verilator_flags2 => ['--binary --trace'], + ); + +execute( + check_finished => 1, + ); + +vcd_identical("$Self->{obj_dir}/simx.vcd", $Self->{golden_filename}); + +ok(1); +1; diff --git a/test_regress/t/t_trace_class.v b/test_regress/t/t_trace_class.v new file mode 100644 index 000000000..fdb3438a5 --- /dev/null +++ b/test_regress/t/t_trace_class.v @@ -0,0 +1,32 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +`define STRINGIFY(x) `"x`" + +class Cls #(parameter int PARAM); + static int s_cls_static = 123; +endclass + +module top(); + typedef Cls#(.PARAM(0)) Cls_t; + + Cls_t obj; + + initial begin + obj = new; +`ifdef verilator + obj.s_cls_static = $c("100"); // no-opt +`else + obj.s_cls_static = 100; +`endif + if (obj.s_cls_static != 100) $stop; + if (obj.PARAM != 0) $stop; + $dumpfile(`STRINGIFY(`TEST_DUMPFILE)); + $dumpvars(0); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule