From 756bcf574241ffa673ddf9d3e7a4eab9fdb0eab1 Mon Sep 17 00:00:00 2001 From: Matthew Ballance Date: Mon, 22 Jun 2026 19:14:43 -0700 Subject: [PATCH] Fix '$' as unsupported coverpoint-bin range bounds (#7750) (#7825) --- src/V3Covergroup.cpp | 86 +++++++++------- test_regress/t/t_covergroup_array_bins.out | 16 +++ test_regress/t/t_covergroup_array_bins.v | 103 +++++++++++++++++++ test_regress/t/t_covergroup_autobins_bad.out | 90 +++++++++------- test_regress/t/t_covergroup_autobins_bad.v | 11 ++ 5 files changed, 229 insertions(+), 77 deletions(-) diff --git a/src/V3Covergroup.cpp b/src/V3Covergroup.cpp index 0d43e9f60..054f27c34 100644 --- a/src/V3Covergroup.cpp +++ b/src/V3Covergroup.cpp @@ -655,22 +655,54 @@ class FunctionalCoverageVisitor final : public VNVisitor { return refp; } - // Individual equality targets of an array bin (bins b[N] = {values/ranges}), in order. - std::vector extractArrayValues(AstCoverBin* arrayBinp, AstNodeExpr* exprp) { + // Individual equality targets of an array bin (bins b[] = {values/ranges}), in order. + // An open-ended bound ('$', AstUnbounded) resolves to the coverpoint domain: '[lo:$]' + // covers [lo:maxVal] and '[$:hi]' covers [0:hi]. One target is produced per value; a + // range whose resolved size would exceed COVER_BINS_LIMIT (e.g. an open '[lo:$]' over a + // wide coverpoint) is unsupported -- emits COVERIGN, sets unsupportedOut, yields nothing. + std::vector extractArrayValues(AstCoverBin* arrayBinp, AstNodeExpr* exprp, + bool& unsupportedOut) { + unsupportedOut = false; + const int width = exprp->width(); + const uint64_t maxVal = (width >= 64) ? UINT64_MAX : ((1ULL << width) - 1); std::vector values; for (AstNode* rangep = arrayBinp->rangesp(); rangep; rangep = rangep->nextp()) { if (AstInsideRange* const irp = VN_CAST(rangep, InsideRange)) { - AstConst* const minp = VN_CAST(V3Const::constifyEdit(irp->lhsp()), Const); - AstConst* const maxp = VN_CAST(V3Const::constifyEdit(irp->rhsp()), Const); - if (!minp || !maxp) { + AstNodeExpr* const lhsp = V3Const::constifyEdit(irp->lhsp()); + AstNodeExpr* const rhsp = V3Const::constifyEdit(irp->rhsp()); + const bool loUnb = VN_IS(lhsp, Unbounded); + const bool hiUnb = VN_IS(rhsp, Unbounded); + AstConst* const minp = VN_CAST(lhsp, Const); + AstConst* const maxp = VN_CAST(rhsp, Const); + if ((!minp && !loUnb) || (!maxp && !hiUnb)) { arrayBinp->v3error("Non-constant expression in array bins range; " "range bounds must be constants"); return values; } - for (int val = minp->toSInt(); val <= maxp->toSInt(); ++val) - values.push_back(new AstConst{irp->fileline(), AstConst::WidthedValue{}, - static_cast(exprp->width()), - static_cast(val)}); + if ((minp && minp->num().isFourState()) || (maxp && maxp->num().isFourState())) { + arrayBinp->v3error("Four-state (x/z) value in array bins range bound; " + "range bounds must be two-state constants"); + return values; + } + const uint64_t lo = loUnb ? 0 : minp->toUQuad(); + const uint64_t hi = hiUnb ? maxVal : maxp->toUQuad(); + if (hi < lo) continue; // empty range contributes no bins + // Guard against a '$'-bounded or otherwise huge range exploding the bin count. + const uint64_t span = hi - lo; // == valueCount - 1 (no overflow: hi >= lo) + if (span >= static_cast(COVER_BINS_LIMIT) + || values.size() + span + 1 > static_cast(COVER_BINS_LIMIT)) { + arrayBinp->v3warn(COVERIGN, "Unsupported: array 'bins' covering more than " + << COVER_BINS_LIMIT + << " values (e.g. an open '[lo:$]' range over " + "a wide coverpoint); bin ignored"); + unsupportedOut = true; + for (AstNodeExpr* const vp : values) VL_DO_DANGLING(pushDeletep(vp), vp); + values.clear(); + return values; + } + for (uint64_t v = lo; v <= hi; ++v) + values.push_back(new AstConst{irp->fileline(), AstConst::WidthedValue{}, width, + static_cast(v)}); } else { values.push_back(VN_AS(rangep->cloneTree(false), NodeExpr)); } @@ -744,7 +776,9 @@ class FunctionalCoverageVisitor final : public VNVisitor { continue; } if (cbinp->isArray()) { // value array: bins b[N] = {...} -> b[0]..b[N-1] - std::vector values = extractArrayValues(cbinp, exprp); + bool unsupported = false; + std::vector values = extractArrayValues(cbinp, exprp, unsupported); + if (unsupported) continue; // bin ignored (COVERIGN emitted); reserve no slot namerStmts.push_back(makeNamer(cpVarp, cbinp, static_cast(values.size()))); for (AstNodeExpr* valuep : values) { emitConvHitIf(coverpointp, cbinp, cpVarp, idx++, @@ -1056,34 +1090,10 @@ class FunctionalCoverageVisitor final : public VNVisitor { int atLeastValue) { UINFO(4, " Generating array bins for: " << arrayBinp->name()); - // Extract all values from the range list - std::vector values; - for (AstNode* rangep = arrayBinp->rangesp(); rangep; rangep = rangep->nextp()) { - if (AstInsideRange* const insideRangep = VN_CAST(rangep, InsideRange)) { - // For InsideRange [min:max], create bins for each value - AstNodeExpr* const minp = V3Const::constifyEdit(insideRangep->lhsp()); - AstNodeExpr* const maxp = V3Const::constifyEdit(insideRangep->rhsp()); - AstConst* const minConstp = VN_CAST(minp, Const); - AstConst* const maxConstp = VN_CAST(maxp, Const); - if (minConstp && maxConstp) { - const int minVal = minConstp->toSInt(); - const int maxVal = maxConstp->toSInt(); - UINFO(6, " Expanding InsideRange [" << minVal << ":" << maxVal << "]"); - for (int val = minVal; val <= maxVal; ++val) { - values.push_back(new AstConst{insideRangep->fileline(), - AstConst::WidthedValue{}, - (int)exprp->width(), (uint32_t)val}); - } - } else { - arrayBinp->v3error("Non-constant expression in array bins range; " - "range bounds must be constants"); - return; - } - } else { - // Single value - should be an expression - values.push_back(VN_AS(rangep->cloneTree(false), NodeExpr)); - } - } + // Extract all values from the range list (resolves '$', caps/ignores huge ranges) + bool unsupported = false; + std::vector values = extractArrayValues(arrayBinp, exprp, unsupported); + if (unsupported) return; // bin ignored (COVERIGN emitted) // Create a separate bin for each value int index = 0; diff --git a/test_regress/t/t_covergroup_array_bins.out b/test_regress/t/t_covergroup_array_bins.out index ccbbbc297..903c2b307 100644 --- a/test_regress/t/t_covergroup_array_bins.out +++ b/test_regress/t/t_covergroup_array_bins.out @@ -10,3 +10,19 @@ cg3.cp.range_sized[0]: 1 cg3.cp.range_sized[1]: 1 cg3.cp.range_sized[2]: 1 cg3.cp.range_sized[3]: 0 +cg4.cp.all_vals[0]: 1 +cg4.cp.all_vals[1]: 1 +cg4.cp.all_vals[2]: 1 +cg4.cp.all_vals[3]: 0 +cg5.cp.hi_vals[0]: 1 +cg5.cp.hi_vals[1]: 0 +cg6.cp.lo_open[0]: 1 +cg6.cp.lo_open[1]: 0 +cg7.cp.rev[0]: 1 +cg7.cp.rev[1]: 0 +cg8.cp.w[0]: 0 +cg8.cp.w[1]: 1 +cg9.__cross7.cumulative_x_lo [cross]: 1 +cg9.__cross7.ok_x_lo [cross]: 1 +cg9.cpA.ok: 1 +cg9.cpB.lo: 1 diff --git a/test_regress/t/t_covergroup_array_bins.v b/test_regress/t/t_covergroup_array_bins.v index 84033319d..209e1b549 100644 --- a/test_regress/t/t_covergroup_array_bins.v +++ b/test_regress/t/t_covergroup_array_bins.v @@ -13,6 +13,8 @@ module t; bit [7:0] data; + bit [1:0] sel; + bit [63:0] wide; covergroup cg; coverpoint data { @@ -38,14 +40,79 @@ module t; } endgroup + // cg4: array bins with '$' (open range) - '$' resolves to the coverpoint domain max. + // For 2-bit sel, {[0:$]} == {[0:3]}: one bin per value -> 4 bins (issue #7750). + covergroup cg4; + cp: coverpoint sel { + bins all_vals[] = {[0 : $]}; + } + endgroup + + // cg5: lower-open range {[lo:$]} == {[lo:maxVal]} -> bins for 2 and 3 + covergroup cg5; + cp: coverpoint sel { + bins hi_vals[] = {[2 : $]}; + } + endgroup + + // cg6: upper-open range {[$:hi]} == {[0:hi]} -> bins for 0 and 1 + covergroup cg6; + cp: coverpoint sel { + bins lo_open[] = {[$ : 1]}; + } + endgroup + + // cg7: a reversed range {[hi:lo]} (hi 2 bins total. + covergroup cg7; + cp: coverpoint data { + bins rev[] = {[3 : 1], 5, 7}; + } + endgroup + + // cg8: wide (>= 64-bit) coverpoint, exercising the 64-bit domain-max path + covergroup cg8; + cp: coverpoint wide { + bins w[] = {[0 : 1]}; + } + endgroup + + // cg9: two ranges that are each under COVER_BINS_LIMIT (1000) but whose + // cumulative size exceeds it. The first range populates the value list, the + // second trips the running-total guard -> COVERIGN, the whole bin is ignored. + // cpA is crossed, so it is non-convertible and routes through the legacy + // per-bin generateArrayBins() path (exercising its unsupported-bin guard). + covergroup cg9; + cpA: coverpoint wide { + bins cumulative[] = {[0 : 500], [0 : 500]}; + bins ok = {5}; + } + cpB: coverpoint sel { + bins lo = {1}; + } + cross cpA, cpB; + endgroup + initial begin cg cg_inst; cg2 cg2_inst; cg3 cg3_inst; + cg4 cg4_inst; + cg5 cg5_inst; + cg6 cg6_inst; + cg7 cg7_inst; + cg8 cg8_inst; + cg9 cg9_inst; cg_inst = new(); cg2_inst = new(); cg3_inst = new(); + cg4_inst = new(); + cg5_inst = new(); + cg6_inst = new(); + cg7_inst = new(); + cg8_inst = new(); + cg9_inst = new(); // Hit first array bin value (1) data = 1; @@ -94,6 +161,42 @@ module t; cg3_inst.sample(); `checkr(cg3_inst.get_inst_coverage(), 75.0); + // Hit cg4 '$' bins ([0:$] == [0:3], 4 bins): cover 3 of 4 + sel = 0; + cg4_inst.sample(); + `checkr(cg4_inst.get_inst_coverage(), 25.0); + sel = 1; + cg4_inst.sample(); + `checkr(cg4_inst.get_inst_coverage(), 50.0); + sel = 2; + cg4_inst.sample(); + `checkr(cg4_inst.get_inst_coverage(), 75.0); + + // Hit cg5 lower-open bins ([2:$] == [2:3], 2 bins): cover 1 of 2 + sel = 2; + cg5_inst.sample(); + `checkr(cg5_inst.get_inst_coverage(), 50.0); + + // Hit cg6 upper-open bins ([$:1] == [0:1], 2 bins): cover 1 of 2 + sel = 0; + cg6_inst.sample(); + `checkr(cg6_inst.get_inst_coverage(), 50.0); + + // Hit cg7 bins (reversed [3:1] -> no bins; 5 and 7 -> 2 bins): cover 1 of 2 + data = 5; + cg7_inst.sample(); + `checkr(cg7_inst.get_inst_coverage(), 50.0); + + // Hit cg8 wide bins ([0:1], 2 bins): cover 1 of 2 + wide = 1; + cg8_inst.sample(); + `checkr(cg8_inst.get_inst_coverage(), 50.0); + + // Exercise cg9 (crossed cpA with an ignored cumulative array bin, legacy path) + wide = 5; + sel = 1; + cg9_inst.sample(); + $write("*-* All Finished *-*\n"); $finish; end diff --git a/test_regress/t/t_covergroup_autobins_bad.out b/test_regress/t/t_covergroup_autobins_bad.out index bb8c92918..2e5bb49e5 100644 --- a/test_regress/t/t_covergroup_autobins_bad.out +++ b/test_regress/t/t_covergroup_autobins_bad.out @@ -1,76 +1,88 @@ -%Error: t/t_covergroup_autobins_bad.v:17:12: Automatic bins array size must be a constant +%Error: t/t_covergroup_autobins_bad.v:18:12: Automatic bins array size must be a constant : ... note: In instance 't' - 17 | bins auto[size_var]; + 18 | bins auto[size_var]; | ^~~~ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. -%Error: t/t_covergroup_autobins_bad.v:24:12: Automatic bins array size must be >= 1, got 0 +%Error: t/t_covergroup_autobins_bad.v:25:12: Automatic bins array size must be >= 1, got 0 : ... note: In instance 't' - 24 | bins auto[0]; + 25 | bins auto[0]; | ^~~~ -%Error: t/t_covergroup_autobins_bad.v:31:12: Automatic bins array size of 1001 exceeds limit of 1000 +%Error: t/t_covergroup_autobins_bad.v:32:12: Automatic bins array size of 1001 exceeds limit of 1000 : ... note: In instance 't' - 31 | bins auto[1001]; + 32 | bins auto[1001]; | ^~~~ -%Error: t/t_covergroup_autobins_bad.v:43:26: Non-constant expression in bin value list; values must be constants +%Error: t/t_covergroup_autobins_bad.v:44:26: Non-constant expression in bin value list; values must be constants : ... note: In instance 't' - 43 | ignore_bins ign = {size_var}; + 44 | ignore_bins ign = {size_var}; | ^~~~~~~~ -%Error: t/t_covergroup_autobins_bad.v:44:32: Non-constant expression in bin range; range bounds must be constants +%Error: t/t_covergroup_autobins_bad.v:45:32: Non-constant expression in bin range; range bounds must be constants : ... note: In instance 't' - 44 | ignore_bins ign_range = {[0:size_var]}; + 45 | ignore_bins ign_range = {[0:size_var]}; | ^ -%Error: t/t_covergroup_autobins_bad.v:38:12: Non-constant expression in array bins range; range bounds must be constants - : ... note: In instance 't' - 38 | bins b[] = {[size_var:size_var]}; - | ^ %Error: t/t_covergroup_autobins_bad.v:39:12: Non-constant expression in array bins range; range bounds must be constants : ... note: In instance 't' - 39 | bins b_mixed[] = {[0:size_var]}; + 39 | bins b[] = {[size_var:size_var]}; + | ^ +%Error: t/t_covergroup_autobins_bad.v:40:12: Non-constant expression in array bins range; range bounds must be constants + : ... note: In instance 't' + 40 | bins b_mixed[] = {[0:size_var]}; | ^~~~~~~ -%Error: t/t_covergroup_autobins_bad.v:40:23: Non-constant expression in bin range; range bounds must be constants +%Error: t/t_covergroup_autobins_bad.v:41:23: Non-constant expression in bin range; range bounds must be constants : ... note: In instance 't' - 40 | bins b_range = {[size_var:4]}; + 41 | bins b_range = {[size_var:4]}; | ^ -%Error: t/t_covergroup_autobins_bad.v:41:24: Non-constant expression in bin range; range bounds must be constants +%Error: t/t_covergroup_autobins_bad.v:42:24: Non-constant expression in bin range; range bounds must be constants : ... note: In instance 't' - 41 | bins b_range2 = {[0:size_var]}; + 42 | bins b_range2 = {[0:size_var]}; | ^ -%Error: t/t_covergroup_autobins_bad.v:42:18: Non-constant expression in bin range; values must be constants +%Error: t/t_covergroup_autobins_bad.v:43:18: Non-constant expression in bin range; values must be constants : ... note: In instance 't' - 42 | bins b2 = {size_var}; + 43 | bins b2 = {size_var}; | ^~~~~~~~ -%Error: t/t_covergroup_autobins_bad.v:43:26: Non-constant expression in bin range; values must be constants +%Error: t/t_covergroup_autobins_bad.v:44:26: Non-constant expression in bin range; values must be constants : ... note: In instance 't' - 43 | ignore_bins ign = {size_var}; + 44 | ignore_bins ign = {size_var}; | ^~~~~~~~ -%Warning-COVERIGN: t/t_covergroup_autobins_bad.v:51:25: Ignoring unsupported: non-constant 'option.at_least'; using default value +%Warning-COVERIGN: t/t_covergroup_autobins_bad.v:52:25: Ignoring unsupported: non-constant 'option.at_least'; using default value : ... note: In instance 't' - 51 | option.at_least = size_var; + 52 | option.at_least = size_var; | ^~~~~~~~ ... 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. -%Error: t/t_covergroup_autobins_bad.v:61:31: Non-constant expression in bin range; range bounds must be constants +%Error: t/t_covergroup_autobins_bad.v:62:31: Non-constant expression in bin range; range bounds must be constants : ... note: In instance 't' - 61 | ignore_bins ign_nclo = {[size_var:4]}; + 62 | ignore_bins ign_nclo = {[size_var:4]}; | ^ -%Error: t/t_covergroup_autobins_bad.v:58:20: Four-state (x/z) value in bin range bound; range bounds must be two-state constants +%Error: t/t_covergroup_autobins_bad.v:59:20: Four-state (x/z) value in bin range bound; range bounds must be two-state constants : ... note: In instance 't' - 58 | bins b_xz = {[4'bxxxx:4'hF]}; + 59 | bins b_xz = {[4'bxxxx:4'hF]}; | ^ -%Error: t/t_covergroup_autobins_bad.v:59:32: Four-state (x/z) value in bin range bound; range bounds must be two-state constants - : ... note: In instance 't' - 59 | ignore_bins ign_xz_lo = {[4'bxxxx:4'hF]}; - | ^ %Error: t/t_covergroup_autobins_bad.v:60:32: Four-state (x/z) value in bin range bound; range bounds must be two-state constants : ... note: In instance 't' - 60 | ignore_bins ign_xz_hi = {[4'h0:4'bzzzz]}; + 60 | ignore_bins ign_xz_lo = {[4'bxxxx:4'hF]}; | ^ -%Error: t/t_covergroup_autobins_bad.v:62:23: Non-constant expression in bin range; range bounds must be constants +%Error: t/t_covergroup_autobins_bad.v:61:32: Four-state (x/z) value in bin range bound; range bounds must be two-state constants : ... note: In instance 't' - 62 | bins b_nc_ub = {[size_var:$]}; - | ^ -%Error: t/t_covergroup_autobins_bad.v:63:23: Four-state (x/z) value in bin range bound; range bounds must be two-state constants + 61 | ignore_bins ign_xz_hi = {[4'h0:4'bzzzz]}; + | ^ +%Error: t/t_covergroup_autobins_bad.v:63:23: Non-constant expression in bin range; range bounds must be constants : ... note: In instance 't' - 63 | bins b_xz_ub = {[4'bxxxx:$]}; + 63 | bins b_nc_ub = {[size_var:$]}; | ^ +%Error: t/t_covergroup_autobins_bad.v:64:23: Four-state (x/z) value in bin range bound; range bounds must be two-state constants + : ... note: In instance 't' + 64 | bins b_xz_ub = {[4'bxxxx:$]}; + | ^ +%Error: t/t_covergroup_autobins_bad.v:65:12: Four-state (x/z) value in array bins range bound; range bounds must be two-state constants + : ... note: In instance 't' + 65 | bins b_xz_arr[] = {[4'bxxxx:4'hF]}; + | ^~~~~~~~ +%Error: t/t_covergroup_autobins_bad.v:66:12: Four-state (x/z) value in array bins range bound; range bounds must be two-state constants + : ... note: In instance 't' + 66 | bins b_xz_arr_hi[] = {[4'h0:4'bzzzz]}; + | ^~~~~~~~~~~ +%Warning-COVERIGN: t/t_covergroup_autobins_bad.v:73:12: Unsupported: array 'bins' covering more than 1000 values (e.g. an open '[lo:$]' range over a wide coverpoint); bin ignored + : ... note: In instance 't' + 73 | bins b_huge[] = {[0:$]}; + | ^~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_covergroup_autobins_bad.v b/test_regress/t/t_covergroup_autobins_bad.v index 8a19c0c8e..db8a48fd5 100644 --- a/test_regress/t/t_covergroup_autobins_bad.v +++ b/test_regress/t/t_covergroup_autobins_bad.v @@ -10,6 +10,7 @@ module t; int size_var; logic [3:0] cp_expr; + logic [15:0] cp_wide; // Error: array size must be a constant covergroup cg1; @@ -61,6 +62,15 @@ module t; ignore_bins ign_nclo = {[size_var:4]}; // non-constant lower bound bins b_nc_ub = {[size_var:$]}; // non-constant lower bound, open-ended '$' upper bins b_xz_ub = {[4'bxxxx:$]}; // four-state lower bound, open-ended '$' upper + bins b_xz_arr[] = {[4'bxxxx:4'hF]}; // four-state lower bound (array-bins path) + bins b_xz_arr_hi[] = {[4'h0:4'bzzzz]}; // four-state upper bound (array-bins path) + } + endgroup + + // Warning (COVERIGN): array bins range exceeds COVER_BINS_LIMIT + covergroup cg6; + cp1: coverpoint cp_wide { + bins b_huge[] = {[0:$]}; // open '[lo:$]' over 16-bit coverpoint exceeds bin limit } endgroup @@ -70,6 +80,7 @@ module t; cg3 cg3_inst = new; cg4 cg4_inst = new; cg5 cg5_inst = new; + cg6 cg6_inst = new; initial $finish; endmodule