From b1e6fe7139b9026bf07bfbce938174e2c42dd236 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 11 Oct 2009 20:50:31 -0400 Subject: [PATCH] Fix core dump with SystemVerilog var declarations under unnamed begins. --- Changes | 2 + src/V3AstNodes.cpp | 4 ++ src/V3AstNodes.h | 4 ++ src/V3Begin.cpp | 58 +++++++++++++--------- src/V3Link.cpp | 20 +++++++- src/verilog.y | 3 +- test_regress/t/t_var_nonamebegin.out | 37 ++++++++++++++ test_regress/t/t_var_nonamebegin.pl | 32 ++++++++++++ test_regress/t/t_var_nonamebegin.v | 74 ++++++++++++++++++++++++++++ 9 files changed, 209 insertions(+), 25 deletions(-) create mode 100644 test_regress/t/t_var_nonamebegin.out create mode 100644 test_regress/t/t_var_nonamebegin.pl create mode 100644 test_regress/t/t_var_nonamebegin.v diff --git a/Changes b/Changes index a69f7f0ba..547244db7 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,8 @@ indicates the contributor was also the author of the fix; Thanks! **** Fix writing to out-of-bounds arrays writing element 0. +**** Fix core dump with SystemVerilog var declarations under unnamed begins. + **** Fix VCD files showing internal flattened hierarchy, broke in 3.714. **** Fix cell port connection to unsized integer causing false width warning. diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 03900c19f..911d44ebd 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -380,6 +380,10 @@ void AstNodeFTask::dump(ostream& str) { this->AstNode::dump(str); if (taskPublic()) str<<" [PUBLIC]"; } +void AstBegin::dump(ostream& str) { + this->AstNode::dump(str); + if (unnamed()) str<<" [UNNAMED]"; +} void AstCoverDecl::dump(ostream& str) { this->AstNode::dump(str); if (this->dataDeclNullp()) { diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index ed258df02..38c60dbc7 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -702,19 +702,23 @@ struct AstBegin : public AstNode { // Children: statements private: string m_name; // Name of block + bool m_unnamed; // Originally unnamed public: // Node that simply puts name into the output stream AstBegin(FileLine* fileline, const string& name, AstNode* stmtsp) : AstNode(fileline) , m_name(name) { addNOp1p(stmtsp); + m_unnamed = (name==""); } ASTNODE_NODE_FUNCS(Begin, BEGIN) + virtual void dump(ostream& str); virtual string name() const { return m_name; } // * = Block name virtual void name(const string& name) { m_name = name; } // op1 = Statements AstNode* stmtsp() const { return op1p()->castNode(); } // op1 = List of statements void addStmtp(AstNode* nodep) { addOp1p(nodep); } + bool unnamed() const { return m_unnamed; } }; struct AstGenerate : public AstNode { diff --git a/src/V3Begin.cpp b/src/V3Begin.cpp index eb8fea4e8..625069979 100644 --- a/src/V3Begin.cpp +++ b/src/V3Begin.cpp @@ -25,6 +25,9 @@ // BEGIN(VAR...) -> VAR ... {renamed} // FOR -> WHILEs // +// There are two scopes; named BEGINs change %m and variable scopes. +// Unnamed BEGINs change only variable, not $display("%m") scope. +// //************************************************************************* #include "config_build.h" @@ -47,7 +50,8 @@ private: // STATE AstModule* m_modp; // Current module AstNodeFTask* m_ftaskp; // Current function/task - string m_beginScope; // Name of begin blocks above us + string m_namedScope; // Name of begin blocks above us + string m_unnamedScope; // Name of begin blocks, including unnamed blocks int m_repeatNum; // Repeat counter // METHODS @@ -72,24 +76,31 @@ private: virtual void visit(AstBegin* nodep, AstNUser*) { // Begin blocks were only useful in variable creation, change names and delete UINFO(8," "<name() + "__DOT__"; // So always found - string::size_type pos; - while ((pos=dottedname.find("__DOT__")) != string::npos) { - string ident = dottedname.substr(0,pos); - dottedname = dottedname.substr(pos+strlen("__DOT__")); - if (m_beginScope=="") m_beginScope = ident; - else m_beginScope = m_beginScope + "__DOT__"+ident; - // Create CellInline for dotted resolution - AstCellInline* inlinep = new AstCellInline(nodep->fileline(), - m_beginScope, "__BEGIN__"); - m_modp->addInlinesp(inlinep); // Must be parsed before any AstCells + //UINFO(8,"nname "<name() != "") { // Else unneeded unnamed block + // Create data for dotted variable resolution + string dottedname = nodep->name() + "__DOT__"; // So always found + string::size_type pos; + while ((pos=dottedname.find("__DOT__")) != string::npos) { + string ident = dottedname.substr(0,pos); + dottedname = dottedname.substr(pos+strlen("__DOT__")); + if (!nodep->unnamed()) { + if (m_namedScope=="") m_namedScope = ident; + else m_namedScope = m_namedScope + "__DOT__"+ident; + } + if (m_unnamedScope=="") m_unnamedScope = ident; + else m_unnamedScope = m_unnamedScope + "__DOT__"+ident; + // Create CellInline for dotted var resolution + AstCellInline* inlinep = new AstCellInline(nodep->fileline(), + m_unnamedScope, "__BEGIN__"); + m_modp->addInlinesp(inlinep); // Must be parsed before any AstCells + } } - // Remap var names + // Remap var names and replace lower Begins nodep->iterateChildren(*this); if (AstNode* stmtsp = nodep->stmtsp()) { @@ -100,12 +111,13 @@ private: } pushDeletep(nodep); nodep=NULL; } - m_beginScope = oldScope; + m_namedScope = oldScope; + m_unnamedScope = oldUnnamed; } virtual void visit(AstVar* nodep, AstNUser*) { - if (m_beginScope != "") { + if (m_unnamedScope != "") { // Rename it - nodep->name(m_beginScope+"__DOT__"+nodep->name()); + nodep->name(m_unnamedScope+"__DOT__"+nodep->name()); // Move to module nodep->unlinkFrBack(); if (m_ftaskp) m_ftaskp->addStmtsp(nodep); // Begins under funcs just move into the func @@ -114,9 +126,9 @@ private: } virtual void visit(AstCell* nodep, AstNUser*) { UINFO(8," CELL "<name(m_beginScope+"__DOT__"+nodep->name()); + nodep->name(m_namedScope+"__DOT__"+nodep->name()); UINFO(8," rename to "<name()<unlinkFrBack(); @@ -174,11 +186,11 @@ private: virtual void visit(AstScopeName* nodep, AstNUser*) { // If there's a %m in the display text, we add a special node that will contain the name() // Similar code in V3Inline - if (m_beginScope != "") { + if (m_namedScope != "") { // To keep correct visual order, must add before other Text's AstNode* afterp = nodep->scopeAttrp(); if (afterp) afterp->unlinkFrBackWithNext(); - nodep->scopeAttrp(new AstText(nodep->fileline(), (string)"."+AstNode::prettyName(m_beginScope))); + nodep->scopeAttrp(new AstText(nodep->fileline(), (string)"."+AstNode::prettyName(m_namedScope))); if (afterp) nodep->scopeAttrp(afterp); } nodep->iterateChildren(*this); diff --git a/src/V3Link.cpp b/src/V3Link.cpp index 5c69721bf..733d8495d 100644 --- a/src/V3Link.cpp +++ b/src/V3Link.cpp @@ -72,6 +72,7 @@ private: V3SymTable* m_curVarsp; // Symbol table of variables and tasks under table we're inserting into V3SymTable* m_cellVarsp; // Symbol table of variables under cell's module int m_beginNum; // Begin block number, 0=none seen + int m_modBeginNum; // Begin block number in module, 0=none seen bool m_inGenerate; // Inside a generate vector m_delSymps; // Symbol tables to delete @@ -193,6 +194,7 @@ private: m_cellVarsp = NULL; m_paramNum = 0; m_beginNum = 0; + m_modBeginNum = 0; nodep->iterateChildren(*this); // Prep for next m_curVarsp = NULL; @@ -328,8 +330,23 @@ private: ++m_beginNum; nodep->name(nodep->name()+cvtToStr(m_beginNum)); } + if (m_idState==ID_FIND && nodep->name()=="" && nodep->unnamed()) { + // Unnamed blocks are only important when they contain var + // decls, so search for them. (Otherwise adding all the + // unnamed#'s would just confuse tracing variables in + // places such as tasks, where "task ...; begin ... end" + // are common. + for (AstNode* stmtp = nodep->stmtsp(); stmtp; stmtp=stmtp->nextp()) { + if (stmtp->castVar()) { + ++m_modBeginNum; + nodep->name("unnamedblk"+cvtToStr(m_modBeginNum)); + break; + } + } + } + // Check naming (we don't really care, but some tools do, so better to warn) - if (m_idState==ID_FIND) { + if (m_idState==ID_FIND && nodep->name()!="") { findAndInsertAndCheck(nodep, nodep->name()); } // Recurse @@ -480,6 +497,7 @@ public: m_ftaskp = NULL; m_paramNum = 0; m_beginNum = 0; + m_modBeginNum = 0; m_inGenerate = false; // rootp->accept(*this); diff --git a/src/verilog.y b/src/verilog.y index a4d31bb6d..c5e5b550a 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1395,7 +1395,8 @@ stmtBlock: // IEEE: statement + seq_block + par_block seq_block: // ==IEEE: seq_block // // IEEE doesn't allow declarations in unnamed blocks, but several simulators do. - yBEGIN blockDeclStmtList yEND { $$ = $2; } + // // So need begin's even if unnamed to scope variables down + yBEGIN blockDeclStmtList yEND { $$ = new AstBegin($1,"",$2); } | yBEGIN /**/ yEND { $$ = NULL; } | yBEGIN ':' seq_blockId blockDeclStmtList yEND endLabelE { $$ = new AstBegin($2,*$3,$4); } | yBEGIN ':' seq_blockId /**/ yEND endLabelE { $$ = new AstBegin($2,*$3,NULL); } diff --git a/test_regress/t/t_var_nonamebegin.out b/test_regress/t/t_var_nonamebegin.out new file mode 100644 index 000000000..5ad36b686 --- /dev/null +++ b/test_regress/t/t_var_nonamebegin.out @@ -0,0 +1,37 @@ +$version Generated by SpTraceVcd $end +$date Sun Oct 11 20:21:49 2009 + $end +$timescale 1ns $end + + $scope module TOP $end + $var wire 1 # clk $end + $var wire 1 $ reset_l $end + $scope module v $end + $var wire 1 # clk $end + $var wire 1 % inmod $end + $var wire 32 & rawmod [31:0] $end + $var wire 1 $ reset_l $end + $scope module genblk1 $end + $var wire 32 ' ingen [31:0] $end + $upscope $end + $scope module unnamedblk1 $end + $var wire 32 ( upa [31:0] $end + $scope module d3nameda $end + $var wire 32 ) d3a [31:0] $end + $upscope $end + $upscope $end + $scope module unnamedblk2 $end + $var wire 32 * b2 [31:0] $end + $scope module b3named $end + $var wire 32 + b3n [31:0] $end + $upscope $end + $scope module unnamedblk3 $end + $var wire 32 , b3 [31:0] $end + $scope module unnamedblk4 $end + $var wire 32 - b4 [31:0] $end + $upscope $end + $upscope $end + $upscope $end + $upscope $end + $upscope $end +$enddefinitions $end diff --git a/test_regress/t/t_var_nonamebegin.pl b/test_regress/t/t_var_nonamebegin.pl new file mode 100644 index 000000000..d271e4b3a --- /dev/null +++ b/test_regress/t/t_var_nonamebegin.pl @@ -0,0 +1,32 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you can +# redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. + +compile ( + v_flags2 => [$Self->{v3}?'-trace':''], + ); + +execute ( + expect=>quotemeta( +'ingen: {mod}.genblk1 TOP.v.genblk1 +d3a: {mod}.d3nameda TOP.v.d3nameda +b2: {mod} TOP.v +b3n: {mod}.b3named: TOP.v.b3named +b3: {mod} TOP.v +b4: {mod} TOP.v +t1 {mod}.tsk TOP.v +t2 {mod}.tsk TOP.v +*-* All Finished *-*'), + ); + +if ($Self->{v3}) { + vcd_identical ("$Self->{obj_dir}/simx.vcd", + "t/$Self->{name}.out"); +} +ok(1); +1; diff --git a/test_regress/t/t_var_nonamebegin.v b/test_regress/t/t_var_nonamebegin.v new file mode 100644 index 000000000..4adb27c65 --- /dev/null +++ b/test_regress/t/t_var_nonamebegin.v @@ -0,0 +1,74 @@ +module t (/*AUTOARG*/ + // Inputs + clk, reset_l + ); + + input clk; + input reset_l; + + reg inmod; + + generate + if (1) begin + // Traces as genblk1.ingen + integer ingen; + initial $display("ingen: {mod}.genblk1 %m"); + end + endgenerate + + integer rawmod; + + initial begin + begin + integer upa; + begin : d3nameda + // %m='.d3nameda' var=_unnamed#.d3nameda.b1 + integer d3a; + $display("d3a: {mod}.d3nameda %m"); + end + end + end + initial begin + integer b2; + $display("b2: {mod} %m"); + begin : b3named + integer b3n; + $display("b3n: {mod}.b3named: %m"); + end + if (1) begin + integer b3; + $display("b3: {mod} %m"); + if (1) begin + begin + begin + begin + integer b4; + $display("b4: {mod} %m"); + end + end + end + end + else begin + integer b4; + $display("bb %m"); + end + end + else begin + integer b4; + $display("b4 %m"); + end + tsk; + $write("*-* All Finished *-*\n"); + $finish; + end + + task tsk; + integer t1; + $display("t1 {mod}.tsk %m"); + begin + integer t2; + $display("t2 {mod}.tsk %m"); + end + endtask + +endmodule