From 3d788b4b936ff6d218c725e4110924cea6974ada Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 5 Nov 2008 10:23:03 -0500 Subject: [PATCH] Fix "Missing coverage in PSL" due to "cover property $stop" statements. "cover property" reported hierarchy now includes named begin blocks. --- src/V3Assert.cpp | 2 +- src/V3Begin.cpp | 5 +++++ src/V3Coverage.cpp | 36 ++++++++++++++++++++++++++------- test_regress/driver.pl | 4 +++- test_regress/t/t_assert_cover.v | 15 ++++++++++++++ 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/V3Assert.cpp b/src/V3Assert.cpp index 1adebd183..860afa4fa 100644 --- a/src/V3Assert.cpp +++ b/src/V3Assert.cpp @@ -115,7 +115,7 @@ private: } else { // V3Coverage assigned us a bucket to increment. AstCoverInc* covincp = snodep->coverincp()->castCoverInc(); - if (!covincp) snodep->v3fatalSrc("Missing coverage in PSL"); + if (!covincp) snodep->v3fatalSrc("Missing AstCoverInc under assertion"); covincp->unlinkFrBack(); if (message!="") covincp->declp()->comment(message); bodysp = covincp; diff --git a/src/V3Begin.cpp b/src/V3Begin.cpp index 85d339e3f..49052de43 100644 --- a/src/V3Begin.cpp +++ b/src/V3Begin.cpp @@ -142,6 +142,11 @@ private: } nodep->iterateChildren(*this); } + virtual void visit(AstCoverDecl* nodep, AstNUser*) { + // Don't need to fix path in coverage statements, they're not under + // any BEGINs, but V3Coverage adds them all under the module itself. + nodep->iterateChildren(*this); + } virtual void visit(AstNode* nodep, AstNUser*) { nodep->iterateChildren(*this); } diff --git a/src/V3Coverage.cpp b/src/V3Coverage.cpp index 6544157d3..9be39c75f 100644 --- a/src/V3Coverage.cpp +++ b/src/V3Coverage.cpp @@ -52,11 +52,13 @@ private: bool m_checkBlock; // Should this block get covered? AstModule* m_modp; // Current module to add statement to FileMap m_fileps; // Column counts for each fileline + string m_beginHier; // AstBegin hier name for user coverage points //int debug() { return 9; } // METHODS - AstCoverInc* newCoverInc(FileLine* fl, const string& type, const string& comment) { + AstCoverInc* newCoverInc(FileLine* fl, const string& hier, + const string& type, const string& comment) { int column = 0; FileMap::iterator it = m_fileps.find(fl); if (it == m_fileps.end()) { @@ -66,6 +68,7 @@ private: } AstCoverDecl* declp = new AstCoverDecl(fl, column, type, comment); + declp->hier(hier); m_modp->addStmtp(declp); return new AstCoverInc(fl, declp); @@ -86,7 +89,7 @@ private: if (!nodep->backp()->castIf() || nodep->backp()->castIf()->elsesp()!=nodep) { // Ignore if else; did earlier UINFO(4," COVER: "<addIfsp(newCoverInc(nodep->fileline(), "block", "if")); + nodep->addIfsp(newCoverInc(nodep->fileline(), "", "block", "if")); } } // Don't do empty else's, only empty if/case's @@ -96,9 +99,9 @@ private: if (m_checkBlock && v3Global.opt.coverageLine()) { // if a "else" branch didn't disable it UINFO(4," COVER: "<elsesp()->castIf()) { - nodep->addElsesp(newCoverInc(nodep->elsesp()->fileline(), "block", "elsif")); + nodep->addElsesp(newCoverInc(nodep->elsesp()->fileline(), "", "block", "elsif")); } else { - nodep->addElsesp(newCoverInc(nodep->elsesp()->fileline(), "block", "else")); + nodep->addElsesp(newCoverInc(nodep->elsesp()->fileline(), "", "block", "else")); } } } @@ -111,18 +114,20 @@ private: nodep->bodysp()->iterateAndNext(*this); if (m_checkBlock) { // if the case body didn't disable it UINFO(4," COVER: "<addBodysp(newCoverInc(nodep->fileline(), "block", "case")); + nodep->addBodysp(newCoverInc(nodep->fileline(), "", "block", "case")); } m_checkBlock = true; // Reset as a child may have cleared it } } virtual void visit(AstPslCover* nodep, AstNUser*) { UINFO(4," PSLCOVER: "<iterateChildren(*this); if (!nodep->coverincp()) { // Note the name may be overridden by V3Assert processing - nodep->coverincp(newCoverInc(nodep->fileline(), "psl_cover", "cover")); + nodep->coverincp(newCoverInc(nodep->fileline(), m_beginHier, "psl_cover", "cover")); } + m_checkBlock = true; // Reset as a child may have cleared it } virtual void visit(AstStop* nodep, AstNUser*) { UINFO(4," STOP: "<unlinkFrBack()->deleteTree(); nodep=NULL; + } else { + if (m_checkBlock) nodep->iterateChildren(*this); } - if (m_checkBlock) nodep->iterateChildren(*this); + } + virtual void visit(AstBegin* nodep, AstNUser*) { + // Record the hiearchy of any named begins, so we can apply to user + // coverage points. This is because there may be cov points inside + // generate blocks; each point should get separate consideration. + // (Currently ignored for line coverage, since any generate iteration + // covers the code in that line.) + string oldHier = m_beginHier; + { + if (nodep->name()!="") { + m_beginHier = m_beginHier + (m_beginHier!=""?".":"") + nodep->name(); + } + nodep->iterateChildren(*this); + } + m_beginHier = oldHier; } virtual void visit(AstNode* nodep, AstNUser*) { // Default: Just iterate @@ -150,6 +171,7 @@ public: CoverageVisitor(AstNetlist* rootp) { // Operate on all modules m_checkBlock = true; + m_beginHier = ""; rootp->iterateChildren(*this); } virtual ~CoverageVisitor() {} diff --git a/test_regress/driver.pl b/test_regress/driver.pl index f964d2ebf..2a5bded6a 100755 --- a/test_regress/driver.pl +++ b/test_regress/driver.pl @@ -481,7 +481,9 @@ sub _run { my %param = (tee=>1, @_); my $command = join(' ',@{$param{cmd}}); - print "\t$command\n"; + print "\t$command"; + print " > $param{logfile}" if $param{logfile}; + print "\n"; if ($param{logfile}) { open(SAVEOUT, ">&STDOUT") or die "%Error: Can't dup stdout"; diff --git a/test_regress/t/t_assert_cover.v b/test_regress/t/t_assert_cover.v index 71d103434..541423e5b 100644 --- a/test_regress/t/t_assert_cover.v +++ b/test_regress/t/t_assert_cover.v @@ -64,7 +64,22 @@ module Test cover property (@(posedge clk) disable iff (!toggle) cyc==8) $stop; + //============================================================ + // Using a macro and generate + wire reset = (cyc < 2); + +`define covclk(eqn) cover property (@(posedge clk) disable iff (reset) (eqn)) + + genvar i; + generate + for (i=0; i<32; i=i+1) + begin: cycval + CycCover_i: `covclk( cyc[i] ); + end + endgenerate + `ifndef verilator // Unsupported + //============================================================ // Using a more complicated property property C1; @(posedge clk)