diff --git a/src/V3Covergroup.cpp b/src/V3Covergroup.cpp index f243b0c33..90bae71a2 100644 --- a/src/V3Covergroup.cpp +++ b/src/V3Covergroup.cpp @@ -1692,32 +1692,37 @@ class FunctionalCoverageVisitor final : public VNVisitor { } // VISITORS - AstNode* findEnclosingMemberRef(AstClass* cgClassp) { - // An embedded covergroup is lowered into a sibling AstClass that has no handle to - // the enclosing object. A coverpoint/iff/cross expression that references a - // (non-static) member of the enclosing class therefore emits C++ that accesses the - // member as if it were static ("invalid use of non-static data member"). Detect - // such references so the caller can skip lowering with a clean warning instead of - // producing uncompilable code. Returns the first offending node, or nullptr. - // Collect the covergroup class's own member variables (sample/constructor args); - // references to those are legitimate. + AstNode* findUnsupportedCoverpointRef(AstClass* cgClassp) { + // 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); } + // Flag non-static enclosing-class members reached without a handle as unsupported AstNode* offenderp = nullptr; - const auto scan = [&](AstNode* rootp) { + 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) scan(cpp); - for (AstCoverCross* crossp : m_coverCrosses) scan(crossp); + for (AstCoverpoint* cpp : m_coverpoints) scanEnclosing(cpp); + for (AstCoverCross* crossp : m_coverCrosses) scanEnclosing(crossp); + if (offenderp) return offenderp; + // Flag references to covergroup formal parameters as currently unsupported + const auto scanHandleDeref = [&](AstNode* rootp) { + if (!rootp) return; + rootp->foreach([&](AstMemberSel* selp) { + if (!offenderp) offenderp = selp; + }); + }; + for (AstCoverpoint* cpp : m_coverpoints) { + scanHandleDeref(cpp->exprp()); + scanHandleDeref(cpp->iffp()); + } return offenderp; } @@ -1801,16 +1806,16 @@ 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, lowering would emit uncompilable - // C++ (no handle to the enclosing instance). Skip this covergroup with a clean - // warning rather than crashing the C++ compile. (Full support - an enclosing - // back-pointer - is the planned follow-up.) - if (AstNode* const offenderp = findEnclosingMemberRef(nodep)) { + // 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, - "Unsupported: 'covergroup' coverpoint referencing enclosing " - "class member; ignoring covergroup " - << nodep->prettyNameQ()); + "Unsupported: 'covergroup' coverpoint " + << (viaHandle ? "dereferencing a class handle member " + "(parameterized covergroup)" + : "referencing enclosing class member") + << "; ignoring covergroup " << nodep->prettyNameQ()); for (AstCoverpoint* cpp : m_coverpoints) { VL_DO_DANGLING(pushDeletep(cpp->unlinkFrBack()), cpp); } diff --git a/test_regress/t/t_covergroup_embedded_unsup.out b/test_regress/t/t_covergroup_embedded_unsup.out index 93cf7a3a5..d74a490cf 100644 --- a/test_regress/t/t_covergroup_embedded_unsup.out +++ b/test_regress/t/t_covergroup_embedded_unsup.out @@ -1,7 +1,11 @@ -%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:27: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' - 27 | 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:46:23: Unsupported: 'covergroup' coverpoint dereferencing a class handle member (parameterized covergroup); ignoring covergroup '__vlAnonCG_cov_param' + : ... note: In instance 't' + 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 a35ffb840..8a81eb4ab 100644 --- a/test_regress/t/t_covergroup_embedded_unsup.v +++ b/test_regress/t/t_covergroup_embedded_unsup.v @@ -5,13 +5,9 @@ // SPDX-FileCopyrightText: 2026 Wilson Snyder // SPDX-License-Identifier: CC0-1.0 -// Test the graceful-degradation safety net for embedded covergroups (the dominant -// UVM pattern: a covergroup declared inside a class whose coverpoints reference the -// enclosing object's members). Such a 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"). Until the enclosing -// back-pointer feature exists, Verilator must emit a clean COVERIGN warning and skip -// lowering the covergroup, rather than crashing the C++ compile. +// 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; @@ -35,10 +31,34 @@ class ubus_master_monitor; endfunction endclass +class coverage_state; + bit [3:0] test; + bit [3:0] test2; +endclass + +class parameterized_monitor; + coverage_state cs; + + // Parameterized covergroup: the coverpoints dereference the class-handle argument 'st'. + // Two handle-dereferencing coverpoints ensure the safety net reports only the first + // offender (a second AstMemberSel is seen with the offender already latched). + covergroup cov_param(coverage_state st); + cp: coverpoint st.test; + cp2: coverpoint st.test2; + endgroup + + function new(); + cs = new; + cov_param = new(cs); + endfunction +endclass + module t; ubus_master_monitor m; + parameterized_monitor p; initial begin m = new; + p = new; $write("*-* All Finished *-*\n"); $finish; end