diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index b36ccc1fd..666a3ec22 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1188,17 +1188,16 @@ class AstCoverpointRef final : public AstNode { // @astgen op1 := exprp : Optional[AstNodeExpr] // Non-standard: hierarchical/dotted // // reference (implicit coverpoint), e.g. // // 'cross a.b'; nullptr for a plain coverpoint - // // name. Resolved by the time V3Covergroup - // // runs (AstDot is eliminated at link). - const string m_name; // coverpoint name (or flattened dotted path, for diagnostics) + // // name. An AstDot at parse time, resolved + // // by the time V3Covergroup runs. + const string m_name; // coverpoint name; empty when exprp() carries a hierarchical reference public: AstCoverpointRef(FileLine* fl, const string& name) : ASTGEN_SUPER_CoverpointRef(fl) , m_name{name} {} - AstCoverpointRef(FileLine* fl, const string& name, AstNodeExpr* exprp) - : ASTGEN_SUPER_CoverpointRef(fl) - , m_name{name} { + AstCoverpointRef(FileLine* fl, AstNodeExpr* exprp) + : ASTGEN_SUPER_CoverpointRef(fl) { this->exprp(exprp); } ASTGEN_MEMBERS_AstCoverpointRef; diff --git a/src/V3Covergroup.cpp b/src/V3Covergroup.cpp index fc8440e06..b1890e001 100644 --- a/src/V3Covergroup.cpp +++ b/src/V3Covergroup.cpp @@ -98,7 +98,7 @@ class FunctionalCoverageVisitor final : public VNVisitor { for (AstCoverCross* crossp : m_coverCrosses) { for (AstNode* itemp = crossp->itemsp(); itemp; itemp = itemp->nextp()) { if (const AstCoverpointRef* const refp = VN_CAST(itemp, CoverpointRef)) - m_crossedCpNames.insert(refp->name()); + if (!refp->exprp()) m_crossedCpNames.insert(refp->name()); } } @@ -1297,12 +1297,14 @@ class FunctionalCoverageVisitor final : public VNVisitor { AstCoverpointRef* const refp = VN_AS(itemp, CoverpointRef); if (refp->exprp()) { // Non-standard hierarchical/dotted cross item (e.g. 'cross a.b'): an implicit - // coverpoint over the referenced expression. The grammar already warned NONSTD; - // implicit coverpoints are not yet implemented, so drop the whole cross. This is - // the place to generate the implicit coverpoint when support is added (the - // resolved expression is in refp->exprp()). - refp->v3warn(COVERIGN, "Unsupported: cross of hierarchical reference '" - << refp->prettyName() << "' (implicit coverpoint)"); + // coverpoint over the referenced expression (carried in refp->exprp()). The + // grammar already warned NONSTD; implicit coverpoints are not yet implemented, so + // generate no sampling code for this cross. When support is added the implicit + // coverpoint should be synthesized upstream (V3LinkParse) as a real AstCoverpoint + // so it flows through the normal coverpoint path - by here coverpoint lowering has + // already run. + refp->v3warn(COVERIGN, + "Unsupported: cross of hierarchical reference (implicit coverpoint)"); return; } // Find the referenced coverpoint via name map (O(log n) vs O(n) linear scan) diff --git a/src/verilog.y b/src/verilog.y index 233310821..db145ea8e 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -66,15 +66,6 @@ static void STRENGTH_LIST(AstNode* listp, AstStrengthSpec* specp) { assignp->strengthSpecp(specp->backp() ? specp->cloneTree(false) : specp); } } -// Flatten a parse-time idDotted reference tree (id, a.b, a.b.c) to a dotted name -// string. Used for diagnostics on non-standard hierarchical cross items. -static string CROSS_ITEM_NAME(const AstNode* nodep) { - if (const AstDot* const dotp = VN_CAST(nodep, Dot)) { - return CROSS_ITEM_NAME(dotp->lhsp()) + (dotp->colon() ? "::" : ".") - + CROSS_ITEM_NAME(dotp->rhsp()); - } - return nodep->name(); // AstParseRef (incl. "$root") -} //====================================================================== // Statics (for here only) @@ -7362,10 +7353,13 @@ cross_item: // ==IEEE: cross_item VL_DO_DANGLING(refp->deleteTree(), refp); } else { // Verilator extension beyond strict IEEE (cross_item is a simple - // identifier): some tools accept a hierarchical/dotted reference + // identifier): some tools accept a hierarchical/dotted reference. + // Carry the reference expression (still an AstDot here) out of the + // parser unchanged; later stages resolve and, eventually, implement + // it as an implicit coverpoint. $1->v3warn(NONSTD, "Non-standard hierarchical reference as a coverage " "cross item (an implicit coverpoint)"); - $$ = new AstCoverpointRef{$1->fileline(), CROSS_ITEM_NAME($1), $1}; + $$ = new AstCoverpointRef{$1->fileline(), $1}; } } ; diff --git a/test_regress/t/t_covergroup_cross.out b/test_regress/t/t_covergroup_cross.out index a7311da80..abb035e5e 100644 --- a/test_regress/t/t_covergroup_cross.out +++ b/test_regress/t/t_covergroup_cross.out @@ -109,10 +109,10 @@ cg_range.cp_addr.hi_range: 2 cg_range.cp_addr.lo_range: 2 cg_range.cp_cmd.read: 2 cg_range.cp_cmd.write: 2 -cg_unnamed_cross.__cross7.a0_x_read [cross]: 1 -cg_unnamed_cross.__cross7.a0_x_write [cross]: 0 -cg_unnamed_cross.__cross7.a1_x_read [cross]: 0 -cg_unnamed_cross.__cross7.a1_x_write [cross]: 1 +cg_unnamed_cross.__cross8.a0_x_read [cross]: 1 +cg_unnamed_cross.__cross8.a0_x_write [cross]: 0 +cg_unnamed_cross.__cross8.a1_x_read [cross]: 0 +cg_unnamed_cross.__cross8.a1_x_write [cross]: 1 cg_unnamed_cross.cp_a.a0: 1 cg_unnamed_cross.cp_a.a1: 1 cg_unnamed_cross.cp_c.read: 1 diff --git a/test_regress/t/t_covergroup_cross.v b/test_regress/t/t_covergroup_cross.v index e86f179a4..f0857bc01 100644 --- a/test_regress/t/t_covergroup_cross.v +++ b/test_regress/t/t_covergroup_cross.v @@ -17,6 +17,9 @@ module t; logic mode; logic parity; + typedef struct packed {logic m_p; logic h_mode;} cfg_t; + cfg_t s_cfg = '0; + // 2-way cross covergroup cg2; cp_addr: coverpoint addr {bins addr0 = {0}; bins addr1 = {1};} @@ -90,6 +93,12 @@ module t; addr_cmd_unsup: cross cp_addr, cp_cmd{ option.per_instance = 1; // unsupported for cross - expect COVERIGN warning } + // Non-standard hierarchical reference as a cross item (an implicit coverpoint): + // accepted with NONSTD, but implicit coverpoints are unsupported so the whole + // cross is dropped (COVERIGN, suppressed here) - it contributes no bins. + /* verilator lint_off NONSTD */ + cross_hier: cross cp_addr, s_cfg.m_p; + /* verilator lint_on NONSTD */ endgroup // Covergroup with an unnamed cross - the cross is reported under the default name "cross" diff --git a/test_regress/t/t_covergroup_cross_opt_unsup.out b/test_regress/t/t_covergroup_cross_opt_unsup.out index b324a62ff..e1e2c5621 100644 --- a/test_regress/t/t_covergroup_cross_opt_unsup.out +++ b/test_regress/t/t_covergroup_cross_opt_unsup.out @@ -12,7 +12,7 @@ : ... note: In instance 't' 15 | cross_implicit: cross cp_a, var_x; | ^~~~~ -%Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:19:34: Unsupported: cross of hierarchical reference 's_cfg.m_p' (implicit coverpoint) +%Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:19:34: Unsupported: cross of hierarchical reference (implicit coverpoint) : ... note: In instance 't' 19 | cross_hier: cross cp_a, s_cfg.m_p; | ^