Tighten up comments

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>
This commit is contained in:
Matthew Ballance 2026-07-07 13:55:18 +00:00
parent 8d7a331d40
commit 930976ab30
3 changed files with 15 additions and 43 deletions

View File

@ -1693,41 +1693,26 @@ class FunctionalCoverageVisitor final : public VNVisitor {
// VISITORS // VISITORS
AstNode* findUnsupportedCoverpointRef(AstClass* cgClassp) { AstNode* findUnsupportedCoverpointRef(AstClass* cgClassp) {
// An embedded covergroup is lowered into a sibling AstClass that has no handle to // An embedded covergroup is lowered into a sibling AstClass that (currently) has
// the enclosing object. Two coverpoint/iff/cross shapes cannot be lowered and would // no handle to the enclosing object. Identify refs to the containing context
// otherwise emit uncompilable C++ or crash V3Broken; detect them so the caller can // or a formal param and flag as unsupported
// 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.
std::set<const AstVar*> ownVars; std::set<const AstVar*> ownVars;
for (AstNode* itemp = cgClassp->membersp(); itemp; itemp = itemp->nextp()) { for (AstNode* itemp = cgClassp->membersp(); itemp; itemp = itemp->nextp()) {
if (const AstVar* const varp = VN_CAST(itemp, Var)) ownVars.insert(varp); 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 // Flag non-static enclosing-class members reached without a handle as unsupported
// member would be emitted as if it were static ("invalid use of non-static data
// member").
AstNode* offenderp = nullptr; AstNode* offenderp = nullptr;
const auto scanEnclosing = [&](AstNode* rootp) { const auto scanEnclosing = [&](AstNode* rootp) {
rootp->foreach([&](AstVarRef* refp) { rootp->foreach([&](AstVarRef* refp) {
if (offenderp) return; if (offenderp) return;
const AstVar* const varp = refp->varp(); // Always set post-LinkDot const AstVar* const varp = refp->varp();
// 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.
if (varp->isClassMember() && !ownVars.count(varp)) offenderp = refp; if (varp->isClassMember() && !ownVars.count(varp)) offenderp = refp;
}); });
}; };
for (AstCoverpoint* cpp : m_coverpoints) scanEnclosing(cpp); for (AstCoverpoint* cpp : m_coverpoints) scanEnclosing(cpp);
for (AstCoverCross* crossp : m_coverCrosses) scanEnclosing(crossp); for (AstCoverCross* crossp : m_coverCrosses) scanEnclosing(crossp);
if (offenderp) return offenderp; if (offenderp) return offenderp;
// Pass 2: the coverpoint expression dereferences a class handle to reach a member, // Flag references to covergroup formal parameters as currently unsupported
// 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.
const auto scanHandleDeref = [&](AstNode* rootp) { const auto scanHandleDeref = [&](AstNode* rootp) {
if (!rootp) return; if (!rootp) return;
rootp->foreach([&](AstMemberSel* selp) { rootp->foreach([&](AstMemberSel* selp) {
@ -1821,13 +1806,8 @@ class FunctionalCoverageVisitor final : public VNVisitor {
iterateChildren(nodep); iterateChildren(nodep);
// Option B safety net for embedded covergroups: if a coverpoint/iff/cross // Identify embedded covergroup refs to enclosing class members or
// references a member of the enclosing class (no handle to the enclosing // covergroup formal parameters and flag as currently unsupported.
// 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.)
if (AstNode* const offenderp = findUnsupportedCoverpointRef(nodep)) { if (AstNode* const offenderp = findUnsupportedCoverpointRef(nodep)) {
const bool viaHandle = VN_IS(offenderp, MemberSel); const bool viaHandle = VN_IS(offenderp, MemberSel);
offenderp->v3warn(COVERIGN, offenderp->v3warn(COVERIGN,

View File

@ -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' : ... 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 ... 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. ... 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' : ... note: In instance 't'
54 | cp: coverpoint st.test; 46 | cp: coverpoint st.test;
| ^~~~ | ^~~~
%Error: Exiting due to %Error: Exiting due to

View File

@ -5,17 +5,9 @@
// SPDX-FileCopyrightText: 2026 Wilson Snyder // SPDX-FileCopyrightText: 2026 Wilson Snyder
// SPDX-License-Identifier: CC0-1.0 // SPDX-License-Identifier: CC0-1.0
// Test the graceful-degradation safety net for covergroups that cannot yet be lowered. // Test that two currently-unsupported coverpoint reference styles are properly flagged
// Two unsupported shapes must degrade to a clean COVERIGN warning (rather than emitting // as COVIGN: references to containing-class members ; references to covergroup formal
// uncompilable C++ or crashing) until full support exists: // parameters
// 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
class ubus_transfer; class ubus_transfer;
bit [15:0] addr; bit [15:0] addr;