From 3ccb2e0f2da1186982d46385c5d3c1f35903e4ea Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Fri, 23 Dec 2022 10:51:52 -0500 Subject: [PATCH] Fix initiation of function variables (#3815). --- Changes | 1 + include/verilated.cpp | 15 ++++---- include/verilated_funcs.h | 7 ++-- src/V3AstNodes.cpp | 1 + src/V3CCtors.cpp | 16 ++++++-- src/V3EmitCFunc.cpp | 9 +++-- src/V3Localize.cpp | 1 + src/V3Sched.cpp | 2 + test_regress/t/t_0.pl | 21 ++++++++++ test_regress/t/t_0.v | 60 +++++++++++++++++++++++++++++ test_regress/t/t_xml_debugcheck.out | 18 ++++++--- 11 files changed, 126 insertions(+), 25 deletions(-) create mode 100755 test_regress/t/t_0.pl create mode 100644 test_regress/t/t_0.v diff --git a/Changes b/Changes index 29c11eaa6..150b971e6 100644 --- a/Changes +++ b/Changes @@ -17,6 +17,7 @@ Verilator 5.005 devel * Support Windows-native builds using cmake (#3814). [Kritik Bhimani] * Optimize expansion of extend operators. * Internal multithreading tests. [Mariusz Glebocki, et al, Antmicro Ltd] +* Fix initiation of function variables (#3815). [Dan Gisselquist] * Fix to zero possibly uninitialized bits in replications (#3815). * Fix crash in DFT due to width use after free (#3817) (#3820). [Jevin Sweval] * Fix signed/unsigned comparison compile warning (#3822). [Kamil Rakoczy] diff --git a/include/verilated.cpp b/include/verilated.cpp index f1f477483..e99fcf738 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -355,10 +355,9 @@ WDataOutP VL_RAND_RESET_W(int obits, WDataOutP outwp) VL_MT_SAFE { outwp[VL_WORDS_I(obits) - 1] = VL_RAND_RESET_I(32) & VL_MASK_E(obits); return outwp; } - WDataOutP VL_ZERO_RESET_W(int obits, WDataOutP outwp) VL_MT_SAFE { - for (int i = 0; i < VL_WORDS_I(obits); ++i) outwp[i] = 0; - return outwp; + // Not inlined to speed up compilation of slowpath code + return VL_ZERO_W(obits, outwp); } //=========================================================================== @@ -611,7 +610,7 @@ std::string VL_DECIMAL_NW(int width, const WDataInP lwp) VL_MT_SAFE { const int maxdecwidth = (width + 3) * 4 / 3; // Or (maxdecwidth+7)/8], but can't have more than 4 BCD bits per word VlWide bcd; - VL_ZERO_RESET_W(maxdecwidth, bcd); + VL_ZERO_W(maxdecwidth, bcd); VlWide tmp; VlWide tmp2; int from_bit = width - 1; @@ -622,7 +621,7 @@ std::string VL_DECIMAL_NW(int width, const WDataInP lwp) VL_MT_SAFE { // Any digits >= 5 need an add 3 (via tmp) for (int nibble_bit = 0; nibble_bit < maxdecwidth; nibble_bit += 4) { if ((VL_BITRSHIFT_W(bcd, nibble_bit) & 0xf) >= 5) { - VL_ZERO_RESET_W(maxdecwidth, tmp2); + VL_ZERO_W(maxdecwidth, tmp2); tmp2[VL_BITWORD_E(nibble_bit)] |= VL_EUL(0x3) << VL_BITBIT_E(nibble_bit); VL_ASSIGN_W(maxdecwidth, tmp, bcd); VL_ADD_W(VL_WORDS_I(maxdecwidth), bcd, tmp, tmp2); @@ -1614,7 +1613,7 @@ IData VL_FREAD_I(int width, int array_lsb, int array_size, void* memp, IData fpi *datap |= ((static_cast(c) << static_cast(shift)) & VL_MASK_Q(width)); } else { WDataOutP datap = &(reinterpret_cast(memp))[entry * VL_WORDS_I(width)]; - if (shift == start_shift) VL_ZERO_RESET_W(width, datap); + if (shift == start_shift) VL_ZERO_W(width, datap); datap[VL_BITWORD_E(shift)] |= (static_cast(c) << VL_BITBIT_E(shift)); } // Prep for next @@ -1703,7 +1702,7 @@ IData VL_VALUEPLUSARGS_INW(int rbits, const std::string& ld, WDataOutP rwp) VL_M const char* const dp = match.c_str() + 1 /*leading + */ + prefix.length(); if (match.empty()) return 0; - VL_ZERO_RESET_W(rbits, rwp); + VL_ZERO_W(rbits, rwp); switch (std::tolower(fmt)) { case 'd': { int64_t lld = 0; @@ -2025,7 +2024,7 @@ void VlReadMem::setData(void* valuep, const std::string& rhs) { & VL_MASK_Q(m_bits); } else { WDataOutP datap = reinterpret_cast(valuep); - if (!innum) VL_ZERO_RESET_W(m_bits, datap); + if (!innum) VL_ZERO_W(m_bits, datap); _vl_shiftl_inplace_w(m_bits, datap, static_cast(shift)); datap[0] |= value; } diff --git a/include/verilated_funcs.h b/include/verilated_funcs.h index 57d76ab9a..6e938135d 100644 --- a/include/verilated_funcs.h +++ b/include/verilated_funcs.h @@ -100,12 +100,11 @@ inline IData VL_URANDOM_RANGE_I(IData hi, IData lo) { } } -// These are init time only, so slow is fine -/// Random reset a signal of given width +/// Random reset a signal of given width (init time only) extern IData VL_RAND_RESET_I(int obits) VL_MT_SAFE; -/// Random reset a signal of given width +/// Random reset a signal of given width (init time only) extern QData VL_RAND_RESET_Q(int obits) VL_MT_SAFE; -/// Random reset a signal of given width +/// Random reset a signal of given width (init time only) extern WDataOutP VL_RAND_RESET_W(int obits, WDataOutP outwp) VL_MT_SAFE; /// Zero reset a signal (slow - else use VL_ZERO_W) extern WDataOutP VL_ZERO_RESET_W(int obits, WDataOutP outwp) VL_MT_SAFE; diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 21678f382..578131836 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -2092,6 +2092,7 @@ void AstVar::dump(std::ostream& str) const { if (isSigPublic()) str << " [P]"; if (isLatched()) str << " [LATCHED]"; if (isUsedLoopIdx()) str << " [LOOP]"; + if (noReset()) str << " [!RST]"; if (attrIsolateAssign()) str << " [aISO]"; if (attrFileDescr()) str << " [aFD]"; if (isFuncReturn()) { diff --git a/src/V3CCtors.cpp b/src/V3CCtors.cpp index 733657e34..d1b4804a8 100644 --- a/src/V3CCtors.cpp +++ b/src/V3CCtors.cpp @@ -138,6 +138,7 @@ private: // STATE AstNodeModule* m_modp = nullptr; // Current module + AstCFunc* m_cfuncp = nullptr; // Current function V3CCtorsBuilder* m_varResetp = nullptr; // Builder of _ctor_var_reset // VISITs @@ -172,16 +173,23 @@ private: void visit(AstCFunc* nodep) override { VL_RESTORER(m_varResetp); + VL_RESTORER(m_cfuncp); m_varResetp = nullptr; + m_cfuncp = nodep; iterateChildren(nodep); } void visit(AstVar* nodep) override { - if (m_varResetp && !nodep->isIfaceParent() && !nodep->isIfaceRef() && !nodep->noReset() - && !nodep->isParam() + if (!nodep->isIfaceParent() && !nodep->isIfaceRef() && !nodep->noReset() + && !nodep->isParam() && !nodep->isStatementTemp() && !(nodep->basicp() && (nodep->basicp()->isEvent() || nodep->basicp()->isTriggerVec()))) { - const auto vrefp = new AstVarRef{nodep->fileline(), nodep, VAccess::WRITE}; - m_varResetp->add(new AstCReset{nodep->fileline(), vrefp}); + if (m_varResetp) { + const auto vrefp = new AstVarRef{nodep->fileline(), nodep, VAccess::WRITE}; + m_varResetp->add(new AstCReset{nodep->fileline(), vrefp}); + } else if (m_cfuncp) { + const auto vrefp = new AstVarRef{nodep->fileline(), nodep, VAccess::WRITE}; + nodep->addNextHere(new AstCReset{nodep->fileline(), vrefp}); + } } } diff --git a/src/V3EmitCFunc.cpp b/src/V3EmitCFunc.cpp index 24818d18d..7e02f6a94 100644 --- a/src/V3EmitCFunc.cpp +++ b/src/V3EmitCFunc.cpp @@ -578,8 +578,9 @@ void EmitCFunc::emitSetVarConstant(const string& assignString, AstConst* constp) void EmitCFunc::emitVarReset(AstVar* varp) { AstNodeDType* const dtypep = varp->dtypep()->skipRefp(); - const string varNameProtected - = VN_IS(m_modp, Class) ? varp->nameProtect() : "vlSelf->" + varp->nameProtect(); + const string varNameProtected = (VN_IS(m_modp, Class) || varp->isFuncLocal()) + ? varp->nameProtect() + : "vlSelf->" + varp->nameProtect(); if (varp->isIO() && m_modp->isTop() && optSystemC()) { // System C top I/O doesn't need loading, as the lower level subinst code does it.} } else if (varp->isParam()) { @@ -697,9 +698,11 @@ string EmitCFunc::emitVarResetRecurse(const AstVar* varp, const string& varNameP } else if (basicp) { const bool zeroit = (varp->attrFileDescr() // Zero so we don't do file IO if never $fopen + || varp->isFuncLocal() // Randomization too slow || (basicp && basicp->isZeroInit()) || (v3Global.opt.underlineZero() && !varp->name().empty() && varp->name()[0] == '_') || (v3Global.opt.xInitial() == "fast" || v3Global.opt.xInitial() == "0")); + const bool slow = !varp->isFuncLocal() && !varp->isClassMember(); splitSizeInc(1); if (dtypep->isWide()) { // Handle unpacked; not basicp->isWide string out; @@ -711,7 +714,7 @@ string EmitCFunc::emitVarResetRecurse(const AstVar* varp, const string& varNameP out += cvtToStr(constp->num().edataWord(w)) + "U;\n"; } } else { - out += zeroit ? "VL_ZERO_RESET_W(" : "VL_RAND_RESET_W("; + out += zeroit ? (slow ? "VL_ZERO_RESET_W(" : "VL_ZERO_W(") : "VL_RAND_RESET_W("; out += cvtToStr(dtypep->widthMin()); out += ", " + varNameProtected + suffix + ");\n"; } diff --git a/src/V3Localize.cpp b/src/V3Localize.cpp index 4a30bcc7b..0d7738dc3 100644 --- a/src/V3Localize.cpp +++ b/src/V3Localize.cpp @@ -108,6 +108,7 @@ private: AstVar* const newVarp = new AstVar{oldVarp->fileline(), oldVarp->varType(), newName, oldVarp}; newVarp->funcLocal(true); + newVarp->noReset(oldVarp->noReset()); funcp->addInitsp(newVarp); // Fix up all the references within this function diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index 658243e93..382f9ee17 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -554,6 +554,7 @@ AstNodeStmt* buildLoop(AstNetlist* netlistp, const string& name, FileLine* const flp = scopeTopp->fileline(); // Create the loop condition variable AstVarScope* const condp = scopeTopp->createTemp("__V" + name + "Continue", 1); + condp->varp()->noReset(true); // Initialize the loop condition variable to true AstNodeStmt* const resp = setVar(condp, 1); // Add the loop @@ -578,6 +579,7 @@ std::pair makeEvalLoop(AstNetlist* netlistp, const s FileLine* const flp = scopeTopp->fileline(); AstVarScope* const counterp = scopeTopp->createTemp("__V" + tag + "IterCount", 32); + counterp->varp()->noReset(true); AstNodeStmt* nodep = setVar(counterp, 0); nodep->addNext(buildLoop(netlistp, tag, [&](AstVarScope* continuep, AstWhile* loopp) { diff --git a/test_regress/t/t_0.pl b/test_regress/t/t_0.pl new file mode 100755 index 000000000..1aa73f80a --- /dev/null +++ b/test_regress/t/t_0.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2022 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(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_0.v b/test_regress/t/t_0.v new file mode 100644 index 000000000..176367d64 --- /dev/null +++ b/test_regress/t/t_0.v @@ -0,0 +1,60 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// The code as shown applies a random vector to the Test +// module, then calculates a CRC on the Test module's outputs. +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t(/*AUTOARG*/ + // Inputs + clk + ); + input clk; + parameter W = 104; + + integer cyc = 0; + reg [63:0] crc; + reg [127:0] sum; + wire [127:0] result; + + wire [103:0] in; + reg [103:0] out; + + assign in = {crc[39:0], crc[63:0]}; + + always @(posedge clk) begin + out <= reverse(in); + end + + assign result = {24'h0, out }; + + // Test loop + always @ (posedge clk) begin + $write("[%0t] cyc==%0d crc=%x result=%x in=%x out=%x\n", + $time, cyc, crc, result, in, out); + cyc <= cyc + 1; + crc <= {crc[62:0], crc[63] ^ crc[2] ^ crc[0]}; + sum <= {sum[127:1], 1'b0} + result; + + if (cyc < 10) begin + crc <= 1; + sum <= '0; + end + else if (cyc >= 90) begin + $display("SUM = %x_%x_%x_%x", sum[127:96], + sum[95:64], sum[63:32], sum[31:0]); +`define EXPECTED_SUM 128'h00002d36_42d1a346_8d1a5936_42d1a319 + if (sum !== `EXPECTED_SUM) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + end + + function [W-1:0] reverse(input [W-1:0] val); + integer i; + for (i = 0; i < W; i = i + 1) + reverse[W-1-i] = val[i]; + endfunction +endmodule diff --git a/test_regress/t/t_xml_debugcheck.out b/test_regress/t/t_xml_debugcheck.out index dbadeec0d..ff409ca14 100644 --- a/test_regress/t/t_xml_debugcheck.out +++ b/test_regress/t/t_xml_debugcheck.out @@ -54,7 +54,13 @@ + + + + + + @@ -679,7 +685,13 @@ + + + + + + @@ -1678,12 +1690,6 @@ - - - - - -