From 930976ab30cdb1f4b0c84aea6cc047ae81cf0fc0 Mon Sep 17 00:00:00 2001 From: Matthew Ballance Date: Tue, 7 Jul 2026 13:55:18 +0000 Subject: [PATCH] Tighten up comments Signed-off-by: Matthew Ballance --- src/V3Covergroup.cpp | 36 +++++-------------- .../t/t_covergroup_embedded_unsup.out | 8 ++--- test_regress/t/t_covergroup_embedded_unsup.v | 14 ++------ 3 files changed, 15 insertions(+), 43 deletions(-) diff --git a/src/V3Covergroup.cpp b/src/V3Covergroup.cpp index 201564747..90bae71a2 100644 --- a/src/V3Covergroup.cpp +++ b/src/V3Covergroup.cpp @@ -1693,41 +1693,26 @@ class FunctionalCoverageVisitor final : public VNVisitor { // VISITORS AstNode* findUnsupportedCoverpointRef(AstClass* cgClassp) { - // An embedded covergroup is lowered into a sibling AstClass that has no handle to - // the enclosing object. Two coverpoint/iff/cross shapes cannot be lowered and would - // otherwise emit uncompilable C++ or crash V3Broken; detect them so the caller can - // skip lowering with a clean COVERIGN warning. Returns the first offending node - // (or nullptr), preferring the enclosing-member case for message clarity. - // Collect the covergroup class's own member variables (sample/constructor args); - // references to those are legitimate. + // An embedded covergroup is lowered into a sibling AstClass that (currently) has + // no handle to the enclosing object. Identify refs to the containing context + // or a formal param and flag as unsupported std::set ownVars; for (AstNode* itemp = cgClassp->membersp(); itemp; itemp = itemp->nextp()) { if (const AstVar* const varp = VN_CAST(itemp, Var)) ownVars.insert(varp); } - // Pass 1: a (non-static) member of the enclosing class reached with no handle. The - // member would be emitted as if it were static ("invalid use of non-static data - // member"). + // Flag non-static enclosing-class members reached without a handle as unsupported AstNode* offenderp = nullptr; const auto scanEnclosing = [&](AstNode* rootp) { rootp->foreach([&](AstVarRef* refp) { if (offenderp) return; - const AstVar* const varp = refp->varp(); // Always set post-LinkDot - // A member of another class (the enclosing class) reached with no handle. - // Members of the covergroup class itself (sample/constructor args) are - // legitimate and excluded via ownVars. + const AstVar* const varp = refp->varp(); if (varp->isClassMember() && !ownVars.count(varp)) offenderp = refp; }); }; for (AstCoverpoint* cpp : m_coverpoints) scanEnclosing(cpp); for (AstCoverCross* crossp : m_coverCrosses) scanEnclosing(crossp); if (offenderp) return offenderp; - // Pass 2: the coverpoint expression dereferences a class handle to reach a member, - // e.g. a parameterized covergroup 'covergroup cg(cls st); coverpoint st.test;'. The - // lowered covergroup class cannot resolve the handle argument, so lowering leaves a - // dangling VarRef and aborts in V3Broken ("Broken link ... VARREF"). AstMemberSel is - // class-handle member access only (packed-struct selects use AstStructSel and options - // use AstCoverOption), so scanning the value/iff expression will not flag supported - // coverpoints. + // Flag references to covergroup formal parameters as currently unsupported const auto scanHandleDeref = [&](AstNode* rootp) { if (!rootp) return; rootp->foreach([&](AstMemberSel* selp) { @@ -1821,13 +1806,8 @@ class FunctionalCoverageVisitor final : public VNVisitor { iterateChildren(nodep); - // Option B safety net for embedded covergroups: if a coverpoint/iff/cross - // references a member of the enclosing class (no handle to the enclosing - // instance), or dereferences a class-handle argument of a parameterized - // covergroup, lowering would emit uncompilable C++ or crash V3Broken with a - // dangling VarRef. Skip this covergroup with a clean warning rather than - // failing. (Full support - an enclosing back-pointer and handle-argument - // lowering - is the planned follow-up.) + // Identify embedded covergroup refs to enclosing class members or + // covergroup formal parameters and flag as currently unsupported. if (AstNode* const offenderp = findUnsupportedCoverpointRef(nodep)) { const bool viaHandle = VN_IS(offenderp, MemberSel); offenderp->v3warn(COVERIGN, diff --git a/test_regress/t/t_covergroup_embedded_unsup.out b/test_regress/t/t_covergroup_embedded_unsup.out index 3c09541ed..d74a490cf 100644 --- a/test_regress/t/t_covergroup_embedded_unsup.out +++ b/test_regress/t/t_covergroup_embedded_unsup.out @@ -1,11 +1,11 @@ -%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:31:34: Unsupported: 'covergroup' coverpoint referencing enclosing class member; ignoring covergroup '__vlAnonCG_cov_trans' +%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:23:34: Unsupported: 'covergroup' coverpoint referencing enclosing class member; ignoring covergroup '__vlAnonCG_cov_trans' : ... note: In instance 't' - 31 | trans_start_addr: coverpoint trans_collected.addr {option.auto_bin_max = 16;} + 23 | trans_start_addr: coverpoint trans_collected.addr {option.auto_bin_max = 16;} | ^~~~~~~~~~~~~~~ ... For warning description see https://verilator.org/warn/COVERIGN?v=latest ... Use "/* verilator lint_off COVERIGN */" and lint_on around source to disable this message. -%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:54:23: Unsupported: 'covergroup' coverpoint dereferencing a class handle member (parameterized covergroup); ignoring covergroup '__vlAnonCG_cov_param' +%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:46:23: Unsupported: 'covergroup' coverpoint dereferencing a class handle member (parameterized covergroup); ignoring covergroup '__vlAnonCG_cov_param' : ... note: In instance 't' - 54 | cp: coverpoint st.test; + 46 | cp: coverpoint st.test; | ^~~~ %Error: Exiting due to diff --git a/test_regress/t/t_covergroup_embedded_unsup.v b/test_regress/t/t_covergroup_embedded_unsup.v index 5e3297dcf..8a81eb4ab 100644 --- a/test_regress/t/t_covergroup_embedded_unsup.v +++ b/test_regress/t/t_covergroup_embedded_unsup.v @@ -5,17 +5,9 @@ // SPDX-FileCopyrightText: 2026 Wilson Snyder // SPDX-License-Identifier: CC0-1.0 -// Test the graceful-degradation safety net for covergroups that cannot yet be lowered. -// Two unsupported shapes must degrade to a clean COVERIGN warning (rather than emitting -// uncompilable C++ or crashing) until full support exists: -// 1. Embedded covergroup (the dominant UVM pattern): coverpoints reference members of -// the enclosing object. The covergroup is lowered into a sibling class with no -// handle to the enclosing instance, so emitting it would produce uncompilable C++ -// ("invalid use of non-static data member"). -// 2. Parameterized covergroup: a coverpoint dereferences a class-handle argument -// ('coverpoint st.test'). The lowered class cannot resolve the handle argument, so -// lowering leaves a dangling VarRef and aborts in V3Broken ("Broken link ... VARREF"). -// See https://github.com/verilator/verilator/issues/7853 +// Test that two currently-unsupported coverpoint reference styles are properly flagged +// as COVIGN: references to containing-class members ; references to covergroup formal +// parameters class ubus_transfer; bit [15:0] addr;