Fix line coverage of public functions.

Line coverage now aggregates by hierarchy automatically.
Previously this would be done inside SystemPerl, which was slower.

Changes

@@ -9,8 +9,13 @@ indicates the contributor was also the author of the fix; Thanks!
 
 ***  Support posedge of bit-selected signals, bug45. [Rodney Sinclair]
 
+***  Line coverage now aggregates by hierarchy automatically.
+     Previously this would be done inside SystemPerl, which was slower.
+
 **** Minor performance improvements of Verilator compiler runtime.
 
+**** Fix line coverage of public functions.  [Soon Koh]
+
 **** Fix SystemC 2.2 deprecated warnings about sensitive() and sc_start().
 
 **** Fix arrayed variables under function not compiling, bug44. [Ralf Karge]

src/V3AstNodes.h

@@ -1006,16 +1006,20 @@ private:
     string	m_text;
     string	m_hier;
     int		m_column;
+    int		m_binNum;	// Set by V3EmitCSyms to tell final V3Emit what to increment
 public:
     AstCoverDecl(FileLine* fl, int column, const string& type, const string& comment)
 	: AstNodeStmt(fl) {
 	m_text = comment; m_typeText = type; m_column = column;
+	m_binNum = 0;
     }
     ASTNODE_NODE_FUNCS(CoverDecl, COVERDECL)
     virtual void dump(ostream& str);
     virtual int instrCount()	const { return 1+2*instrCountLd(); }
     virtual bool maybePointedTo() const { return true; }
     int		column() 	const { return m_column; }
+    void	binNum(int flag) { m_binNum = flag; }
+    int		binNum() 	const { return m_binNum; }
     const string& comment() const { return m_text; }			// text to insert in code
     const string& typeText() const { return m_typeText; }
     const string& hier() const { return m_hier; }

src/V3Depth.cpp

@@ -133,11 +133,6 @@ private:
 	    m_funcp->isStatic(false);
 	}
     }
-    virtual void visit(AstCoverInc* nodep, AstNUser*) {
-	// OPTIMIZE: For now this needs this->__Vcoverage, but could be globalized
-	needNonStaticFunc(nodep);
-	visitStmt(nodep);
-    }
     virtual void visit(AstUCFunc* nodep, AstNUser*) {
 	needNonStaticFunc(nodep);
 	nodep->iterateChildren(*this);

src/V3EmitC.cpp

@@ -34,36 +34,6 @@
 
 #define VL_VALUE_STRING_MAX_WIDTH 1024	// We use a static char array in VL_VALUE_STRING
 
-//######################################################################
-// Coverage ID tracking
-
-class EmitCoverIds {
-    // TYPES
-    typedef map<AstCoverDecl*,int> CoverIdMap;
-    // MEMBERS
-    CoverIdMap	m_coverIds;	// Remapping of coverage IDs to per-module value
-    int		m_coverIdsSize;	// Size of m_coverIds (to avoid O(n^2) insert)
-    // METHODS
-public:
-    void clear() {
-	m_coverIds.clear();
-	m_coverIdsSize = 0;
-    }
-    int size() const { return m_coverIdsSize; }
-    int remap(AstCoverDecl* declp) {
-	// Make cover ID for this point, unique on this module
-	CoverIdMap::iterator it = m_coverIds.find(declp);
-	if (it == m_coverIds.end()) {
-	    m_coverIds.insert(make_pair(declp,m_coverIdsSize++));	// Remapping of coverage IDs to per-module value
-	    return m_coverIdsSize-1;
-	} else {
-	    return it->second;
-	}
-    }
-    EmitCoverIds() : m_coverIdsSize(0) {}
-    ~EmitCoverIds() {}
-};
-
 //######################################################################
 // Emit statements and math operators
 
@@ -74,8 +44,6 @@ private:
     vector<AstVar*>		m_ctorVarsVec;		// All variables in constructor order
     int		m_splitSize;	// # of cfunc nodes placed into output file
     int		m_splitFilenum;	// File number being created, 0 = primary
-public:
-    EmitCoverIds m_coverIds;	// Coverage ID remapping
 public:
     //int debug() { return 9; }
 
@@ -222,8 +190,14 @@ public:
     }
     virtual void visit(AstCoverDecl* nodep, AstNUser*) {
 	puts("__vlCoverInsert(");	// As Declared in emitCoverageDecl
-	puts("&__Vcoverage[");
+	puts("&(vlSymsp->__Vcoverage[");
-	puts(cvtToStr(m_coverIds.remap(nodep))); puts("]");
+	puts(cvtToStr(nodep->binNum())); puts("])");
+	// If this isn't the first instantiation of this module under this
+	// design, don't really count the bucket, and rely on SystemPerl to
+	// aggregate counts.  This is because Verilator combines all
+	// hiearchies itself, and if SystemPerl also did it, you'd end up
+	// with (number-of-instant) times too many counts in this bin.
+	puts(", first");  // Enable, passed from __Vconfigure parameter
 	puts(", \"");	puts(nodep->fileline()->filename()); puts("\"");
 	puts(", ");	puts(cvtToStr(nodep->fileline()->lineno()));
 	puts(", ");	puts(cvtToStr(nodep->column()));
@@ -233,8 +207,9 @@ public:
 	puts(");\n");
     }
     virtual void visit(AstCoverInc* nodep, AstNUser*) {
-	puts("++this->__Vcoverage[");
+	puts("++(vlSymsp->__Vcoverage[");
-	puts(cvtToStr(m_coverIds.remap(nodep->declp()))); puts("];\n");
+	puts(cvtToStr(nodep->declp()->binNum()));
+	puts("]);\n");
     }
     virtual void visit(AstCReturn* nodep, AstNUser*) {
 	puts("return (");
@@ -1221,18 +1196,10 @@ void EmitCImp::emitVarResets(AstModule* modp) {
 }
 
 void EmitCImp::emitCoverageDecl(AstModule* modp) {
-    m_coverIds.clear();
+    if (v3Global.opt.coverage()) {
-    for (AstNode* nodep=modp->stmtsp(); nodep; nodep = nodep->nextp()) {
-	if (AstCoverDecl* declp = nodep->castCoverDecl()) {
-	    m_coverIds.remap(declp);
-	}
-    }
-    if (m_coverIds.size()) {
 	ofp()->putsPrivate(true);
 	puts("// Coverage\n");
-	puts("SpZeroed<uint32_t>\t__Vcoverage["); puts(cvtToStr(m_coverIds.size())); puts("];\n");
+	puts("void __vlCoverInsert(uint32_t* countp, bool enable, const char* filename, int lineno, int column,\n");
-	ofp()->putAlign(V3OutFile::AL_AUTO, sizeof(uint32_t)*m_coverIds.size());
-	puts("void __vlCoverInsert(SpZeroed<uint32_t>* countp, const char* filename, int lineno, int column,\n");
 	puts(  	"const char* hier, const char* type, const char* comment);\n");
     }
 }
@@ -1258,8 +1225,9 @@ void EmitCImp::emitCtorImp(AstModule* modp) {
 }
 
 void EmitCImp::emitConfigureImp(AstModule* modp) {
-    puts("\nvoid "+modClassName(modp)+"::__Vconfigure("+symClassName()+"* symsp) {\n");
+    puts("\nvoid "+modClassName(modp)+"::__Vconfigure("+symClassName()+"* vlSymsp, bool first) {\n");
-    puts(   "__VlSymsp = symsp;\n");  // First, as later stuff needs it.
+    puts(   "if (0 && first) {}  // Prevent unused\n");
+    puts(   "this->__VlSymsp = vlSymsp;\n");  // First, as later stuff needs it.
     bool first=true;
     for (AstNode* nodep=modp->stmtsp(); nodep; nodep = nodep->nextp()) {
 	if (nodep->castCoverDecl()) {
@@ -1274,12 +1242,17 @@ void EmitCImp::emitConfigureImp(AstModule* modp) {
 }
 
 void EmitCImp::emitCoverageImp(AstModule* modp) {
-    if (m_coverIds.size()) {
+    if (v3Global.opt.coverage() ) {
 	puts("\n// Coverage\n");
 	// Rather than putting out SP_COVER_INSERT calls directly, we do it via this function
 	// This gets around gcc slowness constructing all of the template arguments
-	puts("void "+modClassName(m_modp)+"::__vlCoverInsert(SpZeroed<uint32_t>* countp, const char* filename, int lineno, int column,\n");
+	// SystemPerl 1.301 is much faster, but it's nice to remain back
+	// compatible, and have a common wrapper.
+	puts("void "+modClassName(m_modp)+"::__vlCoverInsert(uint32_t* countp, bool enable, const char* filename, int lineno, int column,\n");
 	puts(  	"const char* hier, const char* type, const char* comment) {\n");
+	puts(   "static uint32_t fake_zero_count = 0;\n");
+	puts(   "if (!enable) countp = &fake_zero_count;\n");  // Used for second++ instantiation of identical bin
+	puts(   "*countp = 0;\n");
 	puts(   "SP_COVER_INSERT(countp,");
 	puts(	"  \"filename\",filename,");
 	puts(	"  \"lineno\",lineno,");
@@ -1602,7 +1575,7 @@ void EmitCImp::emitInt(AstModule* modp) {
     if (v3Global.opt.trace() && !optSystemPerl()) {
 	puts("void\ttrace (SpTraceVcdCFile* tfp, int levels, int options=0);\n");
     }
-    puts("void\t__Vconfigure("+symClassName()+"* symsp);\n");
+    puts("void\t__Vconfigure("+symClassName()+"* symsp, bool first);\n");
     if (optSystemPerl()) puts("/*AUTOMETHODS*/\n");
 
     emitTextSection(AstType::SCINT);

src/V3EmitCSyms.cpp

@@ -38,11 +38,17 @@
 // Symbol table emitting
 
 class EmitCSyms : EmitCBaseVisitor {
+    // NODE STATE
+    // Cleared on Netlist
+    //  AstModule::user1()	-> bool.  Set true __Vconfigure called
+    AstUser1InUse	m_inuser1;
+
     // STATE
     AstModule*	m_modp;		// Current module
     typedef pair<AstScope*,AstModule*> ScopeModPair;
     vector<ScopeModPair>  m_scopes;	// Every scope by module
     V3LanguageWords 	m_words;	// Reserved word detector
+    int		m_coverBins;		// Coverage bin number
 
     // METHODS
     void emitInt();
@@ -70,6 +76,7 @@ class EmitCSyms : EmitCBaseVisitor {
 
 	// Sort m_scopes by scope name
 	sort(m_scopes.begin(), m_scopes.end(), CmpName());
+
 	// Output
 	emitInt();
 	emitImp();
@@ -84,6 +91,10 @@ class EmitCSyms : EmitCBaseVisitor {
 	nameCheck(nodep);
 	m_scopes.push_back(make_pair(nodep, m_modp));
     }
+    virtual void visit(AstCoverDecl* nodep, AstNUser*) {
+	// Assign numbers to all bins, so we know how big of an array to use
+	nodep->binNum(m_coverBins++);
+    }
     // NOPs
     virtual void visit(AstNodeStmt*, AstNUser*) {}
     virtual void visit(AstConst*, AstNUser*) {}
@@ -97,6 +108,7 @@ class EmitCSyms : EmitCBaseVisitor {
 public:
     EmitCSyms(AstNetlist* nodep) {
 	m_modp = NULL;
+	m_coverBins = 0;
 	nodep->accept(*this);
     }
 };
@@ -146,6 +158,12 @@ void EmitCSyms::emitInt() {
 	}
     }
 
+    puts("\n// COVERAGE\n");
+    if (m_coverBins) {
+	ofp()->putAlign(V3OutFile::AL_AUTO, sizeof(uint32_t));
+	puts("uint32_t\t__Vcoverage["); puts(cvtToStr(m_coverBins)); puts("];\n");
+    }
+
     puts("\n// CREATORS\n");
     puts(symClassName()+"("+topClassName()+"* topp, const char* namep);\n");
     puts((string)"~"+symClassName()+"() {};\n");
@@ -216,11 +234,16 @@ void EmitCSyms::emitImp() {
 	}
     }
     puts("// Setup each module's pointer back to symbol table (for public functions)\n");
-    puts("TOPp->__Vconfigure(this);\n");
+    puts("TOPp->__Vconfigure(this, true);\n");
     for (vector<ScopeModPair>::iterator it = m_scopes.begin(); it != m_scopes.end(); ++it) {
 	AstScope* scopep = it->first;  AstModule* modp = it->second;
 	if (!modp->isTop()) {
-	    puts(scopep->nameDotless()+".__Vconfigure(this);\n");
+	    // first is used by AstCoverDecl's call to __vlCoverInsert
+	    bool first = !modp->user1();
+	    modp->user1(true);
+	    puts(scopep->nameDotless()+".__Vconfigure(this, "
+		 +(first?"true":"false")
+		 +");\n");
 	}
     }
 

test_regress/driver.pl

@@ -229,7 +229,6 @@ sub new {
 	v_flags => [split(/\s+/,(" -f input.vc --debug-check"
 				 .($opt_verbose ? " +define+TEST_VERBOSE=1":"")
 				 .($opt_benchmark ? " +define+TEST_BENCHMARK=$opt_benchmark":"")
-				 ." -Mdir $self->{obj_dir}"
 				 ))],
 	v_flags2 => [],  # Overridden in some sim files
 	v_other_filenames => [],	# After the filename so we can spec multiple files
@@ -244,7 +243,8 @@ sub new {
 	ncrun_flags => [split(/\s+/,"+licqueue -q +assert +sv -R")],
 	# Verilator
 	'v3' => 0,
-	verilator_flags => [split(/\s+/,"-cc")],
+	verilator_flags => ["-cc",
+			    "-Mdir $self->{obj_dir}"],
 	verilator_flags2 => [],
 	verilator_make_gcc => 1,
 	verilated_debug => $Opt_Verilated_Debug,
@@ -338,10 +338,14 @@ sub compile {
     return 1 if $self->errors;
     $self->oprint("Compile\n");
 
-    $self->{sc} = 1 if (join(' ',@{$param{v_flags}},@{$param{v_flags2}}) =~ /-sc\b/);
+    my $checkflags = join(' ',@{$param{v_flags}},
-    $self->{sp} = 1 if (join(' ',@{$param{v_flags}},@{$param{v_flags2}}) =~ /-sp\b/);
+			  @{$param{v_flags2}},
-    $self->{trace} = 1 if (join(' ',@{$param{v_flags}},@{$param{v_flags2}}) =~ /-trace\b/);
+			  @{$param{verilator_flags}},
-    $self->{coverage} = 1 if (join(' ',@{$param{v_flags}},@{$param{v_flags2}}) =~ /-coverage\b/);
+			  @{$param{verilator_flags2}});
+    $self->{sc} = 1 if ($checkflags =~ /-sc\b/);
+    $self->{sp} = 1 if ($checkflags =~ /-sp\b/);
+    $self->{trace} = 1 if ($checkflags =~ /-trace\b/);
+    $self->{coverage} = 1 if ($checkflags =~ /-coverage\b/);
 
     if ($param{vcs}) {
 	$self->_make_top();
@@ -466,6 +470,44 @@ sub execute {
     }
 }
 
+sub inline_checks {
+    my $self = (ref $_[0]? shift : $Self);
+    return 1 if $self->errors;
+    my %param = (%{$self}, @_);	   # Default arguments are from $self
+
+    my $covfn = $Self->{coverage_filename};
+    my $contents = $self->file_contents($covfn);
+
+    $self->oprint("Extract checks\n");
+    my $fh = IO::File->new("<$self->{top_filename}");
+    while (defined(my $line = $fh->getline)) {
+	if ($line =~ /CHECK/) {
+	    if ($line =~ /CHECK_COVER *\( *([---0-9]+) *, *"([^"]+)" *, *(\d+) *\)/) {
+		my $lineno=($. + $1); my $hier=$2; my $count=$3;
+		my $regexp = "\001l\002".$lineno;
+		$regexp .= ".*\001h\002".quotemeta($hier);
+		$regexp .= ".*' ".$count;
+		if ($contents !~ /$regexp/) {
+		    $self->error("CHECK_COVER: $covfn: Regexp not found: $regexp\n".
+				 "From $self->{top_filename}:$.: $line");
+		}
+	    }
+	    elsif ($line =~ /CHECK_COVER_MISSING *\( *([---0-9]+) *\)/) {
+		my $lineno=($. + $1);
+		my $regexp = "\001l\002".$lineno;
+		if ($contents =~ /$regexp/) {
+		    $self->error("CHECK_COVER_MISSING: $covfn: Regexp found: $regexp\n".
+				 "From $self->{top_filename}:$.: $line");
+		}
+	    }
+	    else {
+		$self->error("$self->{top_filename}:$.: Unknown CHECK request: $line");
+	    }
+	}
+    }
+    $fh->close;
+}
+
 #----------------------------------------------------------------------
 # Accessors
 

test_regress/t/t_cover_line.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-2008 by Wilson Snyder. This program is free software; you can
+# redistribute it and/or modify it under the terms of either the GNU
+# General Public License or the Perl Artistic License.
+
+compile (
+	 verilator_flags2 => [$Self->{v3}?'--sp --coverage-line':''],
+	 );
+
+execute (
+	 check_finished=>1,
+	 );
+
+# Read the input .v file and do any CHECK_COVER requests
+inline_checks();
+
+ok(1);
+1;

test_regress/t/t_cover_line.v

@@ -0,0 +1,136 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2008 by Wilson Snyder.
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+
+   reg 	 toggle; initial toggle=0;
+
+   integer cyc; initial cyc=1;
+   wire [7:0] cyc_copy = cyc[7:0];
+
+   alpha a1 (/*AUTOINST*/
+	     // Inputs
+	     .clk			(clk),
+	     .toggle			(toggle));
+   alpha a2 (/*AUTOINST*/
+	     // Inputs
+	     .clk			(clk),
+	     .toggle			(toggle));
+   beta  b1 (/*AUTOINST*/
+	     // Inputs
+	     .clk			(clk),
+	     .toggle			(toggle));
+   beta  b2 (/*AUTOINST*/
+	     // Inputs
+	     .clk			(clk),
+	     .toggle			(toggle));
+   tsk   t1 (/*AUTOINST*/
+	     // Inputs
+	     .clk			(clk),
+	     .toggle			(toggle));
+
+   always @ (posedge clk) begin
+      if (cyc!=0) begin
+	 cyc <= cyc + 1;
+	 toggle <= '0;
+	 if (cyc==3) begin
+	    toggle <= '1;
+	 end
+	 else if (cyc==5) begin
+	    $c("call_task();");
+	 end
+	 else if (cyc==10) begin
+	    $write("*-* All Finished *-*\n");
+	    $finish;
+	 end
+      end
+   end
+
+   task call_task;
+      /* verilator public */
+      t1.center_task(1'b1);
+   endtask
+
+endmodule
+
+module alpha (/*AUTOARG*/
+   // Inputs
+   clk, toggle
+   );
+   input clk;
+   input toggle;
+   always @ (posedge clk) begin
+      if (toggle) begin
+	 // CHECK_COVER(-1,"TOP.v.a*",2)
+	 // t.a1 and t.a2 collapse to a count of 2
+      end
+      if (toggle) begin
+	 // CHECK_COVER_MISSING(-1)
+	 // This doesn't even get added
+	 // verilator coverage_block_off
+	 $write("");
+      end
+   end
+endmodule
+
+module beta (/*AUTOARG*/
+   // Inputs
+   clk, toggle
+   );
+   input clk;
+   input toggle;
+
+   /* verilator public_module */
+
+   always @ (posedge clk) begin
+      if (0) begin
+	 // CHECK_COVER(-1,"TOP.v.b*",0)
+	 // Make sure that we don't optimize away zero buckets
+      end
+      if (toggle) begin
+	 // CHECK_COVER(-1,"TOP.v.b*",2)
+	 // t.b1 and t.b2 collapse to a count of 2
+      end
+      if (toggle) begin
+	 // CHECK_COVER_MISSING(-1)
+	 // This doesn't
+	 // verilator coverage_block_off
+	 $write("");
+      end
+   end
+endmodule
+
+module tsk (/*AUTOARG*/
+   // Inputs
+   clk, toggle
+   );
+   input clk;
+   input toggle;
+
+   /* verilator public_module */
+
+   always @ (posedge clk) begin
+      center_task(1'b0);
+   end
+
+   task center_task;
+      input external;
+      begin
+	 if (toggle) begin
+	    // CHECK_COVER(-1,"TOP.v.t1",1)
+	 end
+	 if (external) begin
+	    // CHECK_COVER(-1,"TOP.v.t1",1)
+	    $write("[%0t] Got external pulse\n", $time);
+	 end
+      end
+   endtask
+
+endmodule