From 00724597f447e6ebab14b950b30280286750350f Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 14 Dec 2013 16:51:08 -0500 Subject: [PATCH] Fix tracing of packed structs, bug705. --- Changes | 2 + src/V3AstNodes.h | 15 +- src/V3EmitC.cpp | 2 +- src/V3TraceDecl.cpp | 156 +++++++++++++++---- test_regress/t/t_interface1_modport_trace.pl | 21 +++ test_regress/t/t_struct_init_trace.pl | 21 +++ test_regress/t/t_trace_complex.pl | 29 ++++ test_regress/t/t_trace_complex.v | 60 +++++++ 8 files changed, 265 insertions(+), 41 deletions(-) create mode 100755 test_regress/t/t_interface1_modport_trace.pl create mode 100755 test_regress/t/t_struct_init_trace.pl create mode 100755 test_regress/t/t_trace_complex.pl create mode 100644 test_regress/t/t_trace_complex.v diff --git a/Changes b/Changes index afd0011fa..daafa1541 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,8 @@ indicates the contributor was also the author of the fix; Thanks! * Verilator 3.855 devel +**** Fix tracing of packed structs, bug705. [Jie Xu] + **** Fix --lint-only with MinGW, msg1283. [HyungKi Jeong] **** Fix some delayed assignments of typedefed unpacked arrays. diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 90a33155f..3c8e2521d 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -2800,17 +2800,14 @@ private: VNumRange m_arrayRange; // Property of var the trace details uint32_t m_codeInc; // Code increment public: - AstTraceDecl(FileLine* fl, const string& showname, AstVar* varp) + AstTraceDecl(FileLine* fl, const string& showname, AstNode* valuep, + const VNumRange& bitRange, const VNumRange& arrayRange) : AstNodeStmt(fl) - , m_showname(showname) { - dtypeFrom(varp); + , m_showname(showname), m_bitRange(bitRange), m_arrayRange(arrayRange) { + dtypeFrom(valuep); m_code = 0; - m_codeInc = varp->dtypep()->arrayUnpackedElements() * varp->dtypep()->widthWords(); - AstBasicDType* bdtypep = varp->basicp(); - if (bdtypep) m_bitRange = bdtypep->nrange(); - if (AstUnpackArrayDType* adtypep = varp->dtypeSkipRefp()->castUnpackArrayDType()) { - m_arrayRange = adtypep->declRange(); - } + m_codeInc = ((arrayRange.ranged() ? arrayRange.elements() : 1) + * valuep->dtypep()->widthWords()); } virtual int instrCount() const { return 100; } // Large... ASTNODE_NODE_FUNCS(TraceDecl, TRACEDECL) diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index 3c9070c8e..959992399 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -2246,7 +2246,7 @@ class EmitCTrace : EmitCStmts { else if (emitTraceIsScBv(nodep)) puts("VL_SC_BV_DATAP("); varrefp->iterate(*this); // Put var name out // Tracing only supports 1D arrays - if (varp->dtypeSkipRefp()->castUnpackArrayDType()) { + if (nodep->declp()->arrayRange().ranged()) { if (arrayindex==-2) puts("[i]"); else if (arrayindex==-1) puts("[0]"); else puts("["+cvtToStr(arrayindex)+"]"); diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 5c348cf30..5371a875c 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -51,6 +51,9 @@ private: AstCFunc* m_fullFuncp; // Trace function being built AstCFunc* m_chgFuncp; // Trace function being built int m_funcNum; // Function number being built + AstVarScope* m_traVscp; // Signal being trace constructed + AstNode* m_traValuep; // Signal being traced's value to trace in it + string m_traShowname; // Signal being traced's component name V3Double0 m_statSigs; // Statistic tracking V3Double0 m_statIgnSigs; // Statistic tracking @@ -66,22 +69,15 @@ private: // Return true if this shouldn't be traced // See also similar rule in V3Coverage::varIgnoreToggle string prettyName = nodep->prettyName(); - if (!nodep->isTrace()) + if (!nodep->isTrace()) { return "Verilator trace_off"; - if (!v3Global.opt.traceUnderscore()) { + } + else if (!v3Global.opt.traceUnderscore()) { if (prettyName.c_str()[0] == '_') return "Leading underscore"; if (prettyName.find("._") != string::npos) return "Inlined leading underscore"; } - if ((int)nodep->width() > v3Global.opt.traceMaxWidth()) return "Wide bus > --trace-max-width bits"; - if ((int)nodep->dtypep()->arrayUnpackedElements() > v3Global.opt.traceMaxArray()) return "Wide memory > --trace-max-array ents"; - if (!(nodep->dtypeSkipRefp()->castBasicDType() - || (nodep->dtypeSkipRefp()->castUnpackArrayDType() - && (nodep->dtypeSkipRefp()->castUnpackArrayDType()->subDTypep() - ->skipRefp()->castBasicDType())))) { - return "Unsupported: Multi-dimensional array"; - } return NULL; } @@ -109,9 +105,33 @@ private: basep->addStmtsp(callp); return funcp; } - void addCFuncStmt(AstCFunc* basep, AstNode* nodep) { + void addCFuncStmt(AstCFunc* basep, AstNode* nodep, VNumRange arrayRange) { basep->addStmtsp(nodep); } + void addTraceDecl(const VNumRange& arrayRange) { + VNumRange bitRange; + AstBasicDType* bdtypep = m_traValuep->dtypep()->basicp(); + if (bdtypep) bitRange = bdtypep->nrange(); + AstTraceDecl* declp = new AstTraceDecl(m_traVscp->fileline(), m_traShowname, m_traValuep, + bitRange, arrayRange); + + if (m_initSubStmts && v3Global.opt.outputSplitCTrace() + && m_initSubStmts > v3Global.opt.outputSplitCTrace()) { + m_initSubFuncp = newCFuncSub(m_initFuncp); + m_initSubStmts = 0; + } + + m_initSubFuncp->addStmtsp(declp); + m_initSubStmts += EmitCBaseCounterVisitor(declp).count(); + + m_chgFuncp->addStmtsp(new AstTraceInc(m_traVscp->fileline(), declp, m_traValuep->cloneTree(true))); + // The full version will get constructed in V3Trace + } + void addIgnore(const char* why) { + ++m_statIgnSigs; + m_initSubFuncp->addStmtsp( + new AstComment(m_traVscp->fileline(), "Tracing: "+m_traShowname+" // Ignored: "+why)); + } // VISITORS virtual void visit(AstTopScope* nodep, AstNUser*) { @@ -134,35 +154,107 @@ private: // Compute show name // This code assumes SPTRACEVCDC_VERSION >= 1330; // it uses spaces to separate hierarchy components. - string showname = AstNode::vcdName(scopep->name() + " " + varp->name()); - if (showname.substr(0,4) == "TOP ") showname.replace(0,4,""); + m_traShowname = AstNode::vcdName(scopep->name() + " " + varp->name()); + if (m_traShowname.substr(0,4) == "TOP ") m_traShowname.replace(0,4,""); if (!m_initSubFuncp) nodep->v3fatalSrc("NULL"); + + m_traVscp = nodep; + m_traValuep = NULL; if (varIgnoreTrace(varp)) { - ++m_statIgnSigs; - m_initSubFuncp->addStmtsp( - new AstComment(nodep->fileline(), - "Tracing: "+showname+" // Ignored: "+varIgnoreTrace(varp))); + addIgnore(varIgnoreTrace(varp)); } else { ++m_statSigs; - AstNode* valuep = NULL; - if (nodep->valuep()) valuep=nodep->valuep()->cloneTree(true); - else valuep = new AstVarRef(nodep->fileline(), nodep, false); - AstTraceDecl* declp = new AstTraceDecl(nodep->fileline(), showname, varp); - - if (m_initSubStmts && v3Global.opt.outputSplitCTrace() - && m_initSubStmts > v3Global.opt.outputSplitCTrace()) { - m_initSubFuncp = newCFuncSub(m_initFuncp); - m_initSubStmts = 0; + if (nodep->valuep()) m_traValuep = nodep->valuep()->cloneTree(true); + else m_traValuep = new AstVarRef(nodep->fileline(), nodep, false); + { + // Recurse into data type of the signal; the visitors will call addTraceDecl() + varp->dtypeSkipRefp()->accept(*this); } + // Cleanup + if (m_traValuep) { m_traValuep->deleteTree(); m_traValuep=NULL; } + } + m_traVscp = NULL; + m_traValuep = NULL; + m_traShowname = ""; + } + } + // VISITORS - Data types when tracing + virtual void visit(AstConstDType* nodep, AstNUser*) { + if (m_traVscp) { + nodep->subDTypep()->skipRefp()->accept(*this); + } + } + virtual void visit(AstRefDType* nodep, AstNUser*) { + if (m_traVscp) { + nodep->subDTypep()->skipRefp()->accept(*this); + } + } + virtual void visit(AstUnpackArrayDType* nodep, AstNUser*) { + // Note more specific dtypes above + if (m_traVscp) { + if ((int)nodep->arrayUnpackedElements() > v3Global.opt.traceMaxArray()) { + addIgnore("Wide memory > --trace-max-array ents"); + } else if (nodep->subDTypep()->skipRefp()->castBasicDType() // Nothing lower than this array + && m_traVscp->dtypep()->skipRefp() == nodep) { // Nothing above this array + // Simple 1-D array, use exising V3EmitC runtime loop rather than unrolling + // This will put "(index)" at end of signal name for us + addTraceDecl(nodep->declRange()); + } else { + // Unroll now, as have no other method to get right signal names + AstNodeDType* subtypep = nodep->subDTypep()->skipRefp(); + for (int i=nodep->lsb(); i<=nodep->msb(); ++i) { + string oldShowname = m_traShowname; + AstNode* oldValuep = m_traValuep; + { + m_traShowname += string("(")+cvtToStr(i)+string(")"); + m_traValuep = new AstArraySel(nodep->fileline(), m_traValuep->cloneTree(true), + i - nodep->lsb()); - m_initSubFuncp->addStmtsp(declp); - m_initSubStmts += EmitCBaseCounterVisitor(declp).count(); - - m_chgFuncp->addStmtsp(new AstTraceInc(nodep->fileline(), declp, valuep)); - // The full version will get constructed in V3Trace + subtypep->accept(*this); + m_traValuep->deleteTree(); m_traValuep = NULL; + } + m_traShowname = oldShowname; + m_traValuep = oldValuep; + } } } } + virtual void visit(AstPackArrayDType* nodep, AstNUser*) { + if (m_traVscp) { + // Everything downstream is packed, so deal with as one trace unit + // This may not be the nicest for user presentation, but is a much faster way to trace + addTraceDecl(VNumRange()); + } + } + virtual void visit(AstNodeClassDType* nodep, AstNUser*) { + if (m_traVscp) { + if (nodep->packed()) { + // Everything downstream is packed, so deal with as one trace unit + // This may not be the nicest for user presentation, but is a much faster way to trace + addTraceDecl(VNumRange()); + } else { + addIgnore("Unsupported: Unpacked struct/union"); + // Once we have an UnpackedMemberSel which works, this code is straight forward: + //for (AstMemberDType* itemp = adtypep->membersp(); itemp; itemp=itemp->nextp()->castMemberDType()) { + // AstNodeDType* subtypep = itemp->subDTypep()->skipRefp(); + // ... + // m_traShowname += string(" ")+itemp->name(); + // m_traValuep = new AstMemberSel(nodep->fileline(), m_traValuep->cloneTree(true), ... + // and iterate + } + } + } + virtual void visit(AstBasicDType* nodep, AstNUser*) { + if (m_traVscp) { + addTraceDecl(VNumRange()); + } + } + virtual void visit(AstNodeDType* nodep, AstNUser*) { + // Note more specific dtypes above + if (!m_traVscp) return; + addIgnore("Unsupported: data type"); + } + //-------------------- virtual void visit(AstNode* nodep, AstNUser*) { nodep->iterateChildren(*this); @@ -178,6 +270,8 @@ public: m_fullFuncp = NULL; m_chgFuncp = NULL; m_funcNum = 0; + m_traVscp = NULL; + m_traValuep = NULL; nodep->accept(*this); } virtual ~TraceDeclVisitor() { diff --git a/test_regress/t/t_interface1_modport_trace.pl b/test_regress/t/t_interface1_modport_trace.pl new file mode 100755 index 000000000..103690b6b --- /dev/null +++ b/test_regress/t/t_interface1_modport_trace.pl @@ -0,0 +1,21 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003-2009 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. + +top_filename("t/t_interface1_modport.v"); + +compile ( + verilator_flags2 => ['--trace'], + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_struct_init_trace.pl b/test_regress/t/t_struct_init_trace.pl new file mode 100755 index 000000000..d3c5875b3 --- /dev/null +++ b/test_regress/t/t_struct_init_trace.pl @@ -0,0 +1,21 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003-2009 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. + +top_filename("t/t_struct_init.v"); + +compile ( + verilator_flags2 => ['--cc --trace'], + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_trace_complex.pl b/test_regress/t/t_trace_complex.pl new file mode 100755 index 000000000..83d039acc --- /dev/null +++ b/test_regress/t/t_trace_complex.pl @@ -0,0 +1,29 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003-2009 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 ( + verilator_flags2 => ['--cc --trace'], + ); + +execute ( + check_finished=>1, + ); + +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_strp /); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_strp_strp /); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_arrp /); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_arrp_arrp /); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_arrp_strp /); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_arru\(/); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_arru_arru\(/); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_arru_arrp\(/); +file_grep ("$Self->{obj_dir}/simx.vcd", qr/ v_arru_strp\(/); + +ok(1); +1; diff --git a/test_regress/t/t_trace_complex.v b/test_regress/t/t_trace_complex.v new file mode 100644 index 000000000..490fc87be --- /dev/null +++ b/test_regress/t/t_trace_complex.v @@ -0,0 +1,60 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2009 by Wilson Snyder. + +module t (clk); + input clk; + integer cyc=0; + + typedef struct packed { + bit b1; + bit b0; + } strp_t; + + typedef struct packed { + strp_t x1; + strp_t x0; + } strp_strp_t; + + typedef bit [2:1] arrp_t; + typedef arrp_t [4:3] arrp_arrp_t; + + typedef strp_t [4:3] arrp_strp_t; + + typedef bit arru_t [2:1]; + typedef arru_t arru_arru_t [4:3]; + typedef arrp_t arru_arrp_t [4:3]; + typedef strp_t arru_strp_t [4:3]; + + strp_t v_strp; + strp_strp_t v_strp_strp; + arrp_t v_arrp; + arrp_arrp_t v_arrp_arrp; + arrp_strp_t v_arrp_strp; + arru_t v_arru; + arru_arru_t v_arru_arru; + arru_arrp_t v_arru_arrp; + arru_strp_t v_arru_strp; + + always @ (posedge clk) begin + cyc <= cyc + 1; + v_strp <= ~v_strp; + v_strp_strp <= ~v_strp_strp; + v_arrp_strp <= ~v_arrp_strp; + v_arrp <= ~v_arrp; + v_arrp_arrp <= ~v_arrp_arrp; + for (integer b=3; b<=4; b++) begin + v_arru[b] <= ~v_arru[b]; + v_arru_strp[b] <= ~v_arru_strp[b]; + v_arru_arrp[b] <= ~v_arru_arrp[b]; + for (integer a=3; a<=4; a++) begin + v_arru_arru[a][b] = ~v_arru_arru[a][b]; + end + end + if (cyc == 5) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule