From 7bfc1a00a723cd91d8d52c146b5e7b45844a2ed6 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 14 Apr 2022 09:14:44 -0400 Subject: [PATCH 01/10] Fix tracing interfaces inside interfaces (#3309). --- Changes | 2 +- src/V3Inline.cpp | 9 +- src/V3TraceDecl.cpp | 46 +++-- test_regress/t/t_interface_ref_trace.out | 25 ++- test_regress/t/t_interface_ref_trace.v | 6 + test_regress/t/t_interface_ref_trace_fst.out | 172 ++++++++++-------- .../t/t_interface_ref_trace_fst_sc.out | 172 ++++++++++-------- 7 files changed, 261 insertions(+), 171 deletions(-) diff --git a/Changes b/Changes index 91b355795..12f8458f1 100644 --- a/Changes +++ b/Changes @@ -18,10 +18,10 @@ Verilator 4.221 devel * Fix MSVC localtime_s (#3124). * Fix Bison 3.8.2 error (#3366). [elike-ypq] * Fix rare bug in -Oz (V3Localize) (#3286). [Geza Lore, Shunyao CAD] +* Fix tracing interfaces inside interfaces (#3309). [Kevin Millis] * Fix filenames with dots overwriting debug .vpp files (#3373). * Fix including VK_USER_OBJS in make library (#3370). [Julien Margetts] - Verilator 4.220 2022-03-12 ========================== diff --git a/src/V3Inline.cpp b/src/V3Inline.cpp index eabbca566..2c451c62c 100644 --- a/src/V3Inline.cpp +++ b/src/V3Inline.cpp @@ -647,7 +647,11 @@ private: m_scope += "__DOT__" + nodep->name(); } - if (AstModule* const modp = VN_CAST(nodep->modp(), Module)) { + if (VN_IS(nodep->modp(), Iface)) { + nodep->addIntfRefp(new AstIntfRef{nodep->fileline(), m_scope}); + } + { + AstNodeModule* const modp = nodep->modp(); // Pass Cell pointers down to the next module for (AstPin* pinp = nodep->pinsp(); pinp; pinp = VN_AS(pinp->nextp(), Pin)) { AstVar* const varp = pinp->modVarp(); @@ -666,9 +670,6 @@ private: } iterateChildren(modp); - } else if (VN_IS(nodep->modp(), Iface)) { - nodep->addIntfRefp(new AstIntfRef(nodep->fileline(), m_scope)); - // No need to iterate on interface cells } } virtual void visit(AstAssignVarScope* nodep) override { diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 5859ff819..15669c0d1 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -190,6 +190,27 @@ private: std::string getScopeChar(VltTraceScope sct) { return std::string(1, (char)(0x80 + sct)); } + std::string addAboveInterface(const std::string& scopeName) { + std::string out; + // Hierarchical interfaces didn't know if interface vs module + // above them. so convert a scope string to have the interface character. + // Uses list of scopes to see what's an interface above. + size_t begin = 0; + while (true) { + const size_t end = scopeName.find(' ', begin); + if (end == string::npos) break; + const string& extra = scopeName.substr(begin, end - begin); + out += extra; + if (m_scopeSubFuncps.count(out + getScopeChar(VLT_TRACE_SCOPE_INTERFACE) + " ")) { + out += getScopeChar(VLT_TRACE_SCOPE_INTERFACE) + " "; + } else { + out += " "; + } + begin = end + 1; + } + return out; + } + void addTraceDecl(const VNumRange& arrayRange, int widthOverride) { // If !=0, is packed struct/array where basicp size // misreflects one element @@ -199,8 +220,10 @@ private: } else if (const AstBasicDType* const bdtypep = m_traValuep->dtypep()->basicp()) { bitRange = bdtypep->nrange(); } - addToSubFunc(new AstTraceDecl{m_traVscp->fileline(), m_traName, m_traVscp->varp(), - m_traValuep->cloneTree(false), bitRange, arrayRange}); + auto* const newp + = new AstTraceDecl{m_traVscp->fileline(), m_traName, m_traVscp->varp(), + m_traValuep->cloneTree(false), bitRange, arrayRange}; + addToSubFunc(newp); } void addIgnore(const char* why) { @@ -217,17 +240,14 @@ private: UASSERT_OBJ(!m_traVscp, nodep, "Should not nest"); UASSERT_OBJ(m_traName.empty(), nodep, "Should not nest"); - FileLine* const flp = nodep->fileline(); + VL_RESTORER(m_currScopep); m_currScopep = nodep; // Gather all signals under this AstScope iterateChildrenConst(nodep); // If nothing to trace in this scope, then job done - if (m_signals.empty()) { - m_currScopep = nullptr; - return; - } + if (m_signals.empty()) return; // Sort signals, first by enclosing instance, then by source location, then by name std::stable_sort(m_signals.begin(), m_signals.end(), [](const Signal& a, const Signal& b) { @@ -239,6 +259,7 @@ private: }); // Build trace initialization functions for this AstScope + FileLine* const flp = nodep->fileline(); PathAdjustor pathAdjustor{flp, [&](AstNodeStmt* stmtp) { addToSubFunc(stmtp); }}; for (const Signal& signal : m_signals) { // Adjust name prefix based on path in hierarchy @@ -278,6 +299,7 @@ private: scopeName = scopeName.substr(0, lastDot + 1); const size_t scopeLen = scopeName.length(); + UASSERT_OBJ(cellp->intfRefp(), cellp, "Interface without tracing reference"); for (AstIntfRef *irp = cellp->intfRefp(), *nextIrp; irp; irp = nextIrp) { nextIrp = VN_AS(irp->nextp(), IntfRef); @@ -288,6 +310,9 @@ private: string scopeName = AstNode::vcdName(irp->name()); if (scopeName.substr(0, 4) == "TOP ") scopeName.erase(0, 4); + // Note this insert doesn't know what above is interfaces. + // Perhaps all scopes should be changed to include the VLT_TRACE_SCOPE characters. + // Instead we fix up when printing m_scopeSubFuncps scopeName += getScopeChar(VLT_TRACE_SCOPE_INTERFACE) + ' '; m_scopeSubFuncps.emplace(scopeName, m_subFuncps); @@ -300,8 +325,6 @@ private: if (VString::startsWith(scopeName, "TOP ")) scopeName.erase(0, 4); m_scopeSubFuncps.emplace(scopeName, std::move(m_subFuncps)); } - - m_currScopep = nullptr; } virtual void visit(AstVarScope* nodep) override { UASSERT_OBJ(m_currScopep, nodep, "AstVarScope not under AstScope"); @@ -454,9 +477,10 @@ public: // Build top level trace initialization functions PathAdjustor pathAdjustor{flp, [&](AstNodeStmt* stmtp) { addToTopFunc(stmtp); }}; for (const auto& item : m_scopeSubFuncps) { + const std::string scopeName = item.first; + const std::string scopeNameInterfaced = addAboveInterface(scopeName); // Adjust name prefix based on path in hierarchy - pathAdjustor.adjust(item.first); - + pathAdjustor.adjust(scopeNameInterfaced); // Call all sub functions for this path for (AstCFunc* const subFuncp : item.second) { AstCCall* const callp = new AstCCall{flp, subFuncp}; diff --git a/test_regress/t/t_interface_ref_trace.out b/test_regress/t/t_interface_ref_trace.out index 67a6e327b..5e2a4c6b1 100644 --- a/test_regress/t/t_interface_ref_trace.out +++ b/test_regress/t/t_interface_ref_trace.out @@ -1,7 +1,6 @@ $version Generated by VerilatedVcd $end -$date Wed Dec 4 07:47:51 2019 - $end -$timescale 1ps $end +$date Thu Apr 14 07:06:40 2022 $end +$timescale 1ps $end $scope module top $end $var wire 1 0 clk $end @@ -57,6 +56,10 @@ $timescale 1ps $end $var wire 1 0 clk $end $var wire 32 # cyc [31:0] $end $var wire 32 * value [31:0] $end + $scope interface inner $end + $var wire 32 # cyc [31:0] $end + $var wire 32 3 value [31:0] $end + $upscope $end $scope struct the_struct $end $var wire 32 + val100 [31:0] $end $var wire 32 , val200 [31:0] $end @@ -130,6 +133,10 @@ $timescale 1ps $end $var wire 1 0 clk $end $var wire 32 # cyc [31:0] $end $var wire 32 - value [31:0] $end + $scope interface inner $end + $var wire 32 # cyc [31:0] $end + $var wire 32 4 value [31:0] $end + $upscope $end $scope struct the_struct $end $var wire 32 . val100 [31:0] $end $var wire 32 / val200 [31:0] $end @@ -180,6 +187,10 @@ $timescale 1ps $end $var wire 1 0 clk $end $var wire 32 # cyc [31:0] $end $var wire 32 $ value [31:0] $end + $scope interface inner $end + $var wire 32 # cyc [31:0] $end + $var wire 32 1 value [31:0] $end + $upscope $end $scope struct the_struct $end $var wire 32 % val100 [31:0] $end $var wire 32 & val200 [31:0] $end @@ -189,6 +200,10 @@ $timescale 1ps $end $var wire 1 0 clk $end $var wire 32 # cyc [31:0] $end $var wire 32 ' value [31:0] $end + $scope interface inner $end + $var wire 32 # cyc [31:0] $end + $var wire 32 2 value [31:0] $end + $upscope $end $scope struct the_struct $end $var wire 32 ( val100 [31:0] $end $var wire 32 ) val200 [31:0] $end @@ -236,6 +251,10 @@ b00000000000000000000001111101010 - b00000000000000000000010001001110 . b00000000000000000000010010110010 / 00 +b00000000000000000000000000000000 1 +b00000000000000000000000000000000 2 +b00000000000000000000000000000000 3 +b00000000000000000000000000000000 4 #10 b00000000000000000000000000000001 # b00000000000000000000000000000010 $ diff --git a/test_regress/t/t_interface_ref_trace.v b/test_regress/t/t_interface_ref_trace.v index a3fabda04..0f2577d47 100644 --- a/test_regress/t/t_interface_ref_trace.v +++ b/test_regress/t/t_interface_ref_trace.v @@ -11,10 +11,16 @@ typedef struct packed { integer val200; } struct_t; +// This interface is not connected to any cells +interface ifc_inner(input integer cyc); + integer value; +endinterface + interface ifc (input logic clk, input integer cyc); integer value; struct_t the_struct; + ifc_inner inner (.*); endinterface module t (/*AUTOARG*/ diff --git a/test_regress/t/t_interface_ref_trace_fst.out b/test_regress/t/t_interface_ref_trace_fst.out index f26bb0647..858d03801 100644 --- a/test_regress/t/t_interface_ref_trace_fst.out +++ b/test_regress/t/t_interface_ref_trace_fst.out @@ -1,5 +1,5 @@ $date - Tue Feb 22 23:55:07 2022 + Thu Apr 14 07:06:50 2022 $end $version @@ -59,6 +59,10 @@ $upscope $end $upscope $end $upscope $end $scope interface intf_in_sub_all $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 , value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end $var integer 32 ) value [31:0] $end @@ -113,10 +117,10 @@ $scope module ac3 $end $scope interface intf_for_check $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end -$var integer 32 , value [31:0] $end +$var integer 32 - value [31:0] $end $scope struct the_struct $end -$var logic 32 - val100 [31:0] $end -$var logic 32 . val200 [31:0] $end +$var logic 32 . val100 [31:0] $end +$var logic 32 / val200 [31:0] $end $upscope $end $upscope $end $upscope $end @@ -124,20 +128,24 @@ $scope module as3 $end $scope interface intf_for_struct $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end -$var integer 32 , value [31:0] $end +$var integer 32 - value [31:0] $end $scope struct the_struct $end -$var logic 32 - val100 [31:0] $end -$var logic 32 . val200 [31:0] $end +$var logic 32 . val100 [31:0] $end +$var logic 32 / val200 [31:0] $end $upscope $end $upscope $end $upscope $end $scope interface intf_in_sub_all $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 0 value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end -$var integer 32 , value [31:0] $end +$var integer 32 - value [31:0] $end $scope struct the_struct $end -$var logic 32 - val100 [31:0] $end -$var logic 32 . val200 [31:0] $end +$var logic 32 . val100 [31:0] $end +$var logic 32 / val200 [31:0] $end $upscope $end $upscope $end $scope interface intf_one $end @@ -182,6 +190,10 @@ $upscope $end $upscope $end $upscope $end $scope interface intf_1 $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 1 value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end $var integer 32 # value [31:0] $end @@ -191,6 +203,10 @@ $var logic 32 % val200 [31:0] $end $upscope $end $upscope $end $scope interface intf_2 $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 2 value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end $var integer 32 & value [31:0] $end @@ -226,9 +242,13 @@ $upscope $end $enddefinitions $end #0 $dumpvars -b00000000000000000000010010110010 . -b00000000000000000000010001001110 - -b00000000000000000000001111101010 , +b00000000000000000000000000000000 2 +b00000000000000000000000000000000 1 +b00000000000000000000000000000000 0 +b00000000000000000000010010110010 / +b00000000000000000000010001001110 . +b00000000000000000000001111101010 - +b00000000000000000000000000000000 , b00000000000000000000010010110001 + b00000000000000000000010001001101 * b00000000000000000000001111101001 ) @@ -253,16 +273,16 @@ b00000000000000000000000011001011 ( b00000000000000000000001111101010 ) b00000000000000000000010001001110 * b00000000000000000000010010110010 + -b00000000000000000000001111101011 , -b00000000000000000000010001001111 - -b00000000000000000000010010110011 . +b00000000000000000000001111101011 - +b00000000000000000000010001001111 . +b00000000000000000000010010110011 / #15 0! #20 1! -b00000000000000000000010010110100 . -b00000000000000000000010001010000 - -b00000000000000000000001111101100 , +b00000000000000000000010010110100 / +b00000000000000000000010001010000 . +b00000000000000000000001111101100 - b00000000000000000000010010110011 + b00000000000000000000010001001111 * b00000000000000000000001111101011 ) @@ -287,16 +307,16 @@ b00000000000000000000000011001101 ( b00000000000000000000001111101100 ) b00000000000000000000010001010000 * b00000000000000000000010010110100 + -b00000000000000000000001111101101 , -b00000000000000000000010001010001 - -b00000000000000000000010010110101 . +b00000000000000000000001111101101 - +b00000000000000000000010001010001 . +b00000000000000000000010010110101 / #35 0! #40 1! -b00000000000000000000010010110110 . -b00000000000000000000010001010010 - -b00000000000000000000001111101110 , +b00000000000000000000010010110110 / +b00000000000000000000010001010010 . +b00000000000000000000001111101110 - b00000000000000000000010010110101 + b00000000000000000000010001010001 * b00000000000000000000001111101101 ) @@ -321,16 +341,16 @@ b00000000000000000000000011001111 ( b00000000000000000000001111101110 ) b00000000000000000000010001010010 * b00000000000000000000010010110110 + -b00000000000000000000001111101111 , -b00000000000000000000010001010011 - -b00000000000000000000010010110111 . +b00000000000000000000001111101111 - +b00000000000000000000010001010011 . +b00000000000000000000010010110111 / #55 0! #60 1! -b00000000000000000000010010111000 . -b00000000000000000000010001010100 - -b00000000000000000000001111110000 , +b00000000000000000000010010111000 / +b00000000000000000000010001010100 . +b00000000000000000000001111110000 - b00000000000000000000010010110111 + b00000000000000000000010001010011 * b00000000000000000000001111101111 ) @@ -355,16 +375,16 @@ b00000000000000000000000011010001 ( b00000000000000000000001111110000 ) b00000000000000000000010001010100 * b00000000000000000000010010111000 + -b00000000000000000000001111110001 , -b00000000000000000000010001010101 - -b00000000000000000000010010111001 . +b00000000000000000000001111110001 - +b00000000000000000000010001010101 . +b00000000000000000000010010111001 / #75 0! #80 1! -b00000000000000000000010010111010 . -b00000000000000000000010001010110 - -b00000000000000000000001111110010 , +b00000000000000000000010010111010 / +b00000000000000000000010001010110 . +b00000000000000000000001111110010 - b00000000000000000000010010111001 + b00000000000000000000010001010101 * b00000000000000000000001111110001 ) @@ -389,16 +409,16 @@ b00000000000000000000000011010011 ( b00000000000000000000001111110010 ) b00000000000000000000010001010110 * b00000000000000000000010010111010 + -b00000000000000000000001111110011 , -b00000000000000000000010001010111 - -b00000000000000000000010010111011 . +b00000000000000000000001111110011 - +b00000000000000000000010001010111 . +b00000000000000000000010010111011 / #95 0! #100 1! -b00000000000000000000010010111100 . -b00000000000000000000010001011000 - -b00000000000000000000001111110100 , +b00000000000000000000010010111100 / +b00000000000000000000010001011000 . +b00000000000000000000001111110100 - b00000000000000000000010010111011 + b00000000000000000000010001010111 * b00000000000000000000001111110011 ) @@ -423,16 +443,16 @@ b00000000000000000000000011010101 ( b00000000000000000000001111110100 ) b00000000000000000000010001011000 * b00000000000000000000010010111100 + -b00000000000000000000001111110101 , -b00000000000000000000010001011001 - -b00000000000000000000010010111101 . +b00000000000000000000001111110101 - +b00000000000000000000010001011001 . +b00000000000000000000010010111101 / #115 0! #120 1! -b00000000000000000000010010111110 . -b00000000000000000000010001011010 - -b00000000000000000000001111110110 , +b00000000000000000000010010111110 / +b00000000000000000000010001011010 . +b00000000000000000000001111110110 - b00000000000000000000010010111101 + b00000000000000000000010001011001 * b00000000000000000000001111110101 ) @@ -457,16 +477,16 @@ b00000000000000000000000011010111 ( b00000000000000000000001111110110 ) b00000000000000000000010001011010 * b00000000000000000000010010111110 + -b00000000000000000000001111110111 , -b00000000000000000000010001011011 - -b00000000000000000000010010111111 . +b00000000000000000000001111110111 - +b00000000000000000000010001011011 . +b00000000000000000000010010111111 / #135 0! #140 1! -b00000000000000000000010011000000 . -b00000000000000000000010001011100 - -b00000000000000000000001111111000 , +b00000000000000000000010011000000 / +b00000000000000000000010001011100 . +b00000000000000000000001111111000 - b00000000000000000000010010111111 + b00000000000000000000010001011011 * b00000000000000000000001111110111 ) @@ -491,16 +511,16 @@ b00000000000000000000000011011001 ( b00000000000000000000001111111000 ) b00000000000000000000010001011100 * b00000000000000000000010011000000 + -b00000000000000000000001111111001 , -b00000000000000000000010001011101 - -b00000000000000000000010011000001 . +b00000000000000000000001111111001 - +b00000000000000000000010001011101 . +b00000000000000000000010011000001 / #155 0! #160 1! -b00000000000000000000010011000010 . -b00000000000000000000010001011110 - -b00000000000000000000001111111010 , +b00000000000000000000010011000010 / +b00000000000000000000010001011110 . +b00000000000000000000001111111010 - b00000000000000000000010011000001 + b00000000000000000000010001011101 * b00000000000000000000001111111001 ) @@ -525,16 +545,16 @@ b00000000000000000000000011011011 ( b00000000000000000000001111111010 ) b00000000000000000000010001011110 * b00000000000000000000010011000010 + -b00000000000000000000001111111011 , -b00000000000000000000010001011111 - -b00000000000000000000010011000011 . +b00000000000000000000001111111011 - +b00000000000000000000010001011111 . +b00000000000000000000010011000011 / #175 0! #180 1! -b00000000000000000000010011000100 . -b00000000000000000000010001100000 - -b00000000000000000000001111111100 , +b00000000000000000000010011000100 / +b00000000000000000000010001100000 . +b00000000000000000000001111111100 - b00000000000000000000010011000011 + b00000000000000000000010001011111 * b00000000000000000000001111111011 ) @@ -559,16 +579,16 @@ b00000000000000000000000011011101 ( b00000000000000000000001111111100 ) b00000000000000000000010001100000 * b00000000000000000000010011000100 + -b00000000000000000000001111111101 , -b00000000000000000000010001100001 - -b00000000000000000000010011000101 . +b00000000000000000000001111111101 - +b00000000000000000000010001100001 . +b00000000000000000000010011000101 / #195 0! #200 1! -b00000000000000000000010011000110 . -b00000000000000000000010001100010 - -b00000000000000000000001111111110 , +b00000000000000000000010011000110 / +b00000000000000000000010001100010 . +b00000000000000000000001111111110 - b00000000000000000000010011000101 + b00000000000000000000010001100001 * b00000000000000000000001111111101 ) @@ -593,6 +613,6 @@ b00000000000000000000000011011111 ( b00000000000000000000001111111110 ) b00000000000000000000010001100010 * b00000000000000000000010011000110 + -b00000000000000000000001111111111 , -b00000000000000000000010001100011 - -b00000000000000000000010011000111 . +b00000000000000000000001111111111 - +b00000000000000000000010001100011 . +b00000000000000000000010011000111 / diff --git a/test_regress/t/t_interface_ref_trace_fst_sc.out b/test_regress/t/t_interface_ref_trace_fst_sc.out index dbbfe4f3b..7131845ae 100644 --- a/test_regress/t/t_interface_ref_trace_fst_sc.out +++ b/test_regress/t/t_interface_ref_trace_fst_sc.out @@ -1,5 +1,5 @@ $date - Tue Feb 22 23:55:19 2022 + Thu Apr 14 07:06:59 2022 $end $version @@ -58,6 +58,10 @@ $upscope $end $upscope $end $upscope $end $scope interface intf_in_sub_all $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 , value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end $var integer 32 ) value [31:0] $end @@ -112,10 +116,10 @@ $scope module ac3 $end $scope interface intf_for_check $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end -$var integer 32 , value [31:0] $end +$var integer 32 - value [31:0] $end $scope struct the_struct $end -$var logic 32 - val100 [31:0] $end -$var logic 32 . val200 [31:0] $end +$var logic 32 . val100 [31:0] $end +$var logic 32 / val200 [31:0] $end $upscope $end $upscope $end $upscope $end @@ -123,20 +127,24 @@ $scope module as3 $end $scope interface intf_for_struct $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end -$var integer 32 , value [31:0] $end +$var integer 32 - value [31:0] $end $scope struct the_struct $end -$var logic 32 - val100 [31:0] $end -$var logic 32 . val200 [31:0] $end +$var logic 32 . val100 [31:0] $end +$var logic 32 / val200 [31:0] $end $upscope $end $upscope $end $upscope $end $scope interface intf_in_sub_all $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 0 value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end -$var integer 32 , value [31:0] $end +$var integer 32 - value [31:0] $end $scope struct the_struct $end -$var logic 32 - val100 [31:0] $end -$var logic 32 . val200 [31:0] $end +$var logic 32 . val100 [31:0] $end +$var logic 32 / val200 [31:0] $end $upscope $end $upscope $end $scope interface intf_one $end @@ -181,6 +189,10 @@ $upscope $end $upscope $end $upscope $end $scope interface intf_1 $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 1 value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end $var integer 32 # value [31:0] $end @@ -190,6 +202,10 @@ $var logic 32 % val200 [31:0] $end $upscope $end $upscope $end $scope interface intf_2 $end +$scope interface inner $end +$var wire 32 " cyc [31:0] $end +$var integer 32 2 value [31:0] $end +$upscope $end $var wire 1 ! clk $end $var wire 32 " cyc [31:0] $end $var integer 32 & value [31:0] $end @@ -225,9 +241,13 @@ $upscope $end $enddefinitions $end #0 $dumpvars -b00000000000000000000010010110010 . -b00000000000000000000010001001110 - -b00000000000000000000001111101010 , +b00000000000000000000000000000000 2 +b00000000000000000000000000000000 1 +b00000000000000000000000000000000 0 +b00000000000000000000010010110010 / +b00000000000000000000010001001110 . +b00000000000000000000001111101010 - +b00000000000000000000000000000000 , b00000000000000000000010010110001 + b00000000000000000000010001001101 * b00000000000000000000001111101001 ) @@ -252,9 +272,9 @@ b00000000000000000000000011001011 ( b00000000000000000000001111101010 ) b00000000000000000000010001001110 * b00000000000000000000010010110010 + -b00000000000000000000001111101011 , -b00000000000000000000010001001111 - -b00000000000000000000010010110011 . +b00000000000000000000001111101011 - +b00000000000000000000010001001111 . +b00000000000000000000010010110011 / #11 #12 #13 @@ -267,9 +287,9 @@ b00000000000000000000010010110011 . #19 #20 1! -b00000000000000000000010010110100 . -b00000000000000000000010001010000 - -b00000000000000000000001111101100 , +b00000000000000000000010010110100 / +b00000000000000000000010001010000 . +b00000000000000000000001111101100 - b00000000000000000000010010110011 + b00000000000000000000010001001111 * b00000000000000000000001111101011 ) @@ -302,9 +322,9 @@ b00000000000000000000000011001101 ( b00000000000000000000001111101100 ) b00000000000000000000010001010000 * b00000000000000000000010010110100 + -b00000000000000000000001111101101 , -b00000000000000000000010001010001 - -b00000000000000000000010010110101 . +b00000000000000000000001111101101 - +b00000000000000000000010001010001 . +b00000000000000000000010010110101 / #31 #32 #33 @@ -317,9 +337,9 @@ b00000000000000000000010010110101 . #39 #40 1! -b00000000000000000000010010110110 . -b00000000000000000000010001010010 - -b00000000000000000000001111101110 , +b00000000000000000000010010110110 / +b00000000000000000000010001010010 . +b00000000000000000000001111101110 - b00000000000000000000010010110101 + b00000000000000000000010001010001 * b00000000000000000000001111101101 ) @@ -352,9 +372,9 @@ b00000000000000000000000011001111 ( b00000000000000000000001111101110 ) b00000000000000000000010001010010 * b00000000000000000000010010110110 + -b00000000000000000000001111101111 , -b00000000000000000000010001010011 - -b00000000000000000000010010110111 . +b00000000000000000000001111101111 - +b00000000000000000000010001010011 . +b00000000000000000000010010110111 / #51 #52 #53 @@ -367,9 +387,9 @@ b00000000000000000000010010110111 . #59 #60 1! -b00000000000000000000010010111000 . -b00000000000000000000010001010100 - -b00000000000000000000001111110000 , +b00000000000000000000010010111000 / +b00000000000000000000010001010100 . +b00000000000000000000001111110000 - b00000000000000000000010010110111 + b00000000000000000000010001010011 * b00000000000000000000001111101111 ) @@ -402,9 +422,9 @@ b00000000000000000000000011010001 ( b00000000000000000000001111110000 ) b00000000000000000000010001010100 * b00000000000000000000010010111000 + -b00000000000000000000001111110001 , -b00000000000000000000010001010101 - -b00000000000000000000010010111001 . +b00000000000000000000001111110001 - +b00000000000000000000010001010101 . +b00000000000000000000010010111001 / #71 #72 #73 @@ -417,9 +437,9 @@ b00000000000000000000010010111001 . #79 #80 1! -b00000000000000000000010010111010 . -b00000000000000000000010001010110 - -b00000000000000000000001111110010 , +b00000000000000000000010010111010 / +b00000000000000000000010001010110 . +b00000000000000000000001111110010 - b00000000000000000000010010111001 + b00000000000000000000010001010101 * b00000000000000000000001111110001 ) @@ -452,9 +472,9 @@ b00000000000000000000000011010011 ( b00000000000000000000001111110010 ) b00000000000000000000010001010110 * b00000000000000000000010010111010 + -b00000000000000000000001111110011 , -b00000000000000000000010001010111 - -b00000000000000000000010010111011 . +b00000000000000000000001111110011 - +b00000000000000000000010001010111 . +b00000000000000000000010010111011 / #91 #92 #93 @@ -467,9 +487,9 @@ b00000000000000000000010010111011 . #99 #100 1! -b00000000000000000000010010111100 . -b00000000000000000000010001011000 - -b00000000000000000000001111110100 , +b00000000000000000000010010111100 / +b00000000000000000000010001011000 . +b00000000000000000000001111110100 - b00000000000000000000010010111011 + b00000000000000000000010001010111 * b00000000000000000000001111110011 ) @@ -502,9 +522,9 @@ b00000000000000000000000011010101 ( b00000000000000000000001111110100 ) b00000000000000000000010001011000 * b00000000000000000000010010111100 + -b00000000000000000000001111110101 , -b00000000000000000000010001011001 - -b00000000000000000000010010111101 . +b00000000000000000000001111110101 - +b00000000000000000000010001011001 . +b00000000000000000000010010111101 / #111 #112 #113 @@ -517,9 +537,9 @@ b00000000000000000000010010111101 . #119 #120 1! -b00000000000000000000010010111110 . -b00000000000000000000010001011010 - -b00000000000000000000001111110110 , +b00000000000000000000010010111110 / +b00000000000000000000010001011010 . +b00000000000000000000001111110110 - b00000000000000000000010010111101 + b00000000000000000000010001011001 * b00000000000000000000001111110101 ) @@ -552,9 +572,9 @@ b00000000000000000000000011010111 ( b00000000000000000000001111110110 ) b00000000000000000000010001011010 * b00000000000000000000010010111110 + -b00000000000000000000001111110111 , -b00000000000000000000010001011011 - -b00000000000000000000010010111111 . +b00000000000000000000001111110111 - +b00000000000000000000010001011011 . +b00000000000000000000010010111111 / #131 #132 #133 @@ -567,9 +587,9 @@ b00000000000000000000010010111111 . #139 #140 1! -b00000000000000000000010011000000 . -b00000000000000000000010001011100 - -b00000000000000000000001111111000 , +b00000000000000000000010011000000 / +b00000000000000000000010001011100 . +b00000000000000000000001111111000 - b00000000000000000000010010111111 + b00000000000000000000010001011011 * b00000000000000000000001111110111 ) @@ -602,9 +622,9 @@ b00000000000000000000000011011001 ( b00000000000000000000001111111000 ) b00000000000000000000010001011100 * b00000000000000000000010011000000 + -b00000000000000000000001111111001 , -b00000000000000000000010001011101 - -b00000000000000000000010011000001 . +b00000000000000000000001111111001 - +b00000000000000000000010001011101 . +b00000000000000000000010011000001 / #151 #152 #153 @@ -617,9 +637,9 @@ b00000000000000000000010011000001 . #159 #160 1! -b00000000000000000000010011000010 . -b00000000000000000000010001011110 - -b00000000000000000000001111111010 , +b00000000000000000000010011000010 / +b00000000000000000000010001011110 . +b00000000000000000000001111111010 - b00000000000000000000010011000001 + b00000000000000000000010001011101 * b00000000000000000000001111111001 ) @@ -652,9 +672,9 @@ b00000000000000000000000011011011 ( b00000000000000000000001111111010 ) b00000000000000000000010001011110 * b00000000000000000000010011000010 + -b00000000000000000000001111111011 , -b00000000000000000000010001011111 - -b00000000000000000000010011000011 . +b00000000000000000000001111111011 - +b00000000000000000000010001011111 . +b00000000000000000000010011000011 / #171 #172 #173 @@ -667,9 +687,9 @@ b00000000000000000000010011000011 . #179 #180 1! -b00000000000000000000010011000100 . -b00000000000000000000010001100000 - -b00000000000000000000001111111100 , +b00000000000000000000010011000100 / +b00000000000000000000010001100000 . +b00000000000000000000001111111100 - b00000000000000000000010011000011 + b00000000000000000000010001011111 * b00000000000000000000001111111011 ) @@ -702,9 +722,9 @@ b00000000000000000000000011011101 ( b00000000000000000000001111111100 ) b00000000000000000000010001100000 * b00000000000000000000010011000100 + -b00000000000000000000001111111101 , -b00000000000000000000010001100001 - -b00000000000000000000010011000101 . +b00000000000000000000001111111101 - +b00000000000000000000010001100001 . +b00000000000000000000010011000101 / #191 #192 #193 @@ -717,9 +737,9 @@ b00000000000000000000010011000101 . #199 #200 1! -b00000000000000000000010011000110 . -b00000000000000000000010001100010 - -b00000000000000000000001111111110 , +b00000000000000000000010011000110 / +b00000000000000000000010001100010 . +b00000000000000000000001111111110 - b00000000000000000000010011000101 + b00000000000000000000010001100001 * b00000000000000000000001111111101 ) @@ -752,9 +772,9 @@ b00000000000000000000000011011111 ( b00000000000000000000001111111110 ) b00000000000000000000010001100010 * b00000000000000000000010011000110 + -b00000000000000000000001111111111 , -b00000000000000000000010001100011 - -b00000000000000000000010011000111 . +b00000000000000000000001111111111 - +b00000000000000000000010001100011 . +b00000000000000000000010011000111 / #211 #212 #213 From 880a9be3b10763b1d0b68414c1fa02700b3377d2 Mon Sep 17 00:00:00 2001 From: HungMingWu Date: Tue, 19 Apr 2022 01:03:56 +0800 Subject: [PATCH 02/10] Internal: Add C++20ish reverse_view for range loops. No functional change (#3388). Signed-off-by: HungMingWu --- include/verilated_types.h | 8 ++++---- include/verilatedos.h | 13 +++++++++++++ src/V3EmitMk.cpp | 4 ++-- src/V3Inline.cpp | 3 +-- src/V3LinkJump.cpp | 8 ++++---- src/V3ParseSym.h | 3 +-- src/V3Partition.cpp | 12 ++++++------ src/V3Simulate.h | 6 +++--- 8 files changed, 34 insertions(+), 23 deletions(-) diff --git a/include/verilated_types.h b/include/verilated_types.h index 43bad0b4b..8e8da1941 100644 --- a/include/verilated_types.h +++ b/include/verilated_types.h @@ -410,16 +410,16 @@ public: } template VlQueue find_last(Func with_func) const { IData index = m_deque.size() - 1; - for (auto it = m_deque.rbegin(); it != m_deque.rend(); ++it) { - if (with_func(index, *it)) return VlQueue::cons(*it); + for (auto& item : vlstd::reverse_view(m_deque)) { + if (with_func(index, item)) return VlQueue::cons(item); --index; } return VlQueue{}; } template VlQueue find_last_index(Func with_func) const { IData index = m_deque.size() - 1; - for (auto it = m_deque.rbegin(); it != m_deque.rend(); ++it) { - if (with_func(index, *it)) return VlQueue::cons(index); + for (auto& item : vlstd::reverse_view(m_deque)) { + if (with_func(index, item)) return VlQueue::cons(index); --index; } return VlQueue{}; diff --git a/include/verilatedos.h b/include/verilatedos.h index 797ffab99..28412cac4 100644 --- a/include/verilatedos.h +++ b/include/verilatedos.h @@ -521,6 +521,19 @@ using ssize_t = uint32_t; ///< signed size_t; returned from read() // Conversions namespace vlstd { + +template struct reverse_wrapper { + const T& m_v; + + explicit reverse_wrapper(const T& a_v) + : m_v(a_v) {} + inline auto begin() -> decltype(m_v.rbegin()) { return m_v.rbegin(); } + inline auto end() -> decltype(m_v.rend()) { return m_v.rend(); } +}; + +// C++20's std::ranges::reverse_view +template reverse_wrapper reverse_view(const T& v) { return reverse_wrapper(v); } + // C++17's std::as_const template T const& as_const(T& v) { return v; } }; // namespace vlstd diff --git a/src/V3EmitMk.cpp b/src/V3EmitMk.cpp index a39c3f3ee..ce75cb136 100644 --- a/src/V3EmitMk.cpp +++ b/src/V3EmitMk.cpp @@ -333,8 +333,8 @@ class EmitMkHierVerilation final { const V3HierBlockPlan::HierVector blocks = m_planp->hierBlocksSorted(); // leaf comes first // List in order of leaf-last order so that linker can resolve dependency - for (auto it = blocks.rbegin(); it != blocks.rend(); ++it) { - of.puts("\t" + (*it)->hierLib(true) + " \\\n"); + for (auto& block : vlstd::reverse_view(blocks)) { + of.puts("\t" + block->hierLib(true) + " \\\n"); } of.puts("\n"); diff --git a/src/V3Inline.cpp b/src/V3Inline.cpp index 2c451c62c..c882b10c3 100644 --- a/src/V3Inline.cpp +++ b/src/V3Inline.cpp @@ -190,8 +190,7 @@ private: // Iterate through all modules in bottom-up order. // Make a final inlining decision for each. - for (auto it = m_allMods.rbegin(); it != m_allMods.rend(); ++it) { - AstNodeModule* const modp = *it; + for (AstNodeModule* const modp : vlstd::reverse_view(m_allMods)) { // If we're going to inline some modules into this one, // update user4 (statement count) to reflect that: diff --git a/src/V3LinkJump.cpp b/src/V3LinkJump.cpp index c46f2f82a..f7f68c28a 100644 --- a/src/V3LinkJump.cpp +++ b/src/V3LinkJump.cpp @@ -263,10 +263,10 @@ private: UINFO(8, " DISABLE " << nodep << endl); iterateChildren(nodep); AstNodeBlock* blockp = nullptr; - for (auto it = m_blockStack.rbegin(); it != m_blockStack.rend(); ++it) { - UINFO(9, " UNDERBLK " << *it << endl); - if ((*it)->name() == nodep->name()) { - blockp = *it; + for (AstNodeBlock* const stackp : vlstd::reverse_view(m_blockStack)) { + UINFO(9, " UNDERBLK " << stackp << endl); + if (stackp->name() == nodep->name()) { + blockp = stackp; break; } } diff --git a/src/V3ParseSym.h b/src/V3ParseSym.h index 4da0c735c..998dac016 100644 --- a/src/V3ParseSym.h +++ b/src/V3ParseSym.h @@ -128,8 +128,7 @@ public: } void showUpward() { // LCOV_EXCL_START UINFO(1, "ParseSym Stack:\n"); - for (auto it = m_sympStack.rbegin(); it != m_sympStack.rend(); ++it) { - VSymEnt* const symp = *it; + for (VSymEnt* const symp : vlstd::reverse_view(m_sympStack)) { UINFO(1, " " << symp->nodep() << endl); } UINFO(1, "ParseSym Current: " << symCurrentp()->nodep() << endl); diff --git a/src/V3Partition.cpp b/src/V3Partition.cpp index 18ca601d7..a4177c214 100644 --- a/src/V3Partition.cpp +++ b/src/V3Partition.cpp @@ -526,9 +526,9 @@ public: } void checkRelativesCp(GraphWay way) const { const EdgeSet& edges = m_edges[way]; - for (EdgeSet::const_reverse_iterator it = edges.rbegin(); it != edges.rend(); ++it) { - const LogicMTask* const relativep = (*it).key(); - const uint32_t cachedCp = (*it).value(); + for (const auto& edge : vlstd::reverse_view(edges)) { + const LogicMTask* const relativep = edge.key(); + const uint32_t cachedCp = edge.value(); partCheckCachedScoreVsActual(cachedCp, relativep->critPathCost(way.invert()) + relativep->stepCost()); } @@ -555,12 +555,12 @@ public: // wayEdgeEndp(way, withoutp). This should take 2 iterations max. const EdgeSet& edges = m_edges[way.invert()]; uint32_t result = 0; - for (EdgeSet::const_reverse_iterator it = edges.rbegin(); it != edges.rend(); ++it) { - if ((*it).key() != withoutp->furtherp(way.invert())) { + for (const auto& edge : vlstd::reverse_view(edges)) { + if (edge.key() != withoutp->furtherp(way.invert())) { // Use the cached cost. It could be a small overestimate // due to stepping. This is consistent with critPathCost() // which also returns the cached cost. - result = (*it).value(); + result = edge.value(); break; } } diff --git a/src/V3Simulate.h b/src/V3Simulate.h index b5fbdb234..d5a7453ac 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -182,11 +182,11 @@ public: } m_whyNotOptimizable = why; std::ostringstream stack; - for (auto it = m_callStack.rbegin(); it != m_callStack.rend(); ++it) { - AstFuncRef* const funcp = (*it)->m_funcp; + for (auto& callstack : vlstd::reverse_view(m_callStack)) { + AstFuncRef* const funcp = callstack->m_funcp; stack << "\n " << funcp->fileline() << "... Called from " << funcp->prettyName() << "() with parameters:"; - V3TaskConnects* tconnects = (*it)->m_tconnects; + V3TaskConnects* tconnects = callstack->m_tconnects; for (V3TaskConnects::iterator conIt = tconnects->begin(); conIt != tconnects->end(); ++conIt) { AstVar* const portp = conIt->first; From 5f0e1fae7fe5bebefd4e95da3e18f0a2de1c8d41 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Fri, 22 Apr 2022 22:39:45 +0100 Subject: [PATCH 03/10] Simplify and clarify reporting of enclosing instance Rename AstNodeModule::hierName -> someInstanceName and explain that this is only used for user messages. Rename AstNode::locationStr -> instanceStr and simplify implementation. In particular, do not report an instance if we can't find a reasonable guess. --- src/V3Ast.cpp | 56 +++++++++++------------------- src/V3Ast.h | 9 ++--- src/V3Error.cpp | 8 ++--- src/V3Error.h | 2 +- src/V3FileLine.cpp | 6 ++-- src/V3FileLine.h | 2 +- src/V3Hasher.cpp | 3 +- src/V3Param.cpp | 19 +++++----- test_regress/t/t_force_bad_rw.out | 1 - test_regress/t/t_fuzz_eqne_bad.out | 1 - 10 files changed, 46 insertions(+), 61 deletions(-) diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 5a18c98ee..1bd2cc14e 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -1135,49 +1135,33 @@ void AstNode::v3errorEndFatal(std::ostringstream& str) const { VL_UNREACHABLE } -string AstNode::locationStr() const { - string str = "... In instance "; - const AstNode* backp = this; - int itmax = 10000; // Max iterations before giving up on location search - while (backp) { - if (VL_UNCOVERABLE(--itmax < 0)) { - // Likely some circular back link, and V3Ast is trying to report a low-level error - UINFO(1, "Ran out of iterations finding locationStr on " << backp << endl); - return ""; // LCOV_EXCL_LINE - } - const AstScope* scopep; - if ((scopep = VN_CAST(backp, Scope))) { - // The design is flattened and there are no useful scopes - // This is probably because of inlining - if (scopep->isTop()) break; +string AstNode::instanceStr() const { + // Max iterations before giving up on location search, + // in case we have some circular reference bug. + constexpr unsigned maxIterations = 10000; + unsigned iterCount = 0; - str += scopep->prettyName(); - return str; + for (const AstNode* backp = this; backp; backp = backp->backp(), ++iterCount) { + if (VL_UNCOVERABLE(iterCount >= maxIterations)) return ""; // LCOV_EXCL_LINE + + // Prefer the enclosing scope, if there is one. This is always under the enclosing module, + // so just pick it up when encountered + if (const AstScope* const scopep = VN_CAST(backp, Scope)) { + return scopep->isTop() ? "" : "... In instance " + scopep->prettyName(); } - backp = backp->backp(); - } - backp = this; - while (backp) { - const AstModule* modp; - const AstNodeVarRef* nvrp; - if ((modp = VN_CAST(backp, Module)) && !modp->hierName().empty()) { - str += modp->hierName(); - return str; - } else if ((nvrp = VN_CAST(backp, NodeVarRef))) { - const string prettyName = nvrp->prettyName(); - // VarRefs have not been flattened yet and do not contain location information - if (prettyName != nvrp->name()) { - str += prettyName; - return str; - } + + // If scopes don't exist, report an example instance of the enclosing module + if (const AstModule* const modp = VN_CAST(backp, Module)) { + const string instanceName = modp->someInstanceName(); + return instanceName.empty() ? "" : "... In instance " + instanceName; } - backp = backp->backp(); } + return ""; } void AstNode::v3errorEnd(std::ostringstream& str) const { if (!m_fileline) { - V3Error::v3errorEnd(str, locationStr()); + V3Error::v3errorEnd(str, instanceStr()); } else { std::ostringstream nsstr; nsstr << str.str(); @@ -1187,7 +1171,7 @@ void AstNode::v3errorEnd(std::ostringstream& str) const { const_cast(this)->dump(nsstr); nsstr << endl; } - m_fileline->v3errorEnd(nsstr, locationStr()); + m_fileline->v3errorEnd(nsstr, instanceStr()); } } diff --git a/src/V3Ast.h b/src/V3Ast.h index 1e717b6e5..9ddf48e43 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1447,7 +1447,7 @@ private: bool gateOnly); void deleteTreeIter(); void deleteNode(); - string locationStr() const; + string instanceStr() const; public: static void relinkOneLink(AstNode*& pointpr, AstNode* newp); @@ -3077,7 +3077,8 @@ class AstNodeModule VL_NOT_FINAL : public AstNode { private: string m_name; // Name of the module const string m_origName; // Name of the module, ignoring name() changes, for dot lookup - string m_hierName; // Hierarchical name for errors, etc. + string m_someInstanceName; // Hierarchical name of some arbitrary instance of this module. + // Used for user messages only. bool m_modPublic : 1; // Module has public references bool m_modTrace : 1; // Tracing this module bool m_inLibrary : 1; // From a library, no error if not used, never top level @@ -3119,8 +3120,8 @@ public: // ACCESSORS virtual void name(const string& name) override { m_name = name; } virtual string origName() const override { return m_origName; } - string hierName() const { return m_hierName; } - void hierName(const string& hierName) { m_hierName = hierName; } + string someInstanceName() const { return m_someInstanceName; } + void someInstanceName(const string& name) { m_someInstanceName = name; } bool inLibrary() const { return m_inLibrary; } void inLibrary(bool flag) { m_inLibrary = flag; } void level(int level) { m_level = level; } diff --git a/src/V3Error.cpp b/src/V3Error.cpp index 9452c9ff8..1f1f8d08f 100644 --- a/src/V3Error.cpp +++ b/src/V3Error.cpp @@ -183,7 +183,7 @@ void V3Error::suppressThisWarning() { string V3Error::warnMore() { return string(msgPrefix().size(), ' '); } -void V3Error::v3errorEnd(std::ostringstream& sstr, const string& locationStr) { +void V3Error::v3errorEnd(std::ostringstream& sstr, const string& extra) { #if defined(__COVERITY__) || defined(__cppcheck__) if (s_errorCode == V3ErrorCode::EC_FATAL) __coverity_panic__(x); #endif @@ -209,10 +209,10 @@ void V3Error::v3errorEnd(std::ostringstream& sstr, const string& locationStr) { // Suppress duplicate messages if (s_messages.find(msg) != s_messages.end()) return; s_messages.insert(msg); - if (!locationStr.empty()) { - const string locationMsg = warnMore() + locationStr + "\n"; + if (!extra.empty()) { + const string extraMsg = warnMore() + extra + "\n"; const size_t pos = msg.find('\n'); - msg.insert(pos + 1, locationMsg); + msg.insert(pos + 1, extraMsg); } // Output if ( diff --git a/src/V3Error.h b/src/V3Error.h index b0b4663d7..077260995 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -313,7 +313,7 @@ public: static void vlAbortOrExit(); static void vlAbort(); // static, but often overridden in classes. - static void v3errorEnd(std::ostringstream& sstr, const string& locationStr = ""); + static void v3errorEnd(std::ostringstream& sstr, const string& extra = ""); }; // Global versions, so that if the class doesn't define a operator, we get the functions anyways. diff --git a/src/V3FileLine.cpp b/src/V3FileLine.cpp index 29d99d7c4..fa5e324fe 100644 --- a/src/V3FileLine.cpp +++ b/src/V3FileLine.cpp @@ -340,15 +340,15 @@ void FileLine::modifyStateInherit(const FileLine* fromp) { } } -void FileLine::v3errorEnd(std::ostringstream& sstr, const string& locationStr) { +void FileLine::v3errorEnd(std::ostringstream& sstr, const string& extra) { std::ostringstream nsstr; if (lastLineno()) nsstr << this; nsstr << sstr.str(); nsstr << endl; std::ostringstream lstr; - if (!locationStr.empty()) { + if (!extra.empty()) { lstr << std::setw(ascii().length()) << " " - << ": " << locationStr; + << ": " << extra; } m_waive = V3Config::waive(this, V3Error::errorCode(), sstr.str()); if (warnIsOff(V3Error::errorCode()) || m_waive) { diff --git a/src/V3FileLine.h b/src/V3FileLine.h index dbc437588..1928e8f5a 100644 --- a/src/V3FileLine.h +++ b/src/V3FileLine.h @@ -245,7 +245,7 @@ public: void modifyWarnOff(V3ErrorCode code, bool flag) { warnOff(code, flag); } // OPERATORS - void v3errorEnd(std::ostringstream& str, const string& locationStr = ""); + void v3errorEnd(std::ostringstream& str, const string& extra = ""); void v3errorEndFatal(std::ostringstream& str); /// When building an error, prefix for printing continuation lines /// e.g. information referring to the same FileLine as before diff --git a/src/V3Hasher.cpp b/src/V3Hasher.cpp index 56652e647..10c53d4c8 100644 --- a/src/V3Hasher.cpp +++ b/src/V3Hasher.cpp @@ -299,9 +299,8 @@ private: m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [=]() {}); } virtual void visit(AstNodeModule* nodep) override { - m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, false, [=]() { + m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, false, [=]() { // m_hash += nodep->origName(); - m_hash += nodep->hierName(); }); } virtual void visit(AstNodePreSel* nodep) override { diff --git a/src/V3Param.cpp b/src/V3Param.cpp index ea1936ae3..ebd55b526 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -761,7 +761,7 @@ class ParamProcessor final { } public: - void cellDeparam(AstCell* nodep, AstNodeModule* modp, const string& hierName) { + void cellDeparam(AstCell* nodep, AstNodeModule* modp, const string& someInstanceName) { m_modp = modp; // Cell: Check for parameters in the instantiation. // We always run this, even if no parameters, as need to look for interfaces, @@ -772,7 +772,7 @@ public: // Evaluate all module constants V3Const::constifyParamsEdit(nodep); AstNodeModule* const srcModp = nodep->modp(); - srcModp->hierName(hierName + "." + nodep->name()); + srcModp->someInstanceName(someInstanceName + "." + nodep->name()); // Make sure constification worked // Must be a separate loop, as constant conversion may have changed some pointers. @@ -857,11 +857,11 @@ class ParamVisitor final : public VNVisitor { // METHODS VL_DEBUG_FUNC; // Declare debug() - void visitCellDeparam(AstCell* nodep, const string& hierName) { + void visitCellDeparam(AstCell* nodep, const string& someInstanceName) { // Cell: Check for parameters in the instantiation. iterateChildren(nodep); UASSERT_OBJ(nodep->modp(), nodep, "Not linked?"); - m_processor.cellDeparam(nodep, m_modp, hierName); + m_processor.cellDeparam(nodep, m_modp, someInstanceName); // Remember to process the child module at the end of the module m_todoModps.emplace(nodep->modp()->level(), nodep->modp()); } @@ -877,8 +877,11 @@ class ParamVisitor final : public VNVisitor { // again m_modp = nodep; UINFO(4, " MOD " << nodep << endl); - if (m_modp->hierName().empty()) m_modp->hierName(m_modp->origName()); + if (m_modp->someInstanceName().empty()) { + m_modp->someInstanceName(m_modp->origName()); + } iterateChildren(nodep); + // Note above iterate may add to m_todoModps // // Process interface cells, then non-interface which may ref an interface cell @@ -886,13 +889,13 @@ class ParamVisitor final : public VNVisitor { for (AstCell* const cellp : m_cellps) { if ((nonIf == 0 && VN_IS(cellp->modp(), Iface)) || (nonIf == 1 && !VN_IS(cellp->modp(), Iface))) { - string fullName(m_modp->hierName()); + string someInstanceName(m_modp->someInstanceName()); if (const string* const genHierNamep = (string*)cellp->user5p()) { - fullName += *genHierNamep; + someInstanceName += *genHierNamep; cellp->user5p(nullptr); VL_DO_DANGLING(delete genHierNamep, genHierNamep); } - VL_DO_DANGLING(visitCellDeparam(cellp, fullName), cellp); + VL_DO_DANGLING(visitCellDeparam(cellp, someInstanceName), cellp); } } } diff --git a/test_regress/t/t_force_bad_rw.out b/test_regress/t/t_force_bad_rw.out index caa8482e2..0b287ff80 100644 --- a/test_regress/t/t_force_bad_rw.out +++ b/test_regress/t/t_force_bad_rw.out @@ -1,5 +1,4 @@ %Error: t/t_force_bad_rw.v:14:20: Unsupported: Signals used via read-write reference cannot be forced - : ... In instance t.unnamedblk1.unnamedblk1.index 14 | foreach (ass[index]) begin | ^~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_fuzz_eqne_bad.out b/test_regress/t/t_fuzz_eqne_bad.out index 08bd37daf..a04d877ab 100644 --- a/test_regress/t/t_fuzz_eqne_bad.out +++ b/test_regress/t/t_fuzz_eqne_bad.out @@ -1,5 +1,4 @@ %Error: t/t_fuzz_eqne_bad.v:12:23: Slice operator VARREF 't.b' on non-slicable (e.g. non-vector) right-hand-side operand - : ... In instance t.b 12 | initial c = (a != &b); | ^ %Error: Exiting due to From 8189416d0c0c6de80237bf29d084f21a020f1651 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Fri, 22 Apr 2022 21:48:03 +0100 Subject: [PATCH 04/10] Partial cleanup of V3Param. No functional change. --- src/V3Param.cpp | 137 +++++++++++++++++++++++------------------------- 1 file changed, 65 insertions(+), 72 deletions(-) diff --git a/src/V3Param.cpp b/src/V3Param.cpp index ebd55b526..bfad0f84c 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -847,78 +847,74 @@ class ParamVisitor final : public VNVisitor { ParamProcessor m_processor; // De-parameterize a cell, build modules UnrollStateful m_unroller; // Loop unroller - AstNodeModule* m_modp = nullptr; // Current module being processed + bool m_iterateModule = false; // Iterating module body string m_generateHierName; // Generate portion of hierarchy name string m_unlinkedTxt; // Text for AstUnlinkedRef - std::deque m_cellps; // Cells left to process (in this module) - - std::multimap m_todoModps; // Modules left to process + std::deque m_cellps; // Cells left to process (in current module) // METHODS VL_DEBUG_FUNC; // Declare debug() - void visitCellDeparam(AstCell* nodep, const string& someInstanceName) { - // Cell: Check for parameters in the instantiation. - iterateChildren(nodep); - UASSERT_OBJ(nodep->modp(), nodep, "Not linked?"); - m_processor.cellDeparam(nodep, m_modp, someInstanceName); - // Remember to process the child module at the end of the module - m_todoModps.emplace(nodep->modp()->level(), nodep->modp()); - } - void visitModules() { - // Loop on all modules left to process - // Hitting a cell adds to the appropriate level of this level-sorted list, - // so since cells originally exist top->bottom we process in top->bottom order too. - while (!m_todoModps.empty()) { - const auto itm = m_todoModps.cbegin(); - AstNodeModule* const nodep = itm->second; - m_todoModps.erase(itm); - if (!nodep->user5SetOnce()) { // Process once; note clone() must clear so we do it - // again - m_modp = nodep; - UINFO(4, " MOD " << nodep << endl); - if (m_modp->someInstanceName().empty()) { - m_modp->someInstanceName(m_modp->origName()); - } - iterateChildren(nodep); + void visitCells(AstNodeModule* nodep) { + UASSERT_OBJ(!m_iterateModule, nodep, "Should not nest"); + std::multimap workQueue; + workQueue.emplace(nodep->level(), nodep); + m_generateHierName = ""; + m_iterateModule = true; - // Note above iterate may add to m_todoModps - // - // Process interface cells, then non-interface which may ref an interface cell - for (int nonIf = 0; nonIf < 2; ++nonIf) { - for (AstCell* const cellp : m_cellps) { - if ((nonIf == 0 && VN_IS(cellp->modp(), Iface)) - || (nonIf == 1 && !VN_IS(cellp->modp(), Iface))) { - string someInstanceName(m_modp->someInstanceName()); - if (const string* const genHierNamep = (string*)cellp->user5p()) { - someInstanceName += *genHierNamep; - cellp->user5p(nullptr); - VL_DO_DANGLING(delete genHierNamep, genHierNamep); - } - VL_DO_DANGLING(visitCellDeparam(cellp, someInstanceName), cellp); - } + // Visit all cells under module, recursively + do { + const auto itm = workQueue.cbegin(); + AstNodeModule* const modp = itm->second; + workQueue.erase(itm); + + // Process once; note user5 will be cleared on specialization, so we will do the + // specialized module if needed + if (modp->user5SetOnce()) continue; + + // TODO: this really should be an assert, but classes and hier_blocks are special... + if (modp->someInstanceName().empty()) modp->someInstanceName(modp->origName()); + + // Iterate the body + iterateChildren(modp); + + // Process interface cells, then non-interface cells, which may reference an interface + // cell. + for (bool doInterface : {true, false}) { + for (AstCell* const cellp : m_cellps) { + if (doInterface != VN_IS(cellp->modp(), Iface)) continue; + + // Visit parameters in the instantiation. + iterateChildren(cellp); + + // Update path + string someInstanceName(modp->someInstanceName()); + if (const string* const genHierNamep = cellp->user5u().to()) { + someInstanceName += *genHierNamep; + cellp->user5p(nullptr); + VL_DO_DANGLING(delete genHierNamep, genHierNamep); } + + // Apply parameter specialization + m_processor.cellDeparam(cellp, modp, someInstanceName); + + // Add the (now potentially specialized) child module to the work queue + workQueue.emplace(cellp->modp()->level(), cellp->modp()); } - m_cellps.clear(); - m_modp = nullptr; - UINFO(4, " MOD-done\n"); } - } + m_cellps.clear(); + } while (!workQueue.empty()); + + m_iterateModule = false; } // VISITORS virtual void visit(AstNodeModule* nodep) override { - if (nodep->dead()) { - UINFO(4, " MOD-dead. " << nodep << endl); // Marked by LinkDot - return; - } else if (nodep->recursiveClone()) { - // Fake, made for recursive elimination - UINFO(4, " MOD-recursive-dead. " << nodep << endl); - nodep->dead(true); // So Dead checks won't count references to it - return; - } - // - if (!nodep->dead() && VN_IS(nodep, Class)) { + if (nodep->recursiveClone()) nodep->dead(true); // Fake, made for recursive elimination + if (nodep->dead()) return; // Marked by LinkDot (and above) + + // Warn on unsupported parametrised class + if (VN_IS(nodep, Class)) { for (AstNode* stmtp = nodep->stmtsp(); stmtp; stmtp = stmtp->nextp()) { if (const AstVar* const varp = VN_CAST(stmtp, Var)) { if (varp->isParam()) { @@ -927,24 +923,21 @@ class ParamVisitor final : public VNVisitor { } } } - // - if (m_modp) { + + if (m_iterateModule) { // Iterating body UINFO(4, " MOD-under-MOD. " << nodep << endl); iterateChildren(nodep); - } else if (nodep->level() <= 2 // Haven't added top yet, so level 2 is the top - || VN_IS(nodep, Class) // Nor moved classes - || VN_IS(nodep, Package)) { // Likewise haven't done wrapTopPackages yet - // Add request to END of modules left to process - m_todoModps.emplace(nodep->level(), nodep); - m_generateHierName = ""; - visitModules(); - } else if (nodep->user5()) { - UINFO(4, " MOD-done " << nodep << endl); // Already did it - } else { - // Should have been done by now, if not dead - UINFO(4, " MOD-dead? " << nodep << endl); + return; + } + + // Start traversal at root-like things + if (nodep->level() <= 2 // Haven't added top yet, so level 2 is the top + || VN_IS(nodep, Class) // Nor moved classes + || VN_IS(nodep, Package)) { // Likewise haven't done wrapTopPackages yet + visitCells(nodep); } } + virtual void visit(AstCell* nodep) override { // Must do ifaces first, so push to list and do in proper order string* const genHierNamep = new string(m_generateHierName); From 0b74e9b354057d09f57f6969bfe0d6b08af04bb9 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 23 Apr 2022 13:06:20 +0100 Subject: [PATCH 05/10] Ensure topological ordering of module list. At the end of V3Param, fix up the module list to be topologically sorted. We need to do this at the end as a later instantiation of a recursive module might instantiate an earlier specialization, which we cannot know until we processed everything. The rest of the compiler depends on the module list being topologically sorted. Fixes #3393 --- Changes | 1 + src/V3Param.cpp | 67 ++++++++++++++++++---- test_regress/t/t_recursive_module_bug_2.pl | 16 ++++++ test_regress/t/t_recursive_module_bug_2.v | 21 +++++++ 4 files changed, 95 insertions(+), 10 deletions(-) create mode 100755 test_regress/t/t_recursive_module_bug_2.pl create mode 100644 test_regress/t/t_recursive_module_bug_2.v diff --git a/Changes b/Changes index 12f8458f1..8a8a34272 100644 --- a/Changes +++ b/Changes @@ -21,6 +21,7 @@ Verilator 4.221 devel * Fix tracing interfaces inside interfaces (#3309). [Kevin Millis] * Fix filenames with dots overwriting debug .vpp files (#3373). * Fix including VK_USER_OBJS in make library (#3370). [Julien Margetts] +* Fix crash in recursive module inlining (#3393). [david-sawatzke] Verilator 4.220 2022-03-12 ========================== diff --git a/src/V3Param.cpp b/src/V3Param.cpp index bfad0f84c..607639ed9 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -555,14 +555,12 @@ class ParamProcessor final { cellp->v3error("Exceeded maximum --module-recursion-depth of " << v3Global.opt.moduleRecursionDepth()); } - // Keep tree sorted by level. Append to end of sub-list at the same level. This is - // important because due to the way recursive modules are handled, different - // parametrizations of the same recursive module end up with the same level (which in - // itself is a bit unfortunate). Nevertheless, as a later parametrization must not be above - // an earlier parametrization of a recursive module, it is sufficient to add to the end of - // the sub-list to keep the modules topologically sorted. + // Keep tree sorted by level. Note: Different parametrizations of the same recursive module + // end up with the same level, which we will need to fix up at the end, as we do not know + // up front how recursive modules are expanded, and a later expansion might re-use an + // earlier expansion (see t_recursive_module_bug_2). AstNodeModule* insertp = srcModp; - while (VN_IS(insertp->nextp(), NodeModule) + while (insertp->nextp() && VN_AS(insertp->nextp(), NodeModule)->level() <= newmodp->level()) { insertp = VN_AS(insertp->nextp(), NodeModule); } @@ -843,6 +841,9 @@ public: // Process parameter visitor class ParamVisitor final : public VNVisitor { + // NODE STATE + // AstNodeModule::user1 -> bool: already fixed level + // STATE ParamProcessor m_processor; // De-parameterize a cell, build modules UnrollStateful m_unroller; // Loop unroller @@ -852,6 +853,9 @@ class ParamVisitor final : public VNVisitor { string m_unlinkedTxt; // Text for AstUnlinkedRef std::deque m_cellps; // Cells left to process (in current module) + // Map from AstNodeModule to set of all AstNodeModules that instantiates it. + std::unordered_map> m_parentps; + // METHODS VL_DEBUG_FUNC; // Declare debug() @@ -900,6 +904,9 @@ class ParamVisitor final : public VNVisitor { // Add the (now potentially specialized) child module to the work queue workQueue.emplace(cellp->modp()->level(), cellp->modp()); + + // Add to the hierarchy registry + m_parentps[cellp->modp()].insert(modp); } } m_cellps.clear(); @@ -908,6 +915,18 @@ class ParamVisitor final : public VNVisitor { m_iterateModule = false; } + // Fix up level of module, based on who instantiates it + void fixLevel(AstNodeModule* modp) { + if (modp->user1SetOnce()) return; // Already fixed + if (m_parentps[modp].empty()) return; // Leave top levels alone + int maxParentLevel = 0; + for (AstNodeModule* parentp : m_parentps[modp]) { + fixLevel(parentp); // Ensure parent level is correct + maxParentLevel = std::max(maxParentLevel, parentp->level()); + } + if (modp->level() <= maxParentLevel) modp->level(maxParentLevel + 1); + } + // VISITORS virtual void visit(AstNodeModule* nodep) override { if (nodep->recursiveClone()) nodep->dead(true); // Fake, made for recursive elimination @@ -1191,10 +1210,38 @@ class ParamVisitor final : public VNVisitor { public: // CONSTRUCTORS - explicit ParamVisitor(AstNetlist* nodep) - : m_processor{nodep} { + explicit ParamVisitor(AstNetlist* netlistp) + : m_processor{netlistp} { // Relies on modules already being in top-down-order - iterate(nodep); + iterate(netlistp); + + // Re-sort module list to be in topological order and fix-up incorrect levels. We need to + // do this globally at the end due to the presence of recursive modules, which might be + // expanded in orders that reuse earlier specializations later at a lower level. + { + // Gather modules + std::vector modps; + for (AstNodeModule *modp = netlistp->modulesp(), *nextp; modp; modp = nextp) { + nextp = VN_AS(modp->nextp(), NodeModule); + modp->unlinkFrBack(); + modps.push_back(modp); + } + + // Fix-up levels + { + const VNUser1InUse user1InUse; + for (AstNodeModule* const modp : modps) fixLevel(modp); + } + + // Sort by level + std::stable_sort(modps.begin(), modps.end(), + [](const AstNodeModule* ap, const AstNodeModule* bp) { + return ap->level() < bp->level(); + }); + + // Re-insert modules + for (AstNodeModule* const modp : modps) netlistp->addModulep(modp); + } } virtual ~ParamVisitor() override = default; VL_UNCOPYABLE(ParamVisitor); diff --git a/test_regress/t/t_recursive_module_bug_2.pl b/test_regress/t/t_recursive_module_bug_2.pl new file mode 100755 index 000000000..2ef6db6a2 --- /dev/null +++ b/test_regress/t/t_recursive_module_bug_2.pl @@ -0,0 +1,16 @@ +#!/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 Geza Lore. 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(); + +ok(1); +1; diff --git a/test_regress/t/t_recursive_module_bug_2.v b/test_regress/t/t_recursive_module_bug_2.v new file mode 100644 index 000000000..fd47d945b --- /dev/null +++ b/test_regress/t/t_recursive_module_bug_2.v @@ -0,0 +1,21 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Copyright 2022 by Geza Lore. 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. + +module a #(parameter N) (); + generate if (N > 1) begin + // With N == 5, this will first expand N == 2, then expand N == 3, + // which instantiates N == 2. This requires fixing up topological order + // in V3Param. + a #(.N( N/2)) sub_lo(); + a #(.N(N-N/2)) sub_hi(); + end + endgenerate +endmodule + +module top(); + a #(.N(5)) root (); +endmodule From f1ea30f2572faca64ee2851bfe40dc95de0abd8a Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 23 Apr 2022 14:03:37 +0100 Subject: [PATCH 06/10] Use iterate*Const V3EmitV visitors. No functional change. --- src/V3EmitV.cpp | 194 ++++++++++++++++++++++++------------------------ src/V3EmitV.h | 4 +- 2 files changed, 99 insertions(+), 99 deletions(-) diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index ecec3802e..e04c15e4d 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -52,10 +52,10 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } // VISITORS - virtual void visit(AstNetlist* nodep) override { iterateAndNextNull(nodep->modulesp()); } + virtual void visit(AstNetlist* nodep) override { iterateAndNextConstNull(nodep->modulesp()); } virtual void visit(AstNodeModule* nodep) override { putfs(nodep, nodep->verilogKwd() + " " + prefixNameProtect(nodep) + ";\n"); - iterateChildren(nodep); + iterateChildrenConst(nodep); putqs(nodep, "end" + nodep->verilogKwd() + "\n"); } virtual void visit(AstPort* nodep) override {} @@ -65,7 +65,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { puts(nodep->prettyName()); puts(";\n"); // Only putfs the first time for each visitor; later for same node is putqs - iterateAndNextNull(nodep->stmtsp()); + iterateAndNextConstNull(nodep->stmtsp()); putfs(nodep, nodep->isFunction() ? "endfunction\n" : "endtask\n"); } @@ -75,7 +75,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } else { putbs("begin : " + nodep->name() + "\n"); } - iterateChildren(nodep); + iterateChildrenConst(nodep); puts("end\n"); } virtual void visit(AstFork* nodep) override { @@ -84,75 +84,75 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } else { putbs("fork : " + nodep->name() + "\n"); } - iterateChildren(nodep); + iterateChildrenConst(nodep); puts(nodep->joinType().verilogKwd()); puts("\n"); } virtual void visit(AstFinal* nodep) override { putfs(nodep, "final begin\n"); - iterateChildren(nodep); + iterateChildrenConst(nodep); putqs(nodep, "end\n"); } virtual void visit(AstInitial* nodep) override { putfs(nodep, "initial begin\n"); - iterateChildren(nodep); + iterateChildrenConst(nodep); putqs(nodep, "end\n"); } - virtual void visit(AstInitialAutomatic* nodep) override { iterateChildren(nodep); } + virtual void visit(AstInitialAutomatic* nodep) override { iterateChildrenConst(nodep); } virtual void visit(AstAlways* nodep) override { putfs(nodep, "always "); if (m_sensesp) { - iterateAndNextNull(m_sensesp); + iterateAndNextConstNull(m_sensesp); } // In active else { - iterateAndNextNull(nodep->sensesp()); + iterateAndNextConstNull(nodep->sensesp()); } putbs(" begin\n"); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); putqs(nodep, "end\n"); } virtual void visit(AstAlwaysPublic* nodep) override { putfs(nodep, "/*verilator public_flat_rw "); if (m_sensesp) { - iterateAndNextNull(m_sensesp); + iterateAndNextConstNull(m_sensesp); } // In active else { - iterateAndNextNull(nodep->sensesp()); + iterateAndNextConstNull(nodep->sensesp()); } putqs(nodep, " "); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); putqs(nodep, "*/\n"); } virtual void visit(AstNodeAssign* nodep) override { if (VN_IS(nodep, AssignForce)) puts("force "); - iterateAndNextNull(nodep->lhsp()); + iterateAndNextConstNull(nodep->lhsp()); putfs(nodep, " " + nodep->verilogKwd() + " "); - iterateAndNextNull(nodep->rhsp()); + iterateAndNextConstNull(nodep->rhsp()); if (!m_suppressSemi) puts(";\n"); } virtual void visit(AstAssignDly* nodep) override { - iterateAndNextNull(nodep->lhsp()); + iterateAndNextConstNull(nodep->lhsp()); putfs(nodep, " <= "); - iterateAndNextNull(nodep->rhsp()); + iterateAndNextConstNull(nodep->rhsp()); puts(";\n"); } virtual void visit(AstAssignAlias* nodep) override { putbs("alias "); - iterateAndNextNull(nodep->lhsp()); + iterateAndNextConstNull(nodep->lhsp()); putfs(nodep, " = "); - iterateAndNextNull(nodep->rhsp()); + iterateAndNextConstNull(nodep->rhsp()); if (!m_suppressSemi) puts(";\n"); } virtual void visit(AstAssignW* nodep) override { putfs(nodep, "assign "); - iterateAndNextNull(nodep->lhsp()); + iterateAndNextConstNull(nodep->lhsp()); putbs(" = "); - iterateAndNextNull(nodep->rhsp()); + iterateAndNextConstNull(nodep->rhsp()); if (!m_suppressSemi) puts(";\n"); } virtual void visit(AstRelease* nodep) override { puts("release "); - iterateAndNextNull(nodep->lhsp()); + iterateAndNextConstNull(nodep->lhsp()); if (!m_suppressSemi) puts(";\n"); } virtual void visit(AstBreak*) override { @@ -172,7 +172,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { putfs(nodep, ""); puts(nodep->edgeType().verilogKwd()); if (nodep->sensp()) puts(" "); - iterateChildren(nodep); + iterateChildrenConst(nodep); } virtual void visit(AstNodeCase* nodep) override { putfs(nodep, ""); @@ -183,7 +183,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } puts(nodep->verilogKwd()); puts(" ("); - iterateAndNextNull(nodep->exprp()); + iterateAndNextConstNull(nodep->exprp()); puts(")\n"); if (const AstCase* const casep = VN_CAST(nodep, Case)) { if (casep->fullPragma() || casep->parallelPragma()) { @@ -192,22 +192,22 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { if (casep->parallelPragma()) puts(" parallel_case"); } } - iterateAndNextNull(nodep->itemsp()); + iterateAndNextConstNull(nodep->itemsp()); putqs(nodep, "endcase\n"); } virtual void visit(AstCaseItem* nodep) override { if (nodep->condsp()) { - iterateAndNextNull(nodep->condsp()); + iterateAndNextConstNull(nodep->condsp()); } else { putbs("default"); } putfs(nodep, ": begin "); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); putqs(nodep, "end\n"); } virtual void visit(AstComment* nodep) override { puts(string("// ") + nodep->name() + "\n"); - iterateChildren(nodep); + iterateChildrenConst(nodep); } virtual void visit(AstContinue*) override { putbs("continue"); @@ -222,13 +222,13 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { putfs(nodep, nodep->verilogKwd()); putbs("("); if (fileOrStrgp) { - iterateAndNextNull(fileOrStrgp); + iterateAndNextConstNull(fileOrStrgp); putbs(", "); } putsQuoted(text); for (AstNode* expp = exprsp; expp; expp = expp->nextp()) { puts(", "); - iterateAndNextNull(expp); + iterateAndNextConstNull(expp); } puts(");\n"); } @@ -254,32 +254,32 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { virtual void visit(AstFOpen* nodep) override { putfs(nodep, nodep->verilogKwd()); putbs("("); - iterateAndNextNull(nodep->filenamep()); + iterateAndNextConstNull(nodep->filenamep()); putbs(", "); - iterateAndNextNull(nodep->modep()); + iterateAndNextConstNull(nodep->modep()); puts(");\n"); } virtual void visit(AstFOpenMcd* nodep) override { putfs(nodep, nodep->verilogKwd()); putbs("("); - iterateAndNextNull(nodep->filenamep()); + iterateAndNextConstNull(nodep->filenamep()); puts(");\n"); } virtual void visit(AstFClose* nodep) override { putfs(nodep, nodep->verilogKwd()); putbs("("); - if (nodep->filep()) iterateAndNextNull(nodep->filep()); + if (nodep->filep()) iterateAndNextConstNull(nodep->filep()); puts(");\n"); } virtual void visit(AstFFlush* nodep) override { putfs(nodep, nodep->verilogKwd()); putbs("("); - if (nodep->filep()) iterateAndNextNull(nodep->filep()); + if (nodep->filep()) iterateAndNextConstNull(nodep->filep()); puts(");\n"); } virtual void visit(AstJumpBlock* nodep) override { putbs("begin : label" + cvtToStr(nodep->labelNum()) + "\n"); - if (nodep->stmtsp()) iterateAndNextNull(nodep->stmtsp()); + if (nodep->stmtsp()) iterateAndNextConstNull(nodep->stmtsp()); puts("end\n"); } virtual void visit(AstJumpGo* nodep) override { @@ -291,27 +291,27 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { virtual void visit(AstNodeReadWriteMem* nodep) override { putfs(nodep, nodep->verilogKwd()); putbs("("); - if (nodep->filenamep()) iterateAndNextNull(nodep->filenamep()); + if (nodep->filenamep()) iterateAndNextConstNull(nodep->filenamep()); putbs(", "); - if (nodep->memp()) iterateAndNextNull(nodep->memp()); + if (nodep->memp()) iterateAndNextConstNull(nodep->memp()); if (nodep->lsbp()) { putbs(", "); - iterateAndNextNull(nodep->lsbp()); + iterateAndNextConstNull(nodep->lsbp()); } if (nodep->msbp()) { putbs(", "); - iterateAndNextNull(nodep->msbp()); + iterateAndNextConstNull(nodep->msbp()); } puts(");\n"); } virtual void visit(AstSysFuncAsTask* nodep) override { - iterateAndNextNull(nodep->lhsp()); + iterateAndNextConstNull(nodep->lhsp()); puts(";\n"); } virtual void visit(AstSysIgnore* nodep) override { putfs(nodep, nodep->verilogKwd()); putbs("("); - iterateAndNextNull(nodep->exprsp()); + iterateAndNextConstNull(nodep->exprsp()); puts(");\n"); } virtual void visit(AstNodeFor* nodep) override { @@ -319,31 +319,31 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { { VL_RESTORER(m_suppressSemi); m_suppressSemi = true; - iterateAndNextNull(nodep->initsp()); + iterateAndNextConstNull(nodep->initsp()); puts(";"); - iterateAndNextNull(nodep->condp()); + iterateAndNextConstNull(nodep->condp()); puts(";"); - iterateAndNextNull(nodep->incsp()); + iterateAndNextConstNull(nodep->incsp()); } puts(") begin\n"); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); putqs(nodep, "end\n"); } virtual void visit(AstRepeat* nodep) override { putfs(nodep, "repeat ("); - iterateAndNextNull(nodep->countp()); + iterateAndNextConstNull(nodep->countp()); puts(") begin\n"); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); putfs(nodep, "end\n"); } virtual void visit(AstWhile* nodep) override { - iterateAndNextNull(nodep->precondsp()); + iterateAndNextConstNull(nodep->precondsp()); putfs(nodep, "while ("); - iterateAndNextNull(nodep->condp()); + iterateAndNextConstNull(nodep->condp()); puts(") begin\n"); - iterateAndNextNull(nodep->bodysp()); - iterateAndNextNull(nodep->incsp()); - iterateAndNextNull(nodep->precondsp()); // Need to recompute before next loop + iterateAndNextConstNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->incsp()); + iterateAndNextConstNull(nodep->precondsp()); // Need to recompute before next loop putfs(nodep, "end\n"); } virtual void visit(AstNodeIf* nodep) override { @@ -354,28 +354,28 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { if (ifp->unique0Pragma()) puts("unique0 "); } puts("if ("); - iterateAndNextNull(nodep->condp()); + iterateAndNextConstNull(nodep->condp()); puts(") begin\n"); - iterateAndNextNull(nodep->ifsp()); + iterateAndNextConstNull(nodep->ifsp()); if (nodep->elsesp()) { putqs(nodep, "end\n"); putqs(nodep, "else begin\n"); - iterateAndNextNull(nodep->elsesp()); + iterateAndNextConstNull(nodep->elsesp()); } putqs(nodep, "end\n"); } virtual void visit(AstPast* nodep) override { putfs(nodep, "$past("); - iterateAndNextNull(nodep->exprp()); + iterateAndNextConstNull(nodep->exprp()); if (nodep->ticksp()) { puts(", "); - iterateAndNextNull(nodep->ticksp()); + iterateAndNextConstNull(nodep->ticksp()); } puts(")"); } virtual void visit(AstReturn* nodep) override { putfs(nodep, "return "); - iterateAndNextNull(nodep->lhsp()); + iterateAndNextConstNull(nodep->lhsp()); puts(";\n"); } virtual void visit(AstStop* nodep) override { putfs(nodep, "$stop;\n"); } @@ -401,22 +401,22 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { virtual void visit(AstScopeName* nodep) override {} virtual void visit(AstCStmt* nodep) override { putfs(nodep, "$_CSTMT("); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); puts(");\n"); } virtual void visit(AstCMath* nodep) override { putfs(nodep, "$_CMATH("); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); puts(");\n"); } virtual void visit(AstUCStmt* nodep) override { putfs(nodep, "$c("); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); puts(");\n"); } virtual void visit(AstUCFunc* nodep) override { putfs(nodep, "$c("); - iterateAndNextNull(nodep->bodysp()); + iterateAndNextConstNull(nodep->bodysp()); puts(")"); } @@ -450,27 +450,27 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { case 'k': putbs(""); break; case 'l': { UASSERT_OBJ(lhsp, nodep, "emitVerilog() references undef node"); - iterateAndNextNull(lhsp); + iterateAndNextConstNull(lhsp); break; } case 'r': { UASSERT_OBJ(rhsp, nodep, "emitVerilog() references undef node"); - iterateAndNextNull(rhsp); + iterateAndNextConstNull(rhsp); break; } case 't': { UASSERT_OBJ(thsp, nodep, "emitVerilog() references undef node"); - iterateAndNextNull(thsp); + iterateAndNextConstNull(thsp); break; } case 'o': { UASSERT_OBJ(thsp, nodep, "emitVerilog() references undef node"); - iterateAndNextNull(fhsp); + iterateAndNextConstNull(fhsp); break; } case 'd': { UASSERT_OBJ(nodep->dtypep(), nodep, "emitVerilog() references undef node"); - iterateAndNextNull(nodep->dtypep()); + iterateAndNextConstNull(nodep->dtypep()); break; } default: nodep->v3fatalSrc("Unknown emitVerilog format code: %" << c); break; @@ -494,10 +494,10 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } virtual void visit(AstAttrOf* nodep) override { putfs(nodep, "$_ATTROF("); - iterateAndNextNull(nodep->fromp()); + iterateAndNextConstNull(nodep->fromp()); if (nodep->dimp()) { putbs(", "); - iterateAndNextNull(nodep->dimp()); + iterateAndNextConstNull(nodep->dimp()); } puts(")"); } @@ -516,11 +516,11 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } virtual void visit(AstNodeCond* nodep) override { putbs("("); - iterateAndNextNull(nodep->condp()); + iterateAndNextConstNull(nodep->condp()); putfs(nodep, " ? "); - iterateAndNextNull(nodep->expr1p()); + iterateAndNextConstNull(nodep->expr1p()); putbs(" : "); - iterateAndNextNull(nodep->expr2p()); + iterateAndNextConstNull(nodep->expr2p()); puts(")"); } virtual void visit(AstRange* nodep) override { @@ -532,21 +532,21 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { puts(cvtToStr(nodep->rightConst())); puts("]"); } else { - iterateAndNextNull(nodep->leftp()); + iterateAndNextConstNull(nodep->leftp()); puts(":"); - iterateAndNextNull(nodep->rightp()); + iterateAndNextConstNull(nodep->rightp()); puts("]"); } } virtual void visit(AstSel* nodep) override { - iterateAndNextNull(nodep->fromp()); + iterateAndNextConstNull(nodep->fromp()); puts("["); if (VN_IS(nodep->lsbp(), Const)) { if (nodep->widthp()->isOne()) { if (VN_IS(nodep->lsbp(), Const)) { puts(cvtToStr(VN_AS(nodep->lsbp(), Const)->toSInt())); } else { - iterateAndNextNull(nodep->lsbp()); + iterateAndNextConstNull(nodep->lsbp()); } } else { puts(cvtToStr(VN_AS(nodep->lsbp(), Const)->toSInt() @@ -555,20 +555,20 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { puts(cvtToStr(VN_AS(nodep->lsbp(), Const)->toSInt())); } } else { - iterateAndNextNull(nodep->lsbp()); + iterateAndNextConstNull(nodep->lsbp()); putfs(nodep, "+:"); - iterateAndNextNull(nodep->widthp()); + iterateAndNextConstNull(nodep->widthp()); puts("]"); } puts("]"); } virtual void visit(AstSliceSel* nodep) override { - iterateAndNextNull(nodep->fromp()); + iterateAndNextConstNull(nodep->fromp()); puts(cvtToStr(nodep->declRange())); } virtual void visit(AstTypedef* nodep) override { putfs(nodep, "typedef "); - iterateAndNextNull(nodep->dtypep()); + iterateAndNextConstNull(nodep->dtypep()); puts(" "); puts(nodep->prettyName()); puts(";\n"); @@ -578,7 +578,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { putfs(nodep, nodep->prettyName()); if (nodep->rangep()) { puts(" "); - iterateAndNextNull(nodep->rangep()); + iterateAndNextConstNull(nodep->rangep()); puts(" "); } else if (nodep->isRanged()) { puts(" ["); @@ -592,14 +592,14 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } virtual void visit(AstNodeArrayDType* nodep) override { iterate(nodep->subDTypep()); - iterateAndNextNull(nodep->rangep()); + iterateAndNextConstNull(nodep->rangep()); } virtual void visit(AstNodeUOrStructDType* nodep) override { puts(nodep->verilogKwd() + " "); if (nodep->packed()) puts("packed "); puts("\n"); puts("{"); - iterateAndNextNull(nodep->membersp()); + iterateAndNextConstNull(nodep->membersp()); puts("}"); } virtual void visit(AstMemberDType* nodep) override { @@ -616,10 +616,10 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { putfs(nodep, nodep->prettyName()); } puts("("); - iterateAndNextNull(nodep->pinsp()); + iterateAndNextConstNull(nodep->pinsp()); puts(")"); } - virtual void visit(AstArg* nodep) override { iterateAndNextNull(nodep->exprp()); } + virtual void visit(AstArg* nodep) override { iterateAndNextConstNull(nodep->exprp()); } virtual void visit(AstPrintTimeScale* nodep) override { puts(nodep->verilogKwd()); puts(";\n"); @@ -654,8 +654,8 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { virtual void visit(AstConst* nodep) override { putfs(nodep, nodep->num().ascii(true, true)); } // Just iterate - virtual void visit(AstTopScope* nodep) override { iterateChildren(nodep); } - virtual void visit(AstScope* nodep) override { iterateChildren(nodep); } + virtual void visit(AstTopScope* nodep) override { iterateChildrenConst(nodep); } + virtual void visit(AstScope* nodep) override { iterateChildrenConst(nodep); } virtual void visit(AstVar* nodep) override { if (nodep->isIO()) { putfs(nodep, nodep->verilogKwd()); @@ -686,7 +686,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } virtual void visit(AstActive* nodep) override { m_sensesp = nodep->sensesp(); - iterateAndNextNull(nodep->stmtsp()); + iterateAndNextConstNull(nodep->stmtsp()); m_sensesp = nullptr; } virtual void visit(AstParseRef* nodep) override { puts(nodep->prettyName()); } @@ -700,7 +700,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { // Default virtual void visit(AstNode* nodep) override { puts(string("\n???? // ") + nodep->prettyTypeName() + "\n"); - iterateChildren(nodep); + iterateChildrenConst(nodep); // Not v3fatalSrc so we keep processing if (!m_suppressUnknown) { nodep->v3error( @@ -754,10 +754,10 @@ class EmitVStreamVisitor final : public EmitVBaseVisitor { virtual void putqs(AstNode*, const string& str) override { putbs(str); } public: - EmitVStreamVisitor(AstNode* nodep, std::ostream& os) + EmitVStreamVisitor(const AstNode* nodep, std::ostream& os) : EmitVBaseVisitor{false, nullptr} , m_os(os) { // Need () or GCC 4.8 false warning - iterate(nodep); + iterate(const_cast(nodep)); } virtual ~EmitVStreamVisitor() override = default; }; @@ -828,12 +828,12 @@ class EmitVPrefixedVisitor final : public EmitVBaseVisitor { } public: - EmitVPrefixedVisitor(AstNode* nodep, std::ostream& os, const string& prefix, int flWidth, + EmitVPrefixedVisitor(const AstNode* nodep, std::ostream& os, const string& prefix, int flWidth, AstSenTree* domainp, bool user3mark) : EmitVBaseVisitor{false, domainp} , m_formatter{os, prefix, flWidth} { if (user3mark) VNUser3InUse::check(); - iterate(nodep); + iterate(const_cast(nodep)); } virtual ~EmitVPrefixedVisitor() override = default; }; @@ -841,11 +841,11 @@ public: //###################################################################### // EmitV class functions -void V3EmitV::verilogForTree(AstNode* nodep, std::ostream& os) { +void V3EmitV::verilogForTree(const AstNode* nodep, std::ostream& os) { { EmitVStreamVisitor{nodep, os}; } } -void V3EmitV::verilogPrefixedTree(AstNode* nodep, std::ostream& os, const string& prefix, +void V3EmitV::verilogPrefixedTree(const AstNode* nodep, std::ostream& os, const string& prefix, int flWidth, AstSenTree* domainp, bool user3mark) { { EmitVPrefixedVisitor{nodep, os, prefix, flWidth, domainp, user3mark}; } } diff --git a/src/V3EmitV.h b/src/V3EmitV.h index e2be6ab25..6e74823ae 100644 --- a/src/V3EmitV.h +++ b/src/V3EmitV.h @@ -27,8 +27,8 @@ class AstSenTree; class V3EmitV final { public: - static void verilogForTree(AstNode* nodep, std::ostream& os = std::cout); - static void verilogPrefixedTree(AstNode* nodep, std::ostream& os, const string& prefix, + static void verilogForTree(const AstNode* nodep, std::ostream& os = std::cout); + static void verilogPrefixedTree(const AstNode* nodep, std::ostream& os, const string& prefix, int flWidth, AstSenTree* domainp, bool user3mark); static void emitvFiles(); static void debugEmitV(const string& filename); From a9cd2998e5193bd81a539af7630141053e5897a9 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 23 Apr 2022 14:06:26 +0100 Subject: [PATCH 07/10] Don't mangle run-time library method names. --- Changes | 1 + src/V3EmitCFunc.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Changes b/Changes index 8a8a34272..016f91f50 100644 --- a/Changes +++ b/Changes @@ -22,6 +22,7 @@ Verilator 4.221 devel * Fix filenames with dots overwriting debug .vpp files (#3373). * Fix including VK_USER_OBJS in make library (#3370). [Julien Margetts] * Fix crash in recursive module inlining (#3393). [david-sawatzke] +* Fix --protect-ids mangling names of library methods. [Geza Lore, Shunyao CAD] Verilator 4.220 2022-03-12 ========================== diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index 127bf1032..4bdfb1c57 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -406,7 +406,7 @@ public: virtual void visit(AstCMethodHard* nodep) override { iterate(nodep->fromp()); puts("."); - puts(nodep->nameProtect()); + puts(nodep->name()); puts("("); bool comma = false; for (AstNode* subnodep = nodep->pinsp(); subnodep; subnodep = subnodep->nextp()) { From b22e368b25c5afcb5985b84b6e3223decbf3e340 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 23 Apr 2022 14:16:19 +0100 Subject: [PATCH 08/10] Add default parameters to some Ast nodes for convenience Also update usage to utilize. No functional change. --- src/V3Assert.cpp | 12 +++++------- src/V3AstNodes.h | 8 ++++---- src/V3Case.cpp | 5 ++--- src/V3Clock.cpp | 6 +++--- src/V3Delayed.cpp | 3 +-- src/V3Descope.cpp | 2 +- src/V3MergeCond.cpp | 2 +- src/V3Table.cpp | 2 +- src/V3Trace.cpp | 2 +- src/V3Unknown.cpp | 3 +-- src/V3Width.cpp | 40 +++++++++++++++++----------------------- src/verilog.y | 4 ++-- 12 files changed, 39 insertions(+), 50 deletions(-) diff --git a/src/V3Assert.cpp b/src/V3Assert.cpp index eaf8e3e71..dd58d3674 100644 --- a/src/V3Assert.cpp +++ b/src/V3Assert.cpp @@ -98,7 +98,7 @@ private: ? static_cast( new AstCMath(fl, "vlSymsp->_vm_contextp__->assertOn()", 1)) : static_cast(new AstConst(fl, AstConst::BitFalse())))), - nodep, nullptr); + nodep); newp->user1(true); // Don't assert/cover this if return newp; } @@ -154,7 +154,7 @@ private: } if (bodysp && passsp) bodysp = bodysp->addNext(passsp); - ifp = new AstIf(nodep->fileline(), propp, bodysp, nullptr); + ifp = new AstIf(nodep->fileline(), propp, bodysp); bodysp = ifp; } else if (VN_IS(nodep, Assert) || VN_IS(nodep, AssertIntrinsic)) { if (nodep->immediate()) { @@ -313,8 +313,7 @@ private: AstIf* const ifp = new AstIf( nodep->fileline(), new AstLogNot(nodep->fileline(), ohot), newFireAssert(nodep, - "synthesis parallel_case, but multiple matches found"), - nullptr); + "synthesis parallel_case, but multiple matches found")); ifp->branchPred(VBranchPred::BP_UNLIKELY); nodep->addNotParallelp(ifp); } @@ -384,7 +383,7 @@ private: new AstLogAnd{fl, new AstLogNot{fl, newMonitorOffVarRefp(nodep, VAccess::READ)}, new AstEq{fl, new AstConst{fl, monNum}, newMonitorNumVarRefp(nodep, VAccess::READ)}}, - stmtsp, nullptr}; + stmtsp}; ifp->branchPred(VBranchPred::BP_UNLIKELY); AstNode* const newp = new AstAlwaysPostponed{fl, ifp}; m_modp->addStmtp(newp); @@ -402,8 +401,7 @@ private: nodep->replaceWith(newsetp); // Add "always_comb if (__Vstrobe) begin $display(...); __Vstrobe = '0; end" AstNode* const stmtsp = nodep; - AstIf* const ifp - = new AstIf{fl, new AstVarRef{fl, varp, VAccess::READ}, stmtsp, nullptr}; + AstIf* const ifp = new AstIf{fl, new AstVarRef{fl, varp, VAccess::READ}, stmtsp}; ifp->branchPred(VBranchPred::BP_UNLIKELY); AstNode* const newp = new AstAlwaysPostponed{fl, ifp}; stmtsp->addNext(new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index e1397b810..1c86aad29 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -1914,14 +1914,14 @@ private: bool m_pure = false; // Pure optimizable public: AstCMethodHard(FileLine* fl, AstNode* fromp, VFlagChildDType, const string& name, - AstNode* pinsp) + AstNode* pinsp = nullptr) : ASTGEN_SUPER_CMethodHard(fl, false) , m_name{name} { setOp1p(fromp); dtypep(nullptr); // V3Width will resolve addNOp2p(pinsp); } - AstCMethodHard(FileLine* fl, AstNode* fromp, const string& name, AstNode* pinsp) + AstCMethodHard(FileLine* fl, AstNode* fromp, const string& name, AstNode* pinsp = nullptr) : ASTGEN_SUPER_CMethodHard(fl, false) , m_name{name} { setOp1p(fromp); @@ -4610,7 +4610,7 @@ public: class AstWhile final : public AstNodeStmt { public: - AstWhile(FileLine* fl, AstNode* condp, AstNode* bodysp, AstNode* incsp = nullptr) + AstWhile(FileLine* fl, AstNode* condp, AstNode* bodysp = nullptr, AstNode* incsp = nullptr) : ASTGEN_SUPER_While(fl) { setOp2p(condp); addNOp3p(bodysp); @@ -4714,7 +4714,7 @@ private: bool m_unique0Pragma; // unique0 case bool m_priorityPragma; // priority case public: - AstIf(FileLine* fl, AstNode* condp, AstNode* ifsp, AstNode* elsesp = nullptr) + AstIf(FileLine* fl, AstNode* condp, AstNode* ifsp = nullptr, AstNode* elsesp = nullptr) : ASTGEN_SUPER_If(fl, condp, ifsp, elsesp) { m_uniquePragma = false; m_unique0Pragma = false; diff --git a/src/V3Case.cpp b/src/V3Case.cpp index 403b785b0..161f7db7e 100644 --- a/src/V3Case.cpp +++ b/src/V3Case.cpp @@ -427,8 +427,7 @@ private: if (++depth > CASE_ENCODER_GROUP_DEPTH) depth = 1; if (depth == 1) { // First group or starting new group itemnextp = nullptr; - AstIf* const newp - = new AstIf(itemp->fileline(), ifexprp->cloneTree(true), nullptr, nullptr); + AstIf* const newp = new AstIf(itemp->fileline(), ifexprp->cloneTree(true)); if (groupnextp) { groupnextp->addElsesp(newp); } else { @@ -448,7 +447,7 @@ private: VL_DO_DANGLING(itemexprp->deleteTree(), itemexprp); itemexprp = new AstConst(itemp->fileline(), AstConst::BitTrue()); } - AstIf* const newp = new AstIf(itemp->fileline(), itemexprp, istmtsp, nullptr); + AstIf* const newp = new AstIf(itemp->fileline(), itemexprp, istmtsp); if (itemnextp) { itemnextp->addElsesp(newp); } else { diff --git a/src/V3Clock.cpp b/src/V3Clock.cpp index a9072fe29..a8b8f7006 100644 --- a/src/V3Clock.cpp +++ b/src/V3Clock.cpp @@ -185,7 +185,7 @@ private: AstIf* makeActiveIf(AstSenTree* sensesp) { AstNode* const senEqnp = createSenseEquation(sensesp->sensesp()); UASSERT_OBJ(senEqnp, sensesp, "No sense equation, shouldn't be in sequent activation."); - AstIf* const newifp = new AstIf(sensesp->fileline(), senEqnp, nullptr, nullptr); + AstIf* const newifp = new AstIf(sensesp->fileline(), senEqnp); return newifp; } void clearLastSen() { @@ -310,8 +310,8 @@ private: AstNode* const origp = nodep->origp()->unlinkFrBack(); AstNode* const changeWrp = nodep->changep()->unlinkFrBack(); AstNode* const changeRdp = ConvertWriteRefsToRead::main(changeWrp->cloneTree(false)); - AstIf* const newp = new AstIf( - nodep->fileline(), new AstXor(nodep->fileline(), origp, changeRdp), incp, nullptr); + AstIf* const newp + = new AstIf(nodep->fileline(), new AstXor(nodep->fileline(), origp, changeRdp), incp); // We could add another IF to detect posedges, and only increment if so. // It's another whole branch though versus a potential memory miss. // We'll go with the miss. diff --git a/src/V3Delayed.cpp b/src/V3Delayed.cpp index f16223128..57fe02833 100644 --- a/src/V3Delayed.cpp +++ b/src/V3Delayed.cpp @@ -351,8 +351,7 @@ private: "Delayed assignment misoptimized; prev var found w/o associated IF"); } else { postLogicp = new AstIf(nodep->fileline(), - new AstVarRef(nodep->fileline(), setvscp, VAccess::READ), - nullptr, nullptr); + new AstVarRef(nodep->fileline(), setvscp, VAccess::READ)); UINFO(9, " Created " << postLogicp << endl); finalp->addStmtp(postLogicp); finalp->user3p(setvscp); // Remember IF's vset variable diff --git a/src/V3Descope.cpp b/src/V3Descope.cpp index 3d13cdb8e..9e6aea78f 100644 --- a/src/V3Descope.cpp +++ b/src/V3Descope.cpp @@ -161,7 +161,7 @@ private: new AstCMath(funcp->fileline(), string("&(") + funcp->scopep()->nameVlSym() + ")", 64)), - returnp, nullptr); + returnp); newfuncp->addStmtsp(ifp); } else { newfuncp->addStmtsp(returnp); diff --git a/src/V3MergeCond.cpp b/src/V3MergeCond.cpp index 767d7a30f..77b2ea935 100644 --- a/src/V3MergeCond.cpp +++ b/src/V3MergeCond.cpp @@ -266,7 +266,7 @@ private: // and we also need to keep track of it for comparisons later. m_mgCondp = m_mgCondp->cloneTree(false); // Create equivalent 'if' statement and insert it before the first node - AstIf* const resultp = new AstIf(m_mgCondp->fileline(), m_mgCondp, nullptr, nullptr); + AstIf* const resultp = new AstIf(m_mgCondp->fileline(), m_mgCondp); m_mgFirstp->addHereThisAsNext(resultp); // Unzip the list and insert under branches AstNode* nextp = m_mgFirstp; diff --git a/src/V3Table.cpp b/src/V3Table.cpp index 3233e39fe..822407b4c 100644 --- a/src/V3Table.cpp +++ b/src/V3Table.cpp @@ -367,7 +367,7 @@ private: AstNode* const condp = new AstAnd(fl, select(fl, outputAssignedTableVscp, indexVscp), new AstConst(fl, outputChgMask)); - outsetp = new AstIf(fl, condp, outsetp, nullptr); + outsetp = new AstIf(fl, condp, outsetp); } stmtsp->addNext(outsetp); diff --git a/src/V3Trace.cpp b/src/V3Trace.cpp index 82d688252..cffd498f4 100644 --- a/src/V3Trace.cpp +++ b/src/V3Trace.cpp @@ -656,7 +656,7 @@ private: condp = condp ? new AstOr(flp, condp, selp) : selp; } } - ifp = new AstIf(flp, condp, nullptr, nullptr); + ifp = new AstIf(flp, condp); if (!always) ifp->branchPred(VBranchPred::BP_UNLIKELY); subFuncp->addStmtsp(ifp); subStmts += ifp->nodeCount(); diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index d71317e58..0df1ff3a6 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -127,8 +127,7 @@ private: (needDly ? static_cast( new AstAssignDly(fl, prep, new AstVarRef(fl, varp, VAccess::READ))) : static_cast( - new AstAssign(fl, prep, new AstVarRef(fl, varp, VAccess::READ)))), - nullptr); + new AstAssign(fl, prep, new AstVarRef(fl, varp, VAccess::READ))))); newp->branchPred(VBranchPred::BP_LIKELY); newp->isBoundsCheck(true); if (debug() >= 9) newp->dumpTree(cout, " _new: "); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index e968decd3..f4c350d6e 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1366,7 +1366,7 @@ private: switch (nodep->attrType()) { case VAttrType::DIM_SIZE: { AstNode* const newp = new AstCMethodHard( - nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size", nullptr); + nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size"); newp->dtypeSetSigned32(); newp->didWidth(true); newp->protect(false); @@ -1384,7 +1384,7 @@ private: case VAttrType::DIM_RIGHT: case VAttrType::DIM_HIGH: { AstNode* const sizep = new AstCMethodHard( - nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size", nullptr); + nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size"); sizep->dtypeSetSigned32(); sizep->didWidth(true); sizep->protect(false); @@ -2676,8 +2676,8 @@ private: if (nodep->name() == "num" // function int num() || nodep->name() == "size") { methodOkArguments(nodep, 0, 0); - newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size", - nullptr); // So don't need num() + newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), + "size"); // So don't need num() newp->dtypeSetSigned32(); } else if (nodep->name() == "first" // function int first(ref index) || nodep->name() == "last" // @@ -2703,7 +2703,7 @@ private: methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); if (!nodep->pinsp()) { newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), - "clear", nullptr); + "clear"); newp->makeStatement(); } else { AstNode* const index_exprp = methodCallAssocIndexExpr(nodep, adtypep); @@ -2731,7 +2731,7 @@ private: methodOkArguments(nodep, 0, 0); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::READ); newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), - nodep->name(), nullptr); + nodep->name()); if (nodep->name() == "unique_index") { newp->dtypep(queueDTypeIndexedBy(adtypep->keyDTypep())); } else { @@ -2794,19 +2794,16 @@ private: if (nodep->name() == "at") { // Created internally for [] methodOkArguments(nodep, 1, 1); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); - newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "at", - nullptr); + newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "at"); newp->dtypeFrom(adtypep->subDTypep()); } else if (nodep->name() == "size") { methodOkArguments(nodep, 0, 0); - newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size", - nullptr); + newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size"); newp->dtypeSetSigned32(); } else if (nodep->name() == "delete") { // function void delete() methodOkArguments(nodep, 0, 0); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); - newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "clear", - nullptr); + newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "clear"); newp->makeStatement(); } else if (nodep->name() == "and" || nodep->name() == "or" || nodep->name() == "xor" || nodep->name() == "sum" || nodep->name() == "product") { @@ -2837,7 +2834,7 @@ private: methodOkArguments(nodep, 0, 0); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::READ); newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), - nodep->name(), nullptr); + nodep->name()); if (nodep->name() == "unique_index") { newp->dtypep(newp->findQueueIndexDType()); } else { @@ -2883,27 +2880,25 @@ private: if (nodep->name() == "at") { // Created internally for [] methodOkArguments(nodep, 1, 1); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); - newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "at", - nullptr); + newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "at"); newp->dtypeFrom(adtypep->subDTypep()); } else if (nodep->name() == "num" // function int num() || nodep->name() == "size") { methodOkArguments(nodep, 0, 0); - newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size", - nullptr); + newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), "size"); newp->dtypeSetSigned32(); } else if (nodep->name() == "delete") { // function void delete([input integer index]) methodOkArguments(nodep, 0, 1); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); if (!nodep->pinsp()) { newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), - "clear", nullptr); + "clear"); newp->makeStatement(); } else { AstNode* const index_exprp = methodCallQueueIndexExpr(nodep); if (index_exprp->isZero()) { // delete(0) is a pop_front newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), - "pop_front", nullptr); + "pop_front"); newp->dtypeFrom(adtypep->subDTypep()); newp->makeStatement(); } else { @@ -2933,7 +2928,7 @@ private: // Returns element, so method both consumes (reads) and modifies the queue methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::READWRITE); newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), - nodep->name(), nullptr); + nodep->name()); newp->dtypeFrom(adtypep->subDTypep()); if (!nodep->firstAbovep()) newp->makeStatement(); } else if (nodep->name() == "push_back" || nodep->name() == "push_front") { @@ -2972,7 +2967,7 @@ private: methodOkArguments(nodep, 0, 0); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::READ); newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), - nodep->name(), nullptr); + nodep->name()); if (nodep->name() == "unique_index") { newp->dtypep(newp->findQueueIndexDType()); } else { @@ -3850,8 +3845,7 @@ private: } else if (VN_IS(fromDtp, DynArrayDType) || VN_IS(fromDtp, QueueDType)) { if (varp) { auto* const leftp = new AstConst{fl, AstConst::Signed32{}, 0}; - auto* const sizep - = new AstCMethodHard{fl, fromp->cloneTree(false), "size", nullptr}; + auto* const sizep = new AstCMethodHard{fl, fromp->cloneTree(false), "size"}; sizep->dtypeSetSigned32(); sizep->didWidth(true); sizep->protect(false); diff --git a/src/verilog.y b/src/verilog.y index a7230452e..7c660fc62 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -3111,7 +3111,7 @@ statement_item: // IEEE: statement_item // // // IEEE: conditional_statement | unique_priorityE yIF '(' expr ')' stmtBlock %prec prLOWER_THAN_ELSE - { AstIf* const newp = new AstIf{$2, $4, $6, nullptr}; + { AstIf* const newp = new AstIf{$2, $4, $6}; $$ = newp; if ($1 == uniq_UNIQUE) newp->uniquePragma(true); if ($1 == uniq_UNIQUE0) newp->unique0Pragma(true); @@ -3180,7 +3180,7 @@ statement_item: // IEEE: statement_item $$ = $2->cloneTree(true); $$->addNext(new AstWhile($1,$5,$2)); } - else $$ = new AstWhile($1,$5,nullptr); } + else $$ = new AstWhile($1,$5); } // // IEEE says array_identifier here, but dotted accepted in VMM and 1800-2009 | yFOREACH '(' idClassSelForeach ')' stmtBlock { $$ = new AstForeach($1, $3, $5); } // From 9abab2c36668d363e7756a6f91325b97ea3d074b Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 23 Apr 2022 14:44:48 +0100 Subject: [PATCH 09/10] Add separate AstInitialStatic node for static initializers Static variable initializers run before initial blocks, so use an explicitly different procedure type for them. This also enables us to now raise errors for assignments to const variables in initial blocks. --- Changes | 1 + src/V3Active.cpp | 8 ++++++++ src/V3AstNodes.h | 11 ++++++++++- src/V3Cdc.cpp | 1 + src/V3Class.cpp | 7 ++++++- src/V3EmitV.cpp | 1 + src/V3LinkParse.cpp | 2 +- src/V3Order.cpp | 3 +++ src/V3Width.cpp | 3 ++- 9 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Changes b/Changes index 016f91f50..acac7dee0 100644 --- a/Changes +++ b/Changes @@ -15,6 +15,7 @@ Verilator 4.221 devel * Split --prof-threads into --prof-exec and --prof-pgo (#3365). [Geza Lore, Shunyao CAD] * Deprecate 'vluint64_t' and similar types (#3255). +* Raise error on assignment to const in initial blocks. [Geza Lore, Shunyao CAD] * Fix MSVC localtime_s (#3124). * Fix Bison 3.8.2 error (#3366). [elike-ypq] * Fix rare bug in -Oz (V3Localize) (#3286). [Geza Lore, Shunyao CAD] diff --git a/src/V3Active.cpp b/src/V3Active.cpp index 043f05233..a0ed963ef 100644 --- a/src/V3Active.cpp +++ b/src/V3Active.cpp @@ -422,6 +422,14 @@ private: virtual void visit(AstActive* nodep) override { // Actives are being formed, so we can ignore any already made } + virtual void visit(AstInitialStatic* nodep) override { + // Relink to IACTIVE, unless already under it + UINFO(4, " INITIAL " << nodep << endl); + const ActiveDlyVisitor dlyvisitor{nodep, ActiveDlyVisitor::CT_INITIAL}; + AstActive* const wantactivep = m_namer.getIActive(nodep->fileline()); + nodep->unlinkFrBack(); + wantactivep->addStmtsp(nodep); + } virtual void visit(AstInitial* nodep) override { // Relink to IACTIVE, unless already under it UINFO(4, " INITIAL " << nodep << endl); diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 1c86aad29..5082ae08d 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -3407,7 +3407,7 @@ public: }; class AstInitialAutomatic final : public AstNodeProcedure { - // initial for automatic variables + // Automatic variable initialization // That is, it runs every function start, or class construction public: AstInitialAutomatic(FileLine* fl, AstNode* bodysp) @@ -3415,6 +3415,15 @@ public: ASTNODE_NODE_FUNCS(InitialAutomatic) }; +class AstInitialStatic final : public AstNodeProcedure { + // Static variable initialization + // That is, it runs at the beginning of simulation, before 'initial' blocks +public: + AstInitialStatic(FileLine* fl, AstNode* bodysp) + : ASTGEN_SUPER_InitialStatic(fl, bodysp) {} + ASTNODE_NODE_FUNCS(InitialStatic) +}; + class AstAlways final : public AstNodeProcedure { const VAlwaysKwd m_keyword; diff --git a/src/V3Cdc.cpp b/src/V3Cdc.cpp index 871a5fd8c..acb305ab0 100644 --- a/src/V3Cdc.cpp +++ b/src/V3Cdc.cpp @@ -716,6 +716,7 @@ private: // Ignores virtual void visit(AstInitial*) override {} virtual void visit(AstInitialAutomatic*) override {} + virtual void visit(AstInitialStatic*) override {} virtual void visit(AstTraceDecl*) override {} virtual void visit(AstCoverToggle*) override {} virtual void visit(AstNodeDType*) override {} diff --git a/src/V3Class.cpp b/src/V3Class.cpp index c7bbc91c3..6a7a3ce4f 100644 --- a/src/V3Class.cpp +++ b/src/V3Class.cpp @@ -153,6 +153,11 @@ private: iterateChildren(nodep); if (m_packageScopep) { m_toScopeMoves.push_back(std::make_pair(nodep, m_packageScopep)); } } + virtual void visit(AstInitialStatic* nodep) override { + // But not AstInitialAutomatic, which remains under the class + iterateChildren(nodep); + if (m_packageScopep) { m_toScopeMoves.push_back(std::make_pair(nodep, m_packageScopep)); } + } virtual void visit(AstNodeMath* nodep) override {} // Short circuit virtual void visit(AstNodeStmt* nodep) override {} // Short circuit @@ -173,7 +178,7 @@ public: vscp->scopep(scopep); vscp->unlinkFrBack(); scopep->addVarp(vscp); - } else if (VN_IS(nodep, Initial)) { + } else if (VN_IS(nodep, Initial) || VN_IS(nodep, InitialStatic)) { nodep->unlinkFrBack(); scopep->addActivep(nodep); } else { diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index e04c15e4d..594795fa5 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -99,6 +99,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { putqs(nodep, "end\n"); } virtual void visit(AstInitialAutomatic* nodep) override { iterateChildrenConst(nodep); } + virtual void visit(AstInitialStatic* nodep) override { iterateChildrenConst(nodep); } virtual void visit(AstAlways* nodep) override { putfs(nodep, "always "); if (m_sensesp) { diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp index b99b8e3a9..df1e36436 100644 --- a/src/V3LinkParse.cpp +++ b/src/V3LinkParse.cpp @@ -275,7 +275,7 @@ private: if (nodep->lifetime().isAutomatic()) { nodep->addNextHere(new AstInitialAutomatic{newfl, assp}); } else { - nodep->addNextHere(new AstInitial{newfl, assp}); + nodep->addNextHere(new AstInitialStatic{newfl, assp}); } } // 4. Under blocks, it's an initial value to be under an assign else { diff --git a/src/V3Order.cpp b/src/V3Order.cpp index 090e81b7d..9007ce8f2 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -662,6 +662,9 @@ class OrderBuildVisitor final : public VNVisitor { virtual void visit(AstInitialAutomatic* nodep) override { // iterateLogic(nodep); } + virtual void visit(AstInitialStatic* nodep) override { // + iterateLogic(nodep); + } virtual void visit(AstAlways* nodep) override { // iterateLogic(nodep); } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index f4c350d6e..45a78ac8c 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -2041,7 +2041,8 @@ private: if (nodep->access().isWriteOrRW() && nodep->varp()->direction() == VDirection::CONSTREF) { nodep->v3error("Assigning to const ref variable: " << nodep->prettyNameQ()); } else if (nodep->access().isWriteOrRW() && nodep->varp()->isConst() && !m_paramsOnly - && (!m_ftaskp || !m_ftaskp->isConstructor()) && !VN_IS(m_procedurep, Initial)) { + && (!m_ftaskp || !m_ftaskp->isConstructor()) + && !VN_IS(m_procedurep, InitialStatic)) { // Too loose, but need to allow our generated first assignment // Move this to a property of the AstInitial block nodep->v3error("Assigning to const variable: " << nodep->prettyNameQ()); From 62337bb6accc133e588077b751598a77df80a77a Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 23 Apr 2022 14:46:39 +0100 Subject: [PATCH 10/10] Future proofing some tests. No functional change. --- test_regress/t/t_c_this.pl | 23 ++++++++++++++++------- test_regress/t/t_event.v | 4 ++-- test_regress/t/t_order_multialways.v | 4 ++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/test_regress/t/t_c_this.pl b/test_regress/t/t_c_this.pl index b8edcffb4..93c3b5200 100755 --- a/test_regress/t/t_c_this.pl +++ b/test_regress/t/t_c_this.pl @@ -8,19 +8,28 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Version 2.0. # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -scenarios(vlt => 1); +scenarios(simulator => 1); compile(); if ($Self->{vlt_all}) { # The word 'this' (but only the whole word 'this' should have been replaced # in the contents. - my $file = glob_one("$Self->{obj_dir}/$Self->{VM_PREFIX}___024root__DepSet_*__0.cpp"); - my $text = file_contents($file); - error("$file has 'this->clk'") if ($text =~ m/\bthis->clk\b/); - error("$file does not have 'xthis'") if ($text !~ m/\bxthis\b/); - error("$file does not have 'thisx'") if ($text !~ m/\bthisx\b/); - error("$file does not have 'xthisx'") if ($text !~ m/\bxthisx\b/); + my $has_this = 0; + my $has_xthis = 0; + my $has_thisx = 0; + my $has_xthisx = 0; + for my $file (glob_all("$Self->{obj_dir}/$Self->{VM_PREFIX}___024root__DepSet_*__0.cpp")) { + my $text = file_contents($file); + $has_this = 1 if ($text =~ m/\bthis->clk\b/); + $has_xthis = 1 if ($text =~ m/\bxthis\b/); + $has_thisx = 1 if ($text =~ m/\bthisx\b/); + $has_xthisx = 1 if ($text =~ m/\bxthisx\b/); + } + error("Some file has 'this->clk'") if $has_this; + error("No file has 'xthis'") if !$has_xthis; + error("No file has 'thisx'") if !$has_thisx; + error("No file has 'xthisx'") if !$has_xthisx; } ok(1); diff --git a/test_regress/t/t_event.v b/test_regress/t/t_event.v index 0f5384c14..9bf376779 100644 --- a/test_regress/t/t_event.v +++ b/test_regress/t/t_event.v @@ -22,9 +22,9 @@ module t(/*AUTOARG*/ event ev [3:0]; `endif - int cyc; + int cyc = 0; - int last_event; + int last_event = 0; always @(e1) begin `WRITE_VERBOSE(("[%0t] e1\n", $time)); if (!e1.triggered) $stop; diff --git a/test_regress/t/t_order_multialways.v b/test_regress/t/t_order_multialways.v index 63f75c5eb..65d500455 100644 --- a/test_regress/t/t_order_multialways.v +++ b/test_regress/t/t_order_multialways.v @@ -25,11 +25,11 @@ module t (/*AUTOARG*/ // verilator lint_off UNOPTFLAT reg [31:0] e2,f2,g2,h2; - always @ (/*AS*/f2) begin + always @ (/*AS*/f2, g2) begin h2 = {g2[15:0], g2[31:16]}; g2 = {f2[15:0], f2[31:16]}; end - always @ (/*AS*/in_a) begin + always @ (/*AS*/in_a, e2) begin f2 = {e2[15:0], e2[31:16]}; e2 = in_a; end