From 2db34818b3d65cdee1262a458a395b21d4cfbbf3 Mon Sep 17 00:00:00 2001 From: Marco Bartoli Date: Mon, 8 Jun 2026 02:47:43 +0200 Subject: [PATCH] Fix parameter values in coverage bins widths (#7732) (#7734) Fixes #7732. --- src/V3Width.cpp | 36 ++++++++-- test_regress/t/t_covergroup_param_bins.out | 5 ++ test_regress/t/t_covergroup_param_bins.py | 12 ++++ test_regress/t/t_covergroup_param_bins.v | 76 ++++++++++++++++++++++ 4 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 test_regress/t/t_covergroup_param_bins.out create mode 100755 test_regress/t/t_covergroup_param_bins.py create mode 100644 test_regress/t/t_covergroup_param_bins.v diff --git a/src/V3Width.cpp b/src/V3Width.cpp index e8df0f4df..ff3d8431b 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1926,6 +1926,25 @@ class WidthVisitor final : public VNVisitor { if (nodep->iffp()) iterateCheckBool(nodep, "iff condition", nodep->iffp(), BOTH); userIterateAndNext(nodep->optionsp(), nullptr); } + void visit(AstCoverBin* nodep) override { + // Bin range/value entries are self-determined constant expressions (IEEE 1800-2023 + // 19.5). Width each plain single-value entry self-determined so a referenced + // parameter acquires a dtype, then constify so the reference folds to the AstConst + // value that V3Covergroup requires. AstInsideRange entries fold their own bounds in + // visit(AstInsideRange). + for (AstNode *nextp, *itemp = nodep->rangesp(); itemp; itemp = nextp) { + nextp = itemp->nextp(); + if (VN_IS(itemp, InsideRange)) { + userIterate(itemp, nullptr); + } else { + userIterate(itemp, WidthVP{SELF, BOTH}.p()); + V3Const::constifyEdit(itemp); // itemp may change + } + } + userIterateAndNext(nodep->iffp(), nullptr); + userIterateAndNext(nodep->arraySizep(), nullptr); + userIterateAndNext(nodep->transp(), nullptr); + } void visit(AstPow* nodep) override { // Pow is special, output sign only depends on LHS sign, but // function result depends on both signs @@ -3566,17 +3585,20 @@ class WidthVisitor final : public VNVisitor { } void visit(AstInsideRange* nodep) override { // Just do each side; AstInside will rip these nodes out later. - // When m_vup is null, this range appears outside a normal expression context (e.g. - // in a covergroup bin declaration). Pre-fold constant arithmetic in that case - // (e.g., AstNegate(Const) -> Const) so children have their types set before widthing. - // We cannot do this unconditionally: in a normal 'inside' expression (m_vup set), - // range bounds may be enum refs not yet widthed, and constifyEdit would crash. if (!m_vup) { + // This range appears outside a normal expression context (e.g. in a covergroup + // bin declaration). Width the bounds self-determined first so each bound (and any + // referenced parameter) has a dtype, then constify so constant arithmetic is folded + // (e.g. AstNegate(Const) -> Const, or a parameter reference -> Const). Folding + // preserves the now-present dtype, so no bound reaches V3WidthCommit without one. + userIterateAndNext(nodep->lhsp(), WidthVP{SELF, BOTH}.p()); + userIterateAndNext(nodep->rhsp(), WidthVP{SELF, BOTH}.p()); V3Const::constifyEdit(nodep->lhsp()); // lhsp may change V3Const::constifyEdit(nodep->rhsp()); // rhsp may change + } else { + userIterateAndNext(nodep->lhsp(), m_vup); + userIterateAndNext(nodep->rhsp(), m_vup); } - userIterateAndNext(nodep->lhsp(), m_vup); - userIterateAndNext(nodep->rhsp(), m_vup); nodep->dtypeFrom(nodep->lhsp()); } diff --git a/test_regress/t/t_covergroup_param_bins.out b/test_regress/t/t_covergroup_param_bins.out new file mode 100644 index 000000000..f0cb20f17 --- /dev/null +++ b/test_regress/t/t_covergroup_param_bins.out @@ -0,0 +1,5 @@ +cg.cp.maxv: 2 +cg.cp.minv: 1 +cg.cp.negative: 1 +cg.cp.positive: 3 +cg.cp.zero: 1 diff --git a/test_regress/t/t_covergroup_param_bins.py b/test_regress/t/t_covergroup_param_bins.py new file mode 100755 index 000000000..ab1d96563 --- /dev/null +++ b/test_regress/t/t_covergroup_param_bins.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap +import coverage_covergroup_common + +test.scenarios('vlt') + +coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_param_bins.v b/test_regress/t/t_covergroup_param_bins.v new file mode 100644 index 000000000..ab57a4a62 --- /dev/null +++ b/test_regress/t/t_covergroup_param_bins.v @@ -0,0 +1,76 @@ +// DESCRIPTION: Verilator: Verilog Test module - parameter/localparam refs in bins +// This file ONLY is placed into the Public Domain, for any use, without warranty. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +// Test: Referencing a parameter/localparam identifier inside a covergroup +// `bins` value or range. This previously triggered an internal error +// (V3WidthCommit.cpp: No dtype) because the folded constant had no dtype. +// Expected: compiles and the bounds behave exactly like numeric literals. + +// verilog_format: off +`define stop $stop +`define checkr(gotv,expv) do if ((gotv) != (expv)) begin $write("%%Error: %s:%0d: got=%f exp=%f\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); +// verilog_format: on + +module t #( + parameter int PMIN = -100 +) ( + input clk +); + + localparam int LMAX = 100; + + int signed value; + + covergroup cg; + cp: coverpoint value { + bins negative = {[PMIN : -1]}; // parameter as range lower bound + bins zero = {0}; + bins positive = {[1 : LMAX]}; // localparam as range upper bound + bins maxv = {LMAX}; // localparam as single value + bins minv = {PMIN}; // parameter as single value + } + endgroup + + cg cg_inst = new; + + int cyc = 0; + + always @(posedge clk) begin + cyc <= cyc + 1; + + case (cyc) + 0: value <= -100; // Hit negative + minv + 1: value <= 0; // Hit zero + 2: value <= 50; // Hit positive + 3: value <= 100; // Hit positive (dup) + maxv + 4: begin + $write("*-* All Finished *-*\n"); + $finish; + end + endcase + + cg_inst.sample(); + + // Coverage progression (NBA committed before sample() within always block). + // A final sample() also runs on the $finish cycle (value still 100), so the + // positive/maxv hit counts are one higher than the number of distinct stimuli. + // cyc=0: value=-100 -> negative + minv -> 2/5=40% + // cyc=1: value=0 -> zero -> 3/5=60% + // cyc=2: value=50 -> positive -> 4/5=80% + // cyc=3: value=100 -> positive(dup) + maxv -> 5/5=100% + if (cyc == 0) begin + `checkr(cg_inst.get_inst_coverage(), 40.0); + end + if (cyc == 1) begin + `checkr(cg_inst.get_inst_coverage(), 60.0); + end + if (cyc == 2) begin + `checkr(cg_inst.get_inst_coverage(), 80.0); + end + if (cyc == 3) begin + `checkr(cg_inst.get_inst_coverage(), 100.0); + end + end +endmodule