From 2920715dc8a916b67033f75f146193231d9e74e7 Mon Sep 17 00:00:00 2001 From: Matthew Ballance Date: Tue, 23 Jun 2026 00:19:04 +0000 Subject: [PATCH] Ensure NONSTD warning is always emitted for hier-ref cross references ; Pull through sufficient data to ensure verilog.y doesn't change when we support bare hier-refs as implicit coverpoints Signed-off-by: Matthew Ballance --- src/V3AstNodeOther.h | 12 ++++- src/V3Covergroup.cpp | 11 +++++ src/verilog.y | 46 ++++++++++++------- .../t/t_covergroup_cross_opt_unsup.out | 11 +++-- test_regress/t/t_covergroup_cross_opt_unsup.v | 5 +- 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 19f4d07ee..b36ccc1fd 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1185,12 +1185,22 @@ public: }; class AstCoverpointRef final : public AstNode { // Reference to a coverpoint used in a cross - const string m_name; // coverpoint name + // @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) 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} { + this->exprp(exprp); + } ASTGEN_MEMBERS_AstCoverpointRef; void dump(std::ostream& str) const override; void dumpJson(std::ostream& str) const override; diff --git a/src/V3Covergroup.cpp b/src/V3Covergroup.cpp index 054f27c34..7dc09db24 100644 --- a/src/V3Covergroup.cpp +++ b/src/V3Covergroup.cpp @@ -1295,6 +1295,17 @@ class FunctionalCoverageVisitor final : public VNVisitor { while (itemp) { AstNode* const nextp = itemp->nextp(); 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)"); + return; + } // Find the referenced coverpoint via name map (O(log n) vs O(n) linear scan) const auto it = m_coverpointMap.find(refp->name()); AstCoverpoint* const foundCpp = (it != m_coverpointMap.end()) ? it->second : nullptr; diff --git a/src/verilog.y b/src/verilog.y index 5af60e591..233310821 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -66,6 +66,15 @@ 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) @@ -7339,23 +7348,26 @@ cross_itemList: // IEEE: part of list_of_cross_items ; cross_item: // ==IEEE: cross_item - id/*cover_point_identifier*/ - { $$ = new AstCoverpointRef{$1, *$1}; } - // // Verilator extension beyond strict IEEE (cross_item is a simple - // // identifier): some tools accept a hierarchical/dotted reference - // // here. A dotted reference can never name a coverpoint (coverpoint - // // identifiers are flat, local names), so it is necessarily a plain - // // data reference, i.e. an implicit coverpoint over a variable. - // // Verilator does not support implicit coverpoints, so V3Covergroup - // // drops the whole cross with a COVERIGN warning; accept it here so - // // the file parses instead of erroring on the '.'. - | id/*variable*/ crossItemHier - { $$ = new AstCoverpointRef{$1, *$1 + *$2}; } - ; - -crossItemHier: // Verilator extension: dotted suffix of a hierarchical cross_item - '.' idAny { $$ = PARSEP->newString("." + *$2); } - | crossItemHier '.' idAny { $$ = PARSEP->newString(*$1 + "." + *$3); } + // // IEEE: cover_point_identifier | variable_identifier - both are a + // // simple identifier. We parse idDotted (a plain hierarchical + // // reference a.b.c, with no bit/array selects) to also accept the + // // non-standard dotted form (e.g. 'cross a.b') that several + // // simulators support; the common simple-identifier case is detected + // // and handled exactly as before. + idDotted + { + if (AstParseRef* const refp = VN_CAST($1, ParseRef)) { + // Standard: simple cover_point_identifier / variable_identifier + $$ = new AstCoverpointRef{refp->fileline(), refp->name()}; + 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 + $1->v3warn(NONSTD, "Non-standard hierarchical reference as a coverage " + "cross item (an implicit coverpoint)"); + $$ = new AstCoverpointRef{$1->fileline(), CROSS_ITEM_NAME($1), $1}; + } + } ; cross_body: // ==IEEE: cross_body diff --git a/test_regress/t/t_covergroup_cross_opt_unsup.out b/test_regress/t/t_covergroup_cross_opt_unsup.out index 7297641fd..b324a62ff 100644 --- a/test_regress/t/t_covergroup_cross_opt_unsup.out +++ b/test_regress/t/t_covergroup_cross_opt_unsup.out @@ -1,3 +1,8 @@ +%Warning-NONSTD: t/t_covergroup_cross_opt_unsup.v:19:34: Non-standard hierarchical reference as a coverage cross item (an implicit coverpoint) + 19 | cross_hier: cross cp_a, s_cfg.m_p; + | ^ + ... For warning description see https://verilator.org/warn/NONSTD?v=latest + ... Use "/* verilator lint_off NONSTD */" and lint_on around source to disable this message. %Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:13:7: Ignoring unsupported coverage cross option: 'per_instance' 13 | option.per_instance = 1; | ^~~~~~ @@ -7,8 +12,8 @@ : ... note: In instance 't' 15 | cross_implicit: cross cp_a, var_x; | ^~~~~ -%Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:18:29: Unsupported: cross of 's_cfg.m_p' which is not a coverpoint (implicit coverpoint) +%Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:19:34: Unsupported: cross of hierarchical reference 's_cfg.m_p' (implicit coverpoint) : ... note: In instance 't' - 18 | cross_hier: cross cp_a, s_cfg.m_p; - | ^~~~~ + 19 | cross_hier: cross cp_a, s_cfg.m_p; + | ^ %Error: Exiting due to diff --git a/test_regress/t/t_covergroup_cross_opt_unsup.v b/test_regress/t/t_covergroup_cross_opt_unsup.v index 6cf9452cf..720fbad6c 100644 --- a/test_regress/t/t_covergroup_cross_opt_unsup.v +++ b/test_regress/t/t_covergroup_cross_opt_unsup.v @@ -13,8 +13,9 @@ module t; option.per_instance = 1; // unsupported for cross; triggers COVERIGN } cross_implicit: cross cp_a, var_x; - // Hierarchical/dotted cross item: can only be a data reference (implicit - // coverpoint), never a coverpoint; treated as unsupported (COVERIGN) + // Non-standard hierarchical/dotted cross item: can only be a data reference + // (implicit coverpoint), never a coverpoint. Accepted with a NONSTD warning; + // implicit coverpoints are unsupported so the cross is dropped (COVERIGN). cross_hier: cross cp_a, s_cfg.m_p; endgroup typedef struct packed {logic m_p; logic h_mode;} cfg_t;