Incorporate feedback comments

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>
This commit is contained in:
Matthew Ballance 2026-06-09 00:40:21 +00:00
parent 9063aa6fff
commit b014161359
5 changed files with 108 additions and 73 deletions

View File

@ -14,14 +14,9 @@
/// \file
/// \brief Verilated functional-coverage model interfaces
///
/// Read-side reflection seam: abstract interfaces a coverage writer queries,
/// implemented by the collection runtime (VlCoverpoint, and later VlCoverCross
/// and the covergroup type/instance nodes). Carries no implementation and
/// depends on nothing in verilated_cov.h, so a writer or a second back-end can
/// compile against it independently.
///
/// Step 1 declares only the slice the runtime implements today; it grows a
/// slice per phase as consumers (writer, cross) arrive.
/// Defines interface classes to runtime covergroup coverage-collection classes.
/// These are used to query coverage achievement at runtime, and (future)
/// when writing coverage to the coverage database.
///
//=============================================================================
@ -35,22 +30,29 @@
// Per-bin classification. A bin's kind is which set it lives in (structural),
// not a per-bin field. Only Normal feeds coverage(); the rest are recorded.
enum class VlCovBinKind : uint8_t { Normal = 0, Default = 1, Ignore = 2, Illegal = 3 };
enum class VlCovBinKind : uint8_t {
NORMAL = 0, // Base coverage-collecting bin
DEFAULT = 1, // Bin declared with 'default' range (which is excluded per LRM)
IGNORE = 2, // Ignore bin
ILLEGAL = 3 // Illegal bin
};
//=============================================================================
// VlCoverpointIf
/// Read-side view of a coverpoint. The writer queries bins by index; the
/// implementor computes names/kinds on demand. Bounded bin count, so random
/// access by index is the primary API.
/// access by index is the primary usage.
class VlCoverpointIf VL_NOT_FINAL {
public:
// CONSTRUCTORS
virtual ~VlCoverpointIf() = default;
// METHODS
// All bins, across every set; index range [0, binCount())
virtual int binCount() const = 0;
// Bin name, computed into the caller's reusable buffer (no per-bin alloc after
// warmup); returns buf.c_str()
virtual const char* binName(int i, std::string& buf) const = 0;
// Bin name in declaration order (e.g. "myBin" or "b[3]")
virtual std::string binName(int i) const = 0;
virtual VlCovBinKind binKind(int i) const = 0;
// Bins covered / effective total (Normal set only) for the coverage calc
virtual void coverageParts(double& covered, double& total) const = 0;

View File

@ -33,47 +33,46 @@ void VlCoverpoint::init(const char* hier, uint32_t atLeast, int nBins) {
void VlCoverpoint::addNamer(VlCovBinKind set, int count, VlCovBinNaming naming, const char* name,
const char* file, int line, int col) {
m_namers.push_back(VlCovNamer{set, count, m_nextBase, naming, name, file, line, col});
m_namers.emplace_back(set, count, m_nextBase, naming, name, file, line, col);
m_nextBase += count;
if (set == VlCovBinKind::Normal) m_normal += count;
if (set == VlCovBinKind::NORMAL) m_normal += count;
}
const VlCovNamer& VlCoverpoint::namerFor(int i) const {
// Namers are appended in ascending, contiguous index order covering [0, m_total),
// and i is always a valid bin index, so the matching namer always exists.
for (const VlCovNamer& nm : m_namers) {
if (i < nm.base + nm.count) return nm;
if (i < nm.base() + nm.count()) return nm;
}
VL_UNREACHABLE;
}
const char* VlCoverpoint::binName(int i, std::string& buf) const {
std::string VlCoverpoint::binName(int i) const {
const VlCovNamer& nm = namerFor(i);
buf = nm.name;
if (nm.naming == VlCovBinNaming::Array) buf += '[' + std::to_string(i - nm.base) + ']';
return buf.c_str();
std::string name = nm.name();
if (nm.naming() == VlCovBinNaming::Array) name += '[' + std::to_string(i - nm.base()) + ']';
return name;
}
void VlCoverpoint::registerBins(VerilatedCovContext* covcontextp, const char* page) {
std::string buf;
for (int i = 0; i < binCount(); ++i) {
const VlCovNamer& nm = namerFor(i);
const VlCovBinKind kind = binKind(i);
const char* const binp = binName(i, buf);
const std::string binp = binName(i);
const std::string full = m_hier + "." + binp;
const std::string lineStr = std::to_string(nm.line);
const std::string colStr = std::to_string(nm.col);
if (kind == VlCovBinKind::Normal) {
const std::string lineStr = std::to_string(nm.line());
const std::string colStr = std::to_string(nm.col());
if (kind == VlCovBinKind::NORMAL) {
VL_COVER_INSERT(covcontextp, full.c_str(), &m_counts[i], "page", page, "filename",
nm.file, "lineno", lineStr.c_str(), "column", colStr.c_str(), "bin",
binp);
nm.file(), "lineno", lineStr.c_str(), "column", colStr.c_str(), "bin",
binp.c_str());
} else {
const char* const binType = kind == VlCovBinKind::Ignore ? "ignore"
: kind == VlCovBinKind::Illegal ? "illegal"
const char* const binType = kind == VlCovBinKind::IGNORE ? "ignore"
: kind == VlCovBinKind::ILLEGAL ? "illegal"
: "default";
VL_COVER_INSERT(covcontextp, full.c_str(), &m_counts[i], "page", page, "filename",
nm.file, "lineno", lineStr.c_str(), "column", colStr.c_str(), "bin",
binp, "bin_type", binType);
nm.file(), "lineno", lineStr.c_str(), "column", colStr.c_str(), "bin",
binp.c_str(), "bin_type", binType);
}
}
}

View File

@ -43,42 +43,70 @@ enum class VlCovBinNaming : uint8_t {
Array, // "<name>[i]" bins b[N] value array
};
// Names a contiguous run of bins; carries the run's own source location.
// Specifies the naming scheme for a range of bins, allowing the
// specific name to be computed on-demand.
// All name strings are borrowed literals from the generated code.
struct VlCovNamer final {
VlCovBinKind set; // which set the bins belong to
int count; // bins this namer covers (1 for Single)
int base; // first bin index (declaration order), assigned on append
VlCovBinNaming naming;
const char* name; // bin name (Single) or array base name (Array)
const char* file;
int line;
int col;
class VlCovNamer final {
// MEMBERS
VlCovBinKind m_set; // which set the bins belong to
int m_count; // bins this namer covers (1 for Single)
int m_base; // first bin index (declaration order), assigned on append
VlCovBinNaming m_naming; // how bin names are built
const char* m_name; // bin name (Single) or array base name (Array)
const char* m_file; // declaration file
int m_line; // declaration line
int m_col; // declaration column
public:
// CONSTRUCTORS
VlCovNamer(VlCovBinKind set, int count, int base, VlCovBinNaming naming, const char* name,
const char* file, int line, int col)
: m_set{set}
, m_count{count}
, m_base{base}
, m_naming{naming}
, m_name{name}
, m_file{file}
, m_line{line}
, m_col{col} {}
// METHODS
VlCovBinKind set() const { return m_set; }
int count() const { return m_count; }
int base() const { return m_base; }
VlCovBinNaming naming() const { return m_naming; }
const char* name() const { return m_name; }
const char* file() const { return m_file; }
int line() const { return m_line; }
int col() const { return m_col; }
};
//=============================================================================
// VlCoverpoint
/// Per-instance coverpoint runtime. Bins are stored in declaration order; a
/// bin's set/name come from the owning namer. coverage() is O(1) via the
/// incrementally-maintained covered count.
/// bin's set/name come from the owning namer. coverage() is computed on demand
/// by scanning bin counts, keeping the sample() hot path a plain counter bump.
class VlCoverpoint final : public VlCoverpointIf {
// MEMBERS
std::string m_hier; // "covergroup.coverpoint"
uint32_t m_atLeast = 1; // option.at_least (coverpoint-wide)
int m_total = 0; // bins across all sets
int m_normal = 0; // Normal bins (coverage denominator)
int m_numCovered = 0; // Normal bins with count >= m_atLeast
int m_nextBase = 0; // running append cursor
std::vector<uint32_t> m_counts; // [m_total], one per bin
std::vector<VlCovNamer> m_namers; // appended in declaration order
const VlCovNamer& namerFor(int i) const; // linear scan (namer count is tiny)
// PRIVATE METHODS
const VlCovNamer& namerFor(int i) const; // obtain the bin-specific name producer
void addNamer(VlCovBinKind set, int count, VlCovBinNaming naming, const char* name,
const char* file, int line, int col);
public:
// CONSTRUCTORS
VlCoverpoint() = default;
// METHODS
// ---- configuration (from generated constructor) ----
void init(const char* hier, uint32_t atLeast, int nBins);
void addSingleNamer(VlCovBinKind set, const char* name, const char* file, int line, int col) {
@ -91,17 +119,24 @@ public:
void registerBins(VerilatedCovContext* covcontextp, const char* page);
// ---- hot path (from generated sample()) ----
void incrementBin(int i) { // Normal bin: maintains covered count
if (m_counts[i]++ == m_atLeast - 1) ++m_numCovered;
}
void incrementBin(int i) { ++m_counts[i]; } // Normal bin: count only
void recordHit(int i) { ++m_counts[i]; } // Ignore/Illegal/Default: count only
// ---- VlCoverpointIf ----
int binCount() const override { return m_total; }
const char* binName(int i, std::string& buf) const override;
VlCovBinKind binKind(int i) const override { return namerFor(i).set; }
std::string binName(int i) const override;
VlCovBinKind binKind(int i) const override { return namerFor(i).set(); }
void coverageParts(double& covered, double& total) const override {
covered = m_numCovered;
// Count Normal bins that reached option.at_least on demand, so the hot
// path (incrementBin) stays a plain counter bump.
int numCovered = 0;
for (const VlCovNamer& nm : m_namers) {
if (nm.set() != VlCovBinKind::NORMAL) continue;
for (int i = nm.base(); i < nm.base() + nm.count(); ++i) {
if (m_counts[i] >= m_atLeast) ++numCovered;
}
}
covered = numCovered;
total = m_normal;
}
};

View File

@ -1191,6 +1191,19 @@ public:
= {"user", "array", "auto", "ignore", "illegal", "default", "wildcard", "transition"};
return names[m_e];
}
// VlCovBinKind enumerator naming the bin's set
const char* binSetEnum() const {
switch (m_e) {
case BINS_IGNORE: return "VlCovBinKind::IGNORE";
case BINS_ILLEGAL: return "VlCovBinKind::ILLEGAL";
case BINS_DEFAULT: return "VlCovBinKind::DEFAULT";
default: return "VlCovBinKind::NORMAL";
}
}
// Normal bins (feed coverage) are anything but ignore/illegal/default
bool binIsNormal() const {
return m_e != BINS_IGNORE && m_e != BINS_ILLEGAL && m_e != BINS_DEFAULT;
}
};
constexpr bool operator==(const VCoverBinsType& lhs, VCoverBinsType::en rhs) {
return lhs.m_e == rhs;

View File

@ -655,22 +655,6 @@ class FunctionalCoverageVisitor final : public VNVisitor {
return refp;
}
// VlCovBinKind enumerator naming the bin's set
static const char* binSetEnum(AstCoverBin* binp) {
switch (binp->binsType()) {
case VCoverBinsType::BINS_IGNORE: return "VlCovBinKind::Ignore";
case VCoverBinsType::BINS_ILLEGAL: return "VlCovBinKind::Illegal";
case VCoverBinsType::BINS_DEFAULT: return "VlCovBinKind::Default";
default: return "VlCovBinKind::Normal";
}
}
// Normal bins (feed coverage) are anything but ignore/illegal/default
static bool binIsNormal(AstCoverBin* binp) {
const VCoverBinsType btype = binp->binsType();
return btype != VCoverBinsType::BINS_IGNORE && btype != VCoverBinsType::BINS_ILLEGAL
&& btype != VCoverBinsType::BINS_DEFAULT;
}
// Individual equality targets of an array bin (bins b[N] = {values/ranges}), in order.
std::vector<AstNodeExpr*> extractArrayValues(AstCoverBin* arrayBinp, AstNodeExpr* exprp) {
std::vector<AstNodeExpr*> values;
@ -703,10 +687,11 @@ class FunctionalCoverageVisitor final : public VNVisitor {
+ std::to_string(fl->lineno()) + ", "
+ std::to_string(fl->firstColumn()) + ");";
if (count < 0) { // single bin
cs->add(".addSingleNamer(" + std::string{binSetEnum(binp)} + ", \"" + binp->name()
cs->add(".addSingleNamer(" + std::string{binp->binsType().binSetEnum()} + ", \""
+ binp->name()
+ "\", " + loc);
} else { // value array bin
cs->add(".addArrayNamer(" + std::string{binSetEnum(binp)} + ", "
cs->add(".addArrayNamer(" + std::string{binp->binsType().binSetEnum()} + ", "
+ std::to_string(count) + ", \"" + binp->name() + "\", " + loc);
}
return cs;
@ -718,7 +703,8 @@ class FunctionalCoverageVisitor final : public VNVisitor {
FileLine* const fl = binp->fileline();
AstCStmt* const hitp = new AstCStmt{fl};
hitp->add(memberRef(fl, cpVarp));
hitp->add((binIsNormal(binp) ? ".incrementBin(" : ".recordHit(") + std::to_string(idx)
hitp->add((binp->binsType().binIsNormal() ? ".incrementBin(" : ".recordHit(")
+ std::to_string(idx)
+ ");");
AstNode* actionp = hitp;
if (binp->binsType() == VCoverBinsType::BINS_ILLEGAL) {
@ -1508,7 +1494,7 @@ class FunctionalCoverageVisitor final : public VNVisitor {
}
int legacyRegular = 0;
for (const BinInfo& bi : m_binInfos) {
if (!binIsNormal(bi.binp)) continue;
if (!bi.binp->binsType().binIsNormal()) continue;
++legacyRegular;
AstCStmt* const cs = new AstCStmt{fl};
cs->add("if (");
@ -1532,7 +1518,7 @@ class FunctionalCoverageVisitor final : public VNVisitor {
int totalBins = 0;
for (const BinInfo& bi : m_binInfos) {
UINFO(6, " Bin: " << bi.binp->name() << " type=" << bi.binp->binsType().ascii());
if (binIsNormal(bi.binp)) totalBins++;
if (bi.binp->binsType().binIsNormal()) totalBins++;
}
UINFO(4, " Total regular bins: " << totalBins << " of " << m_binInfos.size());
@ -1560,7 +1546,7 @@ class FunctionalCoverageVisitor final : public VNVisitor {
// For each regular bin, if count > 0, increment covered_count
for (const BinInfo& bi : m_binInfos) {
// Skip ignore/illegal/default bins in coverage calculation
if (!binIsNormal(bi.binp)) continue;
if (!bi.binp->binsType().binIsNormal()) continue;
// if (bin_count >= at_least) covered_count++;
AstIf* ifp = new AstIf{