From 07f959d31d2e265673ffe71688b595cb9b10ca0f Mon Sep 17 00:00:00 2001 From: Matthew Ballance Date: Sat, 28 Feb 2026 20:10:20 +0000 Subject: [PATCH] Refactoring node locations and enums ; Delete coverpoints after V3Covergroup so they don't accidentally hit the generators Signed-off-by: Matthew Ballance --- docs/guide/languages.rst | 4 +- docs/guide/simulating.rst | 380 ++---------------------- src/V3Active.cpp | 8 +- src/V3AstAttr.h | 107 +++++++ src/V3AstNodeOther.h | 26 -- src/V3AstNodes.cpp | 64 +--- src/V3Covergroup.cpp | 18 +- src/V3DfgOptimizer.cpp | 4 +- src/V3EmitCFunc.h | 14 - src/V3MergeCond.cpp | 6 - test_regress/t/t_covergroup_database.py | 4 +- test_regress/t/t_covergroup_database.v | 4 +- 12 files changed, 173 insertions(+), 466 deletions(-) diff --git a/docs/guide/languages.rst b/docs/guide/languages.rst index 989549be6..70ded1ce2 100644 --- a/docs/guide/languages.rst +++ b/docs/guide/languages.rst @@ -365,14 +365,14 @@ appropriate width. Assertions ---------- -Verilator is beginning to add support for assertions and functional coverage. +Verilator partially supports assertions and functional coverage. Verilator currently converts assertions to simple ``if (...) error`` statements, and simple coverage statements to increment the line counters described in the :ref:`coverage section`. Verilator also partially supports SystemVerilog functional coverage with ``covergroup``, ``coverpoint``, bins, cross coverage, and transition bins. See -the :ref:`Functional Coverage` section for details on using +:ref:`Functional Coverage` for details on using covergroups for comprehensive coverage analysis. Verilator does not support SEREs yet. All assertion and coverage statements diff --git a/docs/guide/simulating.rst b/docs/guide/simulating.rst index 094afffa7..6f52d1174 100644 --- a/docs/guide/simulating.rst +++ b/docs/guide/simulating.rst @@ -184,7 +184,8 @@ Verilator supports adding code to the Verilated model to support SystemVerilog code coverage. With :vlopt:`--coverage`, Verilator enables all forms of coverage: -- :ref:`User Coverage` +- :ref:`Property Coverage` +- :ref:`Covergroup Coverage` - :ref:`Line Coverage` - :ref:`Toggle Coverage` @@ -192,19 +193,14 @@ When a model with coverage is executed, it will create a coverage file for collection and later analysis, see :ref:`Coverage Collection`. -.. _user coverage: +.. _property coverage: -Functional Coverage -------------------- +Property Coverage +----------------- With :vlopt:`--coverage` or :vlopt:`--coverage-user`, Verilator will -translate functional coverage points the user has inserted manually in -SystemVerilog code through into the Verilated model. Verilator supports both -simple coverage points and full covergroup-based functional coverage as -defined in IEEE 1800-2023 Section 19. - -Simple Coverage Points -^^^^^^^^^^^^^^^^^^^^^^ +translate property coverage points the user has inserted manually in +SystemVerilog code into the Verilated model. For simple coverage points, use the ``cover property`` construct: @@ -214,14 +210,15 @@ For simple coverage points, use the ``cover property`` construct: This adds a coverage point that tracks whether the condition has been observed. -Covergroups -^^^^^^^^^^^ +.. _covergroup coverage: -Verilator supports SystemVerilog covergroups for comprehensive functional -coverage. A covergroup defines a set of coverage points (coverpoints) with -bins that track specific values or value ranges. +Covergroup Coverage +------------------- -**Basic Example:** +With :vlopt:`--coverage` or :vlopt:`--coverage-user`, Verilator will +translate covergroup coverage points the user has inserted manually in +SystemVerilog code into the Verilated model. Verilator supports +coverpoints with value and transition bins, and cross points. .. code-block:: sv @@ -250,200 +247,26 @@ bins that track specific values or value ranges. end endmodule -**Important:** Verilator requires explicit ``sample()`` calls. The automatic -sampling syntax ``covergroup cg @(posedge clk);`` is parsed but the automatic -sampling is not performed. Always call ``sample()`` explicitly in your code. -Coverpoint Bins -^^^^^^^^^^^^^^^ - -Bins define which values to track for coverage. Verilator supports several bin types: - -**Value Bins:** - -.. code-block:: sv - - coverpoint state { - bins idle = {0}; - bins active = {1, 2, 3}; - bins error = {4}; - } - -**Range Bins:** - -.. code-block:: sv - - coverpoint addr { - bins low = {[0:63]}; - bins medium = {[64:127]}; - bins high = {[128:255]}; - } - -**Array Bins (Automatic):** - -.. code-block:: sv - - coverpoint state { - bins state[] = {[0:3]}; // Creates bins: state[0], state[1], state[2], state[3] - } - -**Wildcard Bins:** - -.. code-block:: sv - - coverpoint opcode { - wildcard bins load_ops = {4'b00??}; // Matches 0000, 0001, 0010, 0011 - wildcard bins store_ops = {4'b01??}; // Matches 0100, 0101, 0110, 0111 - } - -**Special Bins:** - -.. code-block:: sv - - coverpoint value { - bins valid[] = {[0:10]}; - ignore_bins unused = {11, 12, 13}; // Don't track these values - illegal_bins bad = {[14:15]}; // Report error if seen - } - -The ``ignore_bins`` are excluded from coverage calculation, while ``illegal_bins`` -will cause a runtime error if sampled. - -**Default Bins:** - -.. code-block:: sv - - coverpoint state { - bins defined = {0, 1, 2}; - bins others = default; // Catches all other values - } - -Cross Coverage -^^^^^^^^^^^^^^ - -Cross coverage tracks combinations of values from multiple coverpoints: - -.. code-block:: sv - - covergroup cg; - cp_cmd: coverpoint cmd; - cp_addr: coverpoint addr { - bins low = {[0:127]}; - bins high = {[128:255]}; - } - - // Cross coverage of command and address - cross_cmd_addr: cross cp_cmd, cp_addr; - endgroup - -The cross automatically creates bins for all combinations: ``(read, low)``, -``(read, high)``, ``(write, low)``, ``(write, high)``. - -Verilator supports arbitrary N-way cross coverage. - -Transition Bins -^^^^^^^^^^^^^^^ - -Transition bins track sequences of values across multiple samples: - -.. code-block:: sv - - covergroup cg; - coverpoint state { - bins trans_idle_active = (0 => 1); // idle to active - bins trans_active_done = (1 => 2); // active to done - bins trans_done_idle = (2 => 0); // done back to idle - } - endgroup - -**Supported Syntax:** - -Verilator supports multi-value transition sequences: - -.. code-block:: sv - - coverpoint state { - // Two-value transitions - bins trans_2 = (0 => 1); - - // Multi-value transitions - bins trans_3 = (0 => 1 => 2); - bins trans_4 = (0 => 1 => 2 => 3); - - // Transitions with value sets - bins trans_set = (0, 1 => 2, 3); // (0=>2), (0=>3), (1=>2), (1=>3) - } - -**Unsupported Repetition Operators:** - -Verilator does not currently support IEEE 1800-2023 transition bin repetition -operators. The following syntax will generate a ``COVERIGN`` warning and be -ignored: - -* **Consecutive repetition** ``[*N]`` - Repeat value N times consecutively - - .. code-block:: sv - - bins trans = (1 => 2 [*3] => 3); // Unsupported: 1, 2, 2, 2, 3 - -* **Goto repetition** ``[->N]`` - See value N times with any gaps, next value follows immediately - - .. code-block:: sv - - bins trans = (1 => 2 [->3] => 3); // Unsupported: 1, 2, X, 2, Y, 2, 3 - -* **Nonconsecutive repetition** ``[=N]`` - See value N times with gaps allowed everywhere - - .. code-block:: sv - - bins trans = (1 => 2 [=3] => 3); // Unsupported: 1, 2, X, 2, Y, 2, Z, 3 - -If you need repetition behavior, consider using multiple bins to represent the -desired sequences explicitly. - -Bin Options -^^^^^^^^^^^ - -Individual bins can have options: - -.. code-block:: sv - - coverpoint state { - bins idle = {0} with (option.at_least = 10); // Must see 10 times - } - -Querying Coverage -^^^^^^^^^^^^^^^^^ - -To get the current coverage percentage: - -.. code-block:: sv - - real cov = cg_inst.get_inst_coverage(); - $display("Coverage: %0.1f%%", cov); - -The ``get_inst_coverage()`` method returns a real value from 0.0 to 100.0 -representing the percentage of bins that have been hit. - -Coverage Reports -^^^^^^^^^^^^^^^^ - -When running with :vlopt:`--coverage`, Verilator generates coverage data files -that can be analyzed with the :ref:`verilator_coverage` -tool: - -.. code-block:: bash - - # Run simulation with coverage enabled - $ verilator --coverage --exe --build sim.cpp top.v - $ ./obj_dir/Vtop - - # Generate coverage report - $ verilator_coverage --annotate coverage_report coverage.dat - $ verilator_coverage --write merged.dat coverage.dat - -The coverage data integrates with Verilator's existing coverage infrastructure, -so you can view functional coverage alongside line and toggle coverage. +Supported Features +^^^^^^^^^^^^^^^^^^ +* Coverpoints on integral expressions with value, range, wildcard, and transition bins +* Conditional coverpoint sampling (iff) +* Explicit and clocked sampling, with sample-function parameters +* at_least and auto_bin_max options on covergroups and coverpoints +* Cross points with auto-bins + +Unsupported Features +^^^^^^^^^^^^^^^^^^^^ + +* Coverpoints on real (floating-point) expressions +* Coverpoint bin filtering (with) +* Coverpoint bin conditional sampling (iff) +* Transition bins with repetition operators ([\*N], [->N], [=N]) +* Explicitly-typed coverpoints +* Block-event sampling +* Covergroup inheritance (extends) +* Cross points with user-defined bins Functional Coverage Data Format ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -477,143 +300,6 @@ with :command:`verilator_coverage`: # Exclude functional coverage $ verilator_coverage --filter-type '!funccov' --annotate report coverage.dat -Covergroup Options -^^^^^^^^^^^^^^^^^^ - -Covergroups support various options: - -.. code-block:: sv - - covergroup cg with function sample(logic [7:0] addr); - option.name = "my_covergroup"; - option.comment = "Address coverage"; - - coverpoint addr; - endgroup - -Parameterized sampling allows passing values directly to ``sample()``: - -.. code-block:: sv - - cg cg_inst = new; - cg_inst.sample(addr_value); - -Dynamic Covergroup Creation -^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Covergroups can be created dynamically at runtime: - -.. code-block:: sv - - cg cg_inst; - - initial begin - if (enable_coverage) begin - cg_inst = new; - end - end - -Covergroups in Classes -^^^^^^^^^^^^^^^^^^^^^^^ - -Covergroups can be defined inside classes: - -.. code-block:: sv - - class MyClass; - logic [7:0] data; - - covergroup cg; - coverpoint data; - endgroup - - function new(); - cg = new; - endfunction - - task record(); - cg.sample(); - endtask - endclass - -Limitations and Unsupported Features -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -**Automatic Sampling:** The syntax ``covergroup cg @(posedge clk);`` is parsed -but automatic sampling is not performed. Use explicit ``sample()`` calls: - -.. code-block:: sv - - // Instead of this: - covergroup cg @(posedge clk); // Automatic sampling not supported - ... - endgroup - - // Do this: - covergroup cg; - ... - endgroup - - cg cg_inst = new; - always @(posedge clk) cg_inst.sample(); // Explicit sampling - -**Covergroup Inheritance:** Covergroup inheritance using the ``extends`` keyword -is not currently supported. This will generate an error: - -.. code-block:: sv - - covergroup base_cg; - coverpoint value; - endgroup - - covergroup derived_cg extends base_cg; // Not supported - coverpoint other_value; - endgroup - -As a workaround, duplicate the coverpoint definitions in each covergroup. - -**Type-Level (Static) Coverage:** Aggregated type-level coverage using the -static ``get_coverage()`` method is not currently supported. Only instance-level -coverage via ``get_inst_coverage()`` is available: - -.. code-block:: sv - - covergroup cg; - coverpoint value; - endgroup - - cg cg1 = new; - cg cg2 = new; - - // This works - instance-level coverage - real inst_cov = cg1.get_inst_coverage(); - - // This is not supported - type-level coverage - // real type_cov = cg::get_coverage(); // Will not aggregate across instances - -**Advanced Transition Features:** Complex transition patterns including -multi-value transitions with more than 2 states may have incomplete case -statement coverage in generated code. Simple 2-state transitions work correctly: - -.. code-block:: sv - - coverpoint state { - // This works well - bins trans_2state = (0 => 1); - - // This may generate incomplete case statements - bins trans_3state = (0 => 1 => 2); // Limited support - } - -**Transition Bin Repetition Operators:** The repetition operators ``[*N]``, -``[->N]``, and ``[=N]`` for transition bins are not supported. Use multiple -explicit bins to represent repeated sequences. See the -:ref:`Transition Bins` section for details. - -For a complete list of supported features and current implementation status, -see the functional coverage plan in the Verilator source tree at -``docs/functional_coverage_plan.md``. - .. _line coverage: diff --git a/src/V3Active.cpp b/src/V3Active.cpp index 7410851e5..f69cb2b25 100644 --- a/src/V3Active.cpp +++ b/src/V3Active.cpp @@ -783,10 +783,10 @@ class CovergroupSamplingVisitor final : public VNVisitor { UINFO(4, "Fixed VarRef in SenTree: " << refp->varp()->name() << " -> " << vscp->name() << endl); } else { - UINFO(4, "WARNING: Could not find VarScope for " - << refp->varp()->name() << " in scope " << m_scopep->name() - << " - automatic sampling may not work for internal clocks" - << endl); + refp->v3fatalSrc("Could not find VarScope for clock signal '" + << refp->varp()->name() << "' in scope " + << m_scopep->name() + << " when creating covergroup sampling active"); } } }); diff --git a/src/V3AstAttr.h b/src/V3AstAttr.h index 6a81c993c..abd0e716d 100644 --- a/src/V3AstAttr.h +++ b/src/V3AstAttr.h @@ -1107,6 +1107,113 @@ inline std::ostream& operator<<(std::ostream& os, const VCastable& rhs) { //###################################################################### +class VCoverBinsType final { +public: + enum en : uint8_t { + USER, + ARRAY, + AUTO, + BINS_IGNORE, // Renamed to avoid Windows macro conflict + BINS_ILLEGAL, // Renamed to avoid Windows macro conflict + DEFAULT, + BINS_WILDCARD, // Renamed to avoid Windows macro conflict + TRANSITION + }; + enum en m_e; + VCoverBinsType() + : m_e{USER} {} + // cppcheck-suppress noExplicitConstructor + constexpr VCoverBinsType(en _e) + : m_e{_e} {} + explicit VCoverBinsType(int _e) + : m_e(static_cast(_e)) {} // Need () or GCC 4.8 false warning + constexpr operator en() const { return m_e; } + const char* ascii() const { + static const char* const names[] + = {"user", "array", "auto", "ignore", "illegal", "default", "wildcard", "transition"}; + return names[m_e]; + } +}; +constexpr bool operator==(const VCoverBinsType& lhs, const VCoverBinsType& rhs) { + return lhs.m_e == rhs.m_e; +} +constexpr bool operator==(const VCoverBinsType& lhs, VCoverBinsType::en rhs) { + return lhs.m_e == rhs; +} +constexpr bool operator==(VCoverBinsType::en lhs, const VCoverBinsType& rhs) { + return lhs == rhs.m_e; +} + +//###################################################################### + +class VCoverOptionType final { +public: + enum en : uint8_t { WEIGHT, GOAL, AT_LEAST, AUTO_BIN_MAX, PER_INSTANCE, COMMENT }; + enum en m_e; + VCoverOptionType() + : m_e{WEIGHT} {} + // cppcheck-suppress noExplicitConstructor + constexpr VCoverOptionType(en _e) + : m_e{_e} {} + explicit VCoverOptionType(int _e) + : m_e(static_cast(_e)) {} // Need () or GCC 4.8 false warning + constexpr operator en() const { return m_e; } + const char* ascii() const { + static const char* const names[] + = {"weight", "goal", "at_least", "auto_bin_max", "per_instance", "comment"}; + return names[m_e]; + } +}; +constexpr bool operator==(const VCoverOptionType& lhs, const VCoverOptionType& rhs) { + return lhs.m_e == rhs.m_e; +} +constexpr bool operator==(const VCoverOptionType& lhs, VCoverOptionType::en rhs) { + return lhs.m_e == rhs; +} +constexpr bool operator==(VCoverOptionType::en lhs, const VCoverOptionType& rhs) { + return lhs == rhs.m_e; +} + +//###################################################################### + +class VTransRepType final { +public: + enum en : uint8_t { + NONE, // No repetition + CONSEC, // Consecutive repetition [*] + GOTO, // Goto repetition [->] + NONCONS // Nonconsecutive repetition [=] + }; + enum en m_e; + VTransRepType() + : m_e{NONE} {} + // cppcheck-suppress noExplicitConstructor + constexpr VTransRepType(en _e) + : m_e{_e} {} + explicit VTransRepType(int _e) + : m_e(static_cast(_e)) {} // Need () or GCC 4.8 false warning + constexpr operator en() const { return m_e; } + const char* ascii() const { + static const char* const names[] = {"", "[*]", "[->]", "[=]"}; + return names[m_e]; + } + const char* asciiJson() const { + static const char* const names[] = {"", "\"consec\"", "\"goto\"", "\"noncons\""}; + return names[m_e]; + } +}; +constexpr bool operator==(const VTransRepType& lhs, const VTransRepType& rhs) { + return lhs.m_e == rhs.m_e; +} +constexpr bool operator==(const VTransRepType& lhs, VTransRepType::en rhs) { + return lhs.m_e == rhs; +} +constexpr bool operator==(VTransRepType::en lhs, const VTransRepType& rhs) { + return lhs == rhs.m_e; +} + +//###################################################################### + class VDirection final { public: enum en : uint8_t { NONE, INPUT, OUTPUT, INOUT, REF, CONSTREF }; diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 35c87d1f9..8c0f856a5 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1043,32 +1043,6 @@ public: class AstCoverTransSet; class AstCoverSelectExpr; -enum class VCoverBinsType : uint8_t { - USER, - ARRAY, - AUTO, - BINS_IGNORE, // Renamed to avoid Windows macro conflict - BINS_ILLEGAL, // Renamed to avoid Windows macro conflict - DEFAULT, - BINS_WILDCARD, // Renamed to avoid Windows macro conflict - TRANSITION -}; - -enum class VCoverOptionType : uint8_t { - WEIGHT, - GOAL, - AT_LEAST, - AUTO_BIN_MAX, - PER_INSTANCE, - COMMENT -}; - -enum class VTransRepType : uint8_t { - NONE, // No repetition - CONSEC, // Consecutive repetition [*] - GOTO, // Goto repetition [->] - NONCONS // Nonconsecutive repetition [=] -}; class AstCoverBin final : public AstNode { // @astgen op1 := rangesp : List[AstNode] diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 455b9dc68..266cc35c8 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -3506,9 +3506,8 @@ void AstCovergroup::dump(std::ostream& str) const { } void AstCovergroup::dumpJson(std::ostream& str) const { - this->AstNode::dumpJson(str); - str << ", \"name\": " << VString::quotePercent(name()); - if (m_isClass) str << ", \"isClass\": true"; + dumpJsonBoolFuncIf(str, isClass); + dumpJsonGen(str); } void AstCoverpoint::dump(std::ostream& str) const { this->AstNodeFuncCovItem::dump(str); } @@ -3517,57 +3516,26 @@ void AstCoverpoint::dumpJson(std::ostream& str) const { this->AstNodeFuncCovItem void AstCoverBin::dump(std::ostream& str) const { this->AstNode::dump(str); - str << " " << m_name << " "; - switch (m_type) { - case VCoverBinsType::USER: str << "user"; break; - case VCoverBinsType::ARRAY: str << "array"; break; - case VCoverBinsType::AUTO: str << "auto"; break; - case VCoverBinsType::BINS_IGNORE: str << "ignore"; break; - case VCoverBinsType::BINS_ILLEGAL: str << "illegal"; break; - case VCoverBinsType::DEFAULT: str << "default"; break; - case VCoverBinsType::BINS_WILDCARD: str << "wildcard"; break; - case VCoverBinsType::TRANSITION: str << "transition"; break; - } + str << " " << m_name << " " << m_type.ascii(); if (m_isArray) str << "[]"; } void AstCoverBin::dumpJson(std::ostream& str) const { this->AstNode::dumpJson(str); str << ", \"name\": " << VString::quotePercent(m_name); - str << ", \"binsType\": "; - switch (m_type) { - case VCoverBinsType::USER: str << "\"user\""; break; - case VCoverBinsType::ARRAY: str << "\"array\""; break; - case VCoverBinsType::AUTO: str << "\"auto\""; break; - case VCoverBinsType::BINS_IGNORE: str << "\"ignore\""; break; - case VCoverBinsType::BINS_ILLEGAL: str << "\"illegal\""; break; - case VCoverBinsType::DEFAULT: str << "\"default\""; break; - case VCoverBinsType::BINS_WILDCARD: str << "\"wildcard\""; break; - case VCoverBinsType::TRANSITION: str << "\"transition\""; break; - } + str << ", \"binsType\": \"" << m_type.ascii() << "\""; if (m_isArray) str << ", \"isArray\": true"; } void AstCoverTransItem::dump(std::ostream& str) const { this->AstNode::dump(str); - switch (m_repType) { - case VTransRepType::NONE: break; - case VTransRepType::CONSEC: str << " [*]"; break; - case VTransRepType::GOTO: str << " [->]"; break; - case VTransRepType::NONCONS: str << " [=]"; break; - } + if (m_repType != VTransRepType::NONE) str << " " << m_repType.ascii(); } void AstCoverTransItem::dumpJson(std::ostream& str) const { this->AstNode::dumpJson(str); if (m_repType != VTransRepType::NONE) { - str << ", \"repType\": "; - switch (m_repType) { - case VTransRepType::NONE: break; - case VTransRepType::CONSEC: str << "\"consec\""; break; - case VTransRepType::GOTO: str << "\"goto\""; break; - case VTransRepType::NONCONS: str << "\"noncons\""; break; - } + str << ", \"repType\": " << m_repType.asciiJson(); } } @@ -3594,28 +3562,12 @@ void AstCoverCrossBins::dumpJson(std::ostream& str) const { void AstCoverOption::dump(std::ostream& str) const { this->AstNode::dump(str); - str << " "; - switch (m_type) { - case VCoverOptionType::WEIGHT: str << "weight"; break; - case VCoverOptionType::GOAL: str << "goal"; break; - case VCoverOptionType::AT_LEAST: str << "at_least"; break; - case VCoverOptionType::AUTO_BIN_MAX: str << "auto_bin_max"; break; - case VCoverOptionType::PER_INSTANCE: str << "per_instance"; break; - case VCoverOptionType::COMMENT: str << "comment"; break; - } + str << " " << m_type.ascii(); } void AstCoverOption::dumpJson(std::ostream& str) const { this->AstNode::dumpJson(str); - str << ", \"optionType\": "; - switch (m_type) { - case VCoverOptionType::WEIGHT: str << "\"weight\""; break; - case VCoverOptionType::GOAL: str << "\"goal\""; break; - case VCoverOptionType::AT_LEAST: str << "\"at_least\""; break; - case VCoverOptionType::AUTO_BIN_MAX: str << "\"auto_bin_max\""; break; - case VCoverOptionType::PER_INSTANCE: str << "\"per_instance\""; break; - case VCoverOptionType::COMMENT: str << "\"comment\""; break; - } + str << ", \"optionType\": \"" << m_type.ascii() << "\""; } void AstCoverpointRef::dump(std::ostream& str) const { diff --git a/src/V3Covergroup.cpp b/src/V3Covergroup.cpp index 5de7067b2..48e9bac41 100644 --- a/src/V3Covergroup.cpp +++ b/src/V3Covergroup.cpp @@ -544,7 +544,7 @@ class FunctionalCoverageVisitor final : public VNVisitor { // Note: Coverage database registration happens later via VL_COVER_INSERT // (see generateCoverageDeclarations() method around line 1164) - // Classes use "v_funccov/" hier prefix vs modules + // Classes use "v_covergroup/" hier prefix vs modules // Generate bin matching code in sample() // Handle transition bins specially @@ -1745,15 +1745,15 @@ class FunctionalCoverageVisitor final : public VNVisitor { } hierName += "." + binName; - // Generate: VL_COVER_INSERT(contextp, hier, &binVar, "page", "v_funccov/...", ...) + // Generate: VL_COVER_INSERT(contextp, hier, &binVar, "page", "v_covergroup/...", ...) UINFO(6, " Registering bin: " << hierName << " -> " << varp->name() << endl); // Build the coverage insert as a C statement // The variable reference needs to be &this->varname, where varname gets mangled to - // __PVT__varname Use "page" field with v_funccov prefix so type is extracted correctly + // __PVT__varname Use "page" field with v_covergroup prefix so type is extracted correctly // (consistent with code coverage) - std::string pageName = "v_funccov/" + m_covergroupp->name(); + std::string pageName = "v_covergroup/" + m_covergroupp->name(); std::string insertCall = "VL_COVER_INSERT(vlSymsp->_vm_contextp__->coveragep(), "; insertCall += "\"" + hierName + "\", "; insertCall += "&(this->__PVT__" + varp->name() + "), "; @@ -1840,6 +1840,16 @@ class FunctionalCoverageVisitor final : public VNVisitor { iterateChildren(nodep); processCovergroup(); + // Remove lowered coverpoints/crosses from the class - they have been + // fully translated into C++ code and must not reach downstream passes + for (AstCoverpoint* cpp : m_coverpoints) { + cpp->unlinkFrBack(); + VL_DO_DANGLING(cpp->deleteTree(), cpp); + } + for (AstCoverCross* crossp : m_coverCrosses) { + crossp->unlinkFrBack(); + VL_DO_DANGLING(crossp->deleteTree(), crossp); + } } else { iterateChildren(nodep); } diff --git a/src/V3DfgOptimizer.cpp b/src/V3DfgOptimizer.cpp index 8a25e8ef8..b5448cffb 100644 --- a/src/V3DfgOptimizer.cpp +++ b/src/V3DfgOptimizer.cpp @@ -285,9 +285,7 @@ class DataflowOptimize final { // TODO: remove once Actives can tolerate NEVER SenItems if (AstSenItem* senItemp = VN_CAST(nodep, SenItem)) { senItemp->foreach([](const AstVarRef* refp) { - // Check varScopep exists before accessing (may be null for covergroup - // events) - if (refp->varScopep()) DfgVertexVar::setHasExtRdRefs(refp->varScopep()); + DfgVertexVar::setHasExtRdRefs(refp->varScopep()); }); return; } diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index e75803db5..6192c8c33 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -1785,20 +1785,6 @@ public: iterateChildrenConst(nodep); } - // Functional coverage nodes - not yet implemented, just skip for now - void visit(AstCoverpoint* nodep) override { - // Functional coverage nodes are handled during the coverage transformation pass - // They should not reach the C++ emitter - } - void visit(AstCoverBin* nodep) override { - // Functional coverage nodes are handled during the coverage transformation pass - // They should not reach the C++ emitter - } - void visit(AstCoverCross* nodep) override { - // Functional coverage nodes are handled during the coverage transformation pass - // They should not reach the C++ emitter - } - // Default void visit(AstNode* nodep) override { // LCOV_EXCL_START putns(nodep, "\n???? // "s + nodep->prettyTypeName() + "\n"); diff --git a/src/V3MergeCond.cpp b/src/V3MergeCond.cpp index 3e34b4dc1..0ed6f4cd9 100644 --- a/src/V3MergeCond.cpp +++ b/src/V3MergeCond.cpp @@ -299,12 +299,6 @@ class CodeMotionAnalysisVisitor final : public VNVisitorConst { iterateChildrenConst(nodep); } - // VISITORS - void visit(AstCoverpoint* nodep) override { - // Coverpoints are not statements, so don't analyze their expressions - // They will be handled during code generation - // Just skip them to avoid null pointer access in m_propsp - } void visit(AstNode* nodep) override { // Push a new stack entry at the start of a list, but only if the list is not a // single element (this saves a lot of allocations in expressions) diff --git a/test_regress/t/t_covergroup_database.py b/test_regress/t/t_covergroup_database.py index dda88c088..b1a3cf689 100755 --- a/test_regress/t/t_covergroup_database.py +++ b/test_regress/t/t_covergroup_database.py @@ -16,8 +16,8 @@ test.compile(verilator_flags2=['--coverage']) test.execute() # Check that coverage database contains functional coverage entries -# Format uses control characters as delimiters: C '^At^Bfunccov^Apage...bin^Blow...h^Bcg.cp.low' count -test.file_grep(test.coverage_filename, r'funccov') +# Format uses control characters as delimiters: C '^At^Bcovergroup^Apage...bin^Blow...h^Bcg.cp.low' count +test.file_grep(test.coverage_filename, r'covergroup') test.file_grep(test.coverage_filename, r'bin.{0,2}low') # binlow with possible delimiter test.file_grep(test.coverage_filename, r'bin.{0,2}high') # binhigh with possible delimiter test.file_grep(test.coverage_filename, r'cg\.cp\.low') diff --git a/test_regress/t/t_covergroup_database.v b/test_regress/t/t_covergroup_database.v index ad7b1f3c6..5b40792e5 100644 --- a/test_regress/t/t_covergroup_database.v +++ b/test_regress/t/t_covergroup_database.v @@ -5,10 +5,10 @@ // SPDX-License-Identifier: CC0-1.0 // Test that functional coverage is properly written to coverage database -// Checks that coverage.dat contains funccov entries with correct format +// Checks that coverage.dat contains covergroup entries with correct format // Expected coverage database entries will contain: -// - Type "funccov" +// - Type "covergroup" // - Bin names ("low", "high") // - Hierarchy ("cg.cp.low", "cg.cp.high")