Create /*verilator coverage_on/off*/ instead of coverage_module_off.

This allows finer grained bracketing of sections of interest.
Convert tracing_on/off to use the same general scheme.

Changes

@@ -5,7 +5,7 @@ indicates the contributor was also the author of the fix; Thanks!
 
 * Verilator 3.700***
 
-**   Add /*verilator coverage_module_off*/.
+**   Add /*verilator coverage_on/_off */.
 
 ***  Optimize two-level shift and and/or trees, +23% on one test.
 

bin/verilator

@@ -312,7 +312,7 @@ Verilator automatically disables coverage of branches that have a $stop in
 them, as it is assumed $stop branches contain an error check that should
 not occur.  A /*verilator coverage_block_off*/ comment will perform a
 similar function on any code in that block or below, or /*verilator
-coverage_module_off*/ will disable coverage on the entire module.
+coverage_on/coverage_off*/ will disable coverage around lines of code.
 
 Note Verilator may over-count combinatorial (non-clocked) blocks when those
 blocks receive signals which have had the UNOPTFLAT warning disabled; for
@@ -1225,10 +1225,16 @@ ignored in the scheduling algorithm, improving performance.
 Specifies the entire begin/end block should be ignored for coverage
 analysis purposes.
 
-=item /*verilator coverage_module_off*/
+=item /*verilator coverage_off*/
 
-Specifies the entire module should be ignored for coverage analysis
+Specifies that following lines of code should have coverage disabled.
-purposes.
+Often used to ignore an entire module for coverage analysis purposes.
+
+=item /*verilator coverage_on*/
+
+Specifies that following lines of code should have coverage reenabled (if
+appropriate --coverage flags are passed) after being disabled earlier with
+/*verilator coverage_off*/.
 
 =item /*verilator inline_module*/
 

src/V3Ast.h

@@ -56,7 +56,6 @@ class AstPragmaType {
 public:
     enum en {
 	COVERAGE_BLOCK_OFF,
-	COVERAGE_MODULE_OFF,
 	INLINE_MODULE,
 	NO_INLINE_MODULE,
 	NO_INLINE_TASK,

src/V3AstNodes.h

@@ -574,7 +574,6 @@ struct AstModule : public AstNode {
 private:
     string	m_name;		// Name of the module
     string	m_origName;	// Name of the module, ignoring name() changes, for dot lookup
-    bool	m_modCover:1;	// Coverage of this module
     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
@@ -584,7 +583,7 @@ private:
 public:
     AstModule(FileLine* fl, const string& name)
 	: AstNode (fl)
-	,m_name(name), m_origName(name), m_modCover(true)
+	,m_name(name), m_origName(name)
 	,m_modPublic(false), m_modTrace(false), m_inLibrary(false)
 	,m_level(0), m_varNum(0), m_clkReqVarp(NULL) { }
     ASTNODE_NODE_FUNCS(Module, MODULE)
@@ -609,8 +608,6 @@ public:
     int  varNumGetInc() 	{ return ++m_varNum; }
     AstVar* clkReqVarp() const	{ return m_clkReqVarp; }
     void clkReqVarp(AstVar* varp) { m_clkReqVarp = varp; }
-    void modCover(bool flag) 	{ m_modCover = flag; }
-    bool modCover() const 	{ return m_modCover; }
     void modPublic(bool flag) 	{ m_modPublic = flag; }
     bool modPublic() const 	{ return m_modPublic; }
     void modTrace(bool flag) 	{ m_modTrace = flag; }

src/V3Coverage.cpp

@@ -76,10 +76,8 @@ private:
     // VISITORS - BOTH
     virtual void visit(AstModule* nodep, AstNUser*) {
 	m_modp = nodep;
-	if (nodep->modCover()) {   // Cleared by /*verilator coverage_module_off*/
+	m_fileps.clear();
-	    m_fileps.clear();
+	nodep->iterateChildren(*this);
-	    nodep->iterateChildren(*this);
-	}
 	m_modp = NULL;
     }
 
@@ -88,7 +86,8 @@ private:
 	UINFO(4," IF: "<<nodep<<endl);
 	if (m_checkBlock) {
 	    nodep->ifsp()->iterateAndNext(*this);
-	    if (m_checkBlock && v3Global.opt.coverageLine()) {	// if a "if" branch didn't disable it
+	    if (m_checkBlock
+		&& nodep->fileline()->coverageOn() && v3Global.opt.coverageLine()) {	// if a "if" branch didn't disable it
 		if (!nodep->backp()->castIf()
 		    || nodep->backp()->castIf()->elsesp()!=nodep) {  // Ignore if else; did earlier
 		    UINFO(4,"   COVER: "<<nodep<<endl);
@@ -99,7 +98,8 @@ private:
 	    if (nodep->elsesp()) {
 		m_checkBlock = true;
 		nodep->elsesp()->iterateAndNext(*this);
-		if (m_checkBlock && v3Global.opt.coverageLine()) {	// if a "else" branch didn't disable it
+		if (m_checkBlock
+		    && nodep->fileline()->coverageOn() && v3Global.opt.coverageLine()) {	// if a "else" branch didn't disable it
 		    UINFO(4,"   COVER: "<<nodep<<endl);
 		    if (nodep->elsesp()->castIf()) {
 			nodep->addElsesp(newCoverInc(nodep->elsesp()->fileline(), "", "block", "elsif"));
@@ -113,7 +113,8 @@ private:
     }
     virtual void visit(AstCaseItem* nodep, AstNUser*) {
 	UINFO(4," CASEI: "<<nodep<<endl);
-	if (m_checkBlock && v3Global.opt.coverageLine()) {
+	if (m_checkBlock
+	    && nodep->fileline()->coverageOn() && v3Global.opt.coverageLine()) {
 	    nodep->bodysp()->iterateAndNext(*this);
 	    if (m_checkBlock) {	// if the case body didn't disable it
 		UINFO(4,"   COVER: "<<nodep<<endl);

src/V3Error.h

@@ -39,6 +39,9 @@ public:
 	INFO,		// General information out
 	FATAL,		// Kill the program
 	ERROR,		// General error out, can't suppress
+	// Boolean information we track per-line, but aren't errors
+	I_COVERAGE,	// Coverage is on/off from /*verilator coverage_on/off*/
+	I_TRACING,	// Tracing is on/off from /*verilator tracing_on/off*/
 	// Error codes:
 	MULTITOP,	// Error: Multiple top level modules
 	TASKNSVAR,	// Error: Task I/O not simple
@@ -80,7 +83,11 @@ public:
 	const char* names[] = {
 	    // Leading spaces indicate it can't be disabled.
 	    " MIN", " SUPPRESS", " INFO", " FATAL", " ERROR",
+	    // Boolean
+	    " I_COVERAGE", " I_TRACING",
+	    // Errors
 	    "MULTITOP", "TASKNSVAR",
+	    // Warnings
 	    " FIRST_WARN",
 	    "BLKANDNBLK",
 	    "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CMPCONST",
@@ -241,6 +248,13 @@ public:
     void warnStateInherit(const FileLine& from);
     void warnResetDefault() { warnStateFrom(s_defaultFileLine); }
 
+    // Boolean ACCESSORS/METHODS
+    bool coverageOn() const { return m_warnOn.test(V3ErrorCode::I_COVERAGE); }
+    void coverageOn(bool flag) { m_warnOn.set(V3ErrorCode::I_COVERAGE,flag); }
+    bool tracingOn() const { return m_warnOn.test(V3ErrorCode::I_TRACING); }
+    void tracingOn(bool flag) { m_warnOn.set(V3ErrorCode::I_TRACING,flag); }
+
+    // METHODS
     void	v3errorEnd(ostringstream& str);
     inline bool operator==(FileLine rhs) { return (m_lineno==rhs.m_lineno && m_filename==rhs.m_filename); }
 

src/V3LinkResolve.cpp

@@ -334,11 +334,6 @@ private:
 	    m_modp->modPublic(true);  // Need to get to the task...
 	    nodep->unlinkFrBack(); pushDeletep(nodep); nodep=NULL;
 	}
-	else if (nodep->pragType() == AstPragmaType::COVERAGE_MODULE_OFF) {
-	    if (!m_modp) nodep->v3fatalSrc("COVERAGE_MODULE_OFF not under a module\n");
-	    m_modp->modCover(false);
-	    nodep->unlinkFrBack(); pushDeletep(nodep); nodep=NULL;
-	}
 	else if (nodep->pragType() == AstPragmaType::COVERAGE_BLOCK_OFF) {
 	    if (!v3Global.opt.coverageLine()) {  // No need for block statements; may optimize better without
 		nodep->unlinkFrBack(); pushDeletep(nodep); nodep=NULL;

src/verilog.l

@@ -555,7 +555,6 @@ escid	\\[^ \t\f\r\n]+
   "/*verilator"{ws}*"*/"		{}	/* Ignore empty comments, may be `endif // verilator */
   "/*verilator clock_enable*/"		{yylval.fileline = CRELINE(); return yVL_CLOCK_ENABLE;}
   "/*verilator coverage_block_off*/"	{yylval.fileline = CRELINE(); return yVL_COVERAGE_BLOCK_OFF;}
-  "/*verilator coverage_module_off*/"	{yylval.fileline = CRELINE(); return yVL_COVERAGE_MODULE_OFF;}
   "/*verilator full_case*/"		{yylval.fileline = CRELINE(); return yVL_FULL_CASE;}
   "/*verilator inline_module*/"		{yylval.fileline = CRELINE(); return yVL_INLINE_MODULE;}
   "/*verilator isolate_assignments*/"	{yylval.fileline = CRELINE(); return yVL_ISOLATE_ASSIGNMENTS;}
@@ -567,8 +566,10 @@ escid	\\[^ \t\f\r\n]+
   "/*verilator public_module*/"		{yylval.fileline = CRELINE(); return yVL_PUBLIC_MODULE;}
   "/*verilator sc_clock*/"		{yylval.fileline = CRELINE(); return yVL_CLOCK;}
   "/*verilator systemc_clock*/"		{yylval.fileline = CRELINE(); return yVL_CLOCK;}
-  "/*verilator tracing_off*/"		{yylval.fileline = CRELINE(); return yVL_TRACING_OFF;}
+  "/*verilator tracing_off*/"		{V3Read::fileline()->tracingOn(false);}
-  "/*verilator tracing_on*/"		{yylval.fileline = CRELINE(); return yVL_TRACING_ON;}
+  "/*verilator tracing_on*/"		{V3Read::fileline()->tracingOn(true);}
+  "/*verilator coverage_off*/"		{V3Read::fileline()->coverageOn(false);}
+  "/*verilator coverage_on*/"		{V3Read::fileline()->coverageOn(true);}
   "/*verilator lint_off"[^*]*"*/"	{V3Read::verilatorCmtLint(yytext, true); }
   "/*verilator lint_on"[^*]*"*/"	{V3Read::verilatorCmtLint(yytext, false); }
   "/*verilator lint_restore*/"		{V3Read::verilatorCmtLintRestore(); }

src/verilog.y

@@ -56,7 +56,6 @@ public:
     static bool		s_pinStar;	// Encountered SystemVerilog .*
     static string	s_instModule;	// Name of module referenced for instantiations
     static AstPin*	s_instParamp;	// Parameters for instantiations
-    static bool		s_trace;	// Tracing is turned on
     static int		s_uniqueAttr;	// Bitmask of unique/priority keywords
 
     static AstVar*  createVariable(FileLine* fileline, string name, AstRange* arrayp);
@@ -75,7 +74,6 @@ public:
 };
 
 bool		V3Parse::s_impliedDecl = false;
-bool		V3Parse::s_trace = false;   // Set on first module creation
 AstVarType	V3Parse::s_varDecl = AstVarType::UNKNOWN;
 AstVarType	V3Parse::s_varIO = AstVarType::UNKNOWN;
 bool		V3Parse::s_varSigned = false;
@@ -269,7 +267,6 @@ class AstSenTree;
 %token<fileline>	yVL_CLOCK		"/*verilator sc_clock*/"
 %token<fileline>	yVL_CLOCK_ENABLE	"/*verilator clock_enable*/"
 %token<fileline>	yVL_COVERAGE_BLOCK_OFF	"/*verilator coverage_block_off*/"
-%token<fileline>	yVL_COVERAGE_MODULE_OFF	"/*verilator coverage_module_off*/"
 %token<fileline>	yVL_FULL_CASE		"/*verilator full_case*/"
 %token<fileline>	yVL_INLINE_MODULE	"/*verilator inline_module*/"
 %token<fileline>	yVL_ISOLATE_ASSIGNMENTS	"/*verilator isolate_assignments*/"
@@ -279,8 +276,6 @@ class AstSenTree;
 %token<fileline>	yVL_PUBLIC		"/*verilator public*/"
 %token<fileline>	yVL_PUBLIC_FLAT		"/*verilator public_flat*/"
 %token<fileline>	yVL_PUBLIC_MODULE	"/*verilator public_module*/"
-%token<fileline>	yVL_TRACING_OFF		"/*verilator tracing_off*/"
-%token<fileline>	yVL_TRACING_ON		"/*verilator tracing_on*/"
 
 %token<fileline>	yP_OROR		"||"
 %token<fileline>	yP_ANDAND	"&&"
@@ -463,13 +458,12 @@ moduleDecl:	modHeader  timeunitsDeclE modItemListE yENDMODULE endLabelE
 
 modHeader<modulep>:
 		modHdr modParE modPortsE ';'
-			{ $1->modTrace(V3Parse::s_trace);  // Stash for implicit wires, etc
+			{ $1->modTrace(v3Global.opt.trace() && $1->fileline()->tracingOn());  // Stash for implicit wires, etc
 			  if ($2) $1->addStmtp($2); if ($3) $1->addStmtp($3); }
 	;
 
 modHdr<modulep>:
-		yMODULE { V3Parse::s_trace=v3Global.opt.trace();}
+		yMODULE yaID				{ $$ = new AstModule($1,*$2); $$->inLibrary(V3Read::inLibrary()||V3Read::inCellDefine());
-			yaID				{ $$ = new AstModule($1,*$3); $$->inLibrary(V3Read::inLibrary()||V3Read::inCellDefine());
 							  $$->modTrace(v3Global.opt.trace());
 							  V3Read::rootp()->addModulep($$); }
 	;
@@ -630,12 +624,9 @@ modItem<nodep>:
 	|	yaSCIMPH				{ $$ = new AstScImpHdr(CRELINE(),*$1); }
 	|	yaSCCTOR				{ $$ = new AstScCtor(CRELINE(),*$1); }
 	|	yaSCDTOR				{ $$ = new AstScDtor(CRELINE(),*$1); }
-	|	yVL_COVERAGE_MODULE_OFF			{ $$ = new AstPragma($1,AstPragmaType::COVERAGE_MODULE_OFF); }
 	|	yVL_INLINE_MODULE			{ $$ = new AstPragma($1,AstPragmaType::INLINE_MODULE); }
 	|	yVL_NO_INLINE_MODULE			{ $$ = new AstPragma($1,AstPragmaType::NO_INLINE_MODULE); }
 	|	yVL_PUBLIC_MODULE			{ $$ = new AstPragma($1,AstPragmaType::PUBLIC_MODULE); }
-	|	yVL_TRACING_OFF				{ $$ = NULL; V3Parse::s_trace=false; }
-	|	yVL_TRACING_ON				{ $$ = NULL; V3Parse::s_trace=v3Global.opt.trace(); }
 	|	ySPECIFY specifyJunkList yENDSPECIFY	{ $$ = NULL; }
 	|	ySPECIFY yENDSPECIFY			{ $$ = NULL; }
 	;
@@ -1607,7 +1598,7 @@ AstVar* V3Parse::createVariable(FileLine* fileline, string name, AstRange* array
     nodep->width(0,0);
     // Propagate from current module tracing state
     if (nodep->isGenVar() || nodep->isParam()) nodep->trace(false);
-    else nodep->trace(V3Parse::s_trace);
+    else nodep->trace(v3Global.opt.trace() && nodep->fileline()->tracingOn());
 
     // Remember the last variable created, so we can attach attributes to it in later parsing
     V3Parse::s_varAttrp = nodep;

test_regress/t/t_cover_line.v

@@ -146,13 +146,19 @@ module off (/*AUTOARG*/
    input clk;
    input toggle;
 
-   // verilator coverage_module_off
+   // verilator coverage_off
-
    always @ (posedge clk) begin
       if (toggle) begin
 	 // CHECK_COVER_MISSING(-1)
 	 // because under coverage_module_off
       end
    end
+   // verilator coverage_on
+   always @ (posedge clk) begin
+      if (toggle) begin
+	 // CHECK_COVER(-1,"TOP.v.o1",1)
+	 // because under coverage_module_off
+      end
+   end
 
 endmodule