Comment new Ast classes, clean up accidentally added members, clean up a few interim workarounds, and adjust approach to stay out of sensitive paths

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>
This commit is contained in:
Matthew Ballance 2026-03-18 01:44:17 +00:00
parent fe26b9212e
commit 94a65f8e64
14 changed files with 70 additions and 116 deletions

View File

@ -724,8 +724,10 @@ class CovergroupSamplingVisitor final : public VNVisitor {
AstActive* const activep = m_namer.getActive(fl, senTreep);
VL_DO_DANGLING(senTreep->deleteTree(), senTreep);
// Add the CMethodCall statement to the active domain
activep->addStmtsp(cmethodCallp->makeStmt());
// Wrap the sample() call in an AstAlways so SchedPartition handles it
// via visit(AstNodeProcedure*) like any other clocked always block.
activep->addStmtsp(
new AstAlways{fl, VAlwaysKwd::ALWAYS_FF, nullptr, cmethodCallp->makeStmt()});
UINFO(4, " Added automatic sample() call for covergroup " << varp->name() << endl);
}

View File

@ -1110,18 +1110,18 @@ 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
BINS_USER, // Single bin with one or more values/ranges
BINS_ARRAY, // Array of bins with user-speciifed size
BINS_AUTO, // Auto-sized array of bins (eg auto_bin_max)
BINS_IGNORE, // Ignore bin
BINS_ILLEGAL, // Illegal bin
BINS_DEFAULT, // Default bin
BINS_WILDCARD, // Wildcard bin
BINS_TRANSITION // Transition bin
};
enum en m_e;
VCoverBinsType()
: m_e{USER} {}
: m_e{BINS_USER} {}
// cppcheck-suppress noExplicitConstructor
constexpr VCoverBinsType(en _e)
: m_e{_e} {}

View File

@ -1039,18 +1039,15 @@ public:
bool sameNode(const AstNode* /*samep*/) const override { return true; }
};
// Forward declarations for types used in constructors below
class AstCoverTransSet;
class AstCoverSelectExpr;
class AstCoverBin final : public AstNode {
// Captures data for a coverpoint 'bins' declaration
// @astgen op1 := rangesp : List[AstNode]
// @astgen op2 := iffp : Optional[AstNodeExpr]
// @astgen op3 := arraySizep : Optional[AstNodeExpr]
// @astgen op4 := transp : List[AstCoverTransSet]
string m_name;
VCoverBinsType m_type;
bool m_isArray = false;
const string m_name; // Base name of the bin
const VCoverBinsType m_type; // Bin type (eg AUTO, IGNORE, ILLEGAL)
bool m_isArray = false; // Bin is either an auto-sized array of values or transitions
public:
AstCoverBin(FileLine* fl, const string& name, AstNode* rangesp, bool isIgnore, bool isIllegal,
@ -1060,14 +1057,14 @@ public:
, m_type{isWildcard ? VCoverBinsType::BINS_WILDCARD
: (isIllegal ? VCoverBinsType::BINS_ILLEGAL
: (isIgnore ? VCoverBinsType::BINS_IGNORE
: VCoverBinsType::USER))} {
: VCoverBinsType::BINS_USER))} {
if (rangesp) addRangesp(rangesp);
}
// Constructor for automatic bins
AstCoverBin(FileLine* fl, const string& name, AstNodeExpr* arraySizep)
: ASTGEN_SUPER_CoverBin(fl)
, m_name{name}
, m_type{VCoverBinsType::AUTO}
, m_type{VCoverBinsType::BINS_AUTO}
, m_isArray{true} {
this->arraySizep(arraySizep);
}
@ -1077,12 +1074,11 @@ public:
, m_name{name}
, m_type{type} {}
// Constructor for transition bins
AstCoverBin(FileLine* fl, const string& name, AstCoverTransSet* transp, bool isIgnore,
bool isIllegal, bool isArrayBin = false)
AstCoverBin(FileLine* fl, const string& name, AstCoverTransSet* transp,
VCoverBinsType type = VCoverBinsType::BINS_TRANSITION, bool isArrayBin = false)
: ASTGEN_SUPER_CoverBin(fl)
, m_name{name}
, m_type{isIllegal ? VCoverBinsType::BINS_ILLEGAL
: (isIgnore ? VCoverBinsType::BINS_IGNORE : VCoverBinsType::TRANSITION)}
, m_type{type}
, m_isArray{isArrayBin} {
if (transp) addTransp(transp);
}
@ -1095,8 +1091,9 @@ public:
void isArray(bool flag) { m_isArray = flag; }
};
class AstCoverCrossBins final : public AstNode {
// Cross-point bin definition
// @astgen op1 := selectp : Optional[AstCoverSelectExpr]
string m_name;
const string m_name; // Bin name
public:
AstCoverCrossBins(FileLine* fl, const string& name, AstCoverSelectExpr* selectp)
@ -1110,8 +1107,9 @@ public:
string name() const override VL_MT_STABLE { return m_name; }
};
class AstCoverOption final : public AstNode {
// Coverage-option assignment
// @astgen op1 := valuep : AstNodeExpr
VCoverOptionType m_type;
const VCoverOptionType m_type; // Option being assigned
public:
AstCoverOption(FileLine* fl, VCoverOptionType type, AstNodeExpr* valuep)
@ -1125,6 +1123,7 @@ public:
VCoverOptionType optionType() const { return m_type; }
};
class AstCoverSelectExpr final : public AstNode {
// Represents a cross-bins selection expression (eg binsof(cp1) intersect {1,2})
// @astgen op1 := exprp : AstNodeExpr
public:
AstCoverSelectExpr(FileLine* fl, AstNodeExpr* exprp)
@ -1140,7 +1139,7 @@ class AstCoverTransItem final : public AstNode {
// @astgen op1 := valuesp : List[AstNode]
// @astgen op2 := repMinp : Optional[AstNodeExpr]
// @astgen op3 := repMaxp : Optional[AstNodeExpr]
VTransRepType m_repType;
const VTransRepType m_repType;
public:
AstCoverTransItem(FileLine* fl, AstNode* valuesp, VTransRepType repType = VTransRepType::NONE)
@ -1166,12 +1165,14 @@ public:
void dumpJson(std::ostream& str) const override;
};
class AstCovergroup final : public AstNode {
// Represents a covergroup declaration. V3LinkParse transforms this
// into an AstClass with isCovergroup==true and attaches the clocking
// event as a new AstCovergroup sentinel in class membersp.
// @astgen op1 := argsp : List[AstVar]
// @astgen op2 := membersp : List[AstNode]
// @astgen op3 := eventp : Optional[AstSenTree]
// @astgen op4 := sampleArgsp : List[AstVar]
string m_name;
bool m_isClass = false;
string m_name; // covergroup name
public:
AstCovergroup(FileLine* fl, const string& name, AstVar* argsp, AstVar* sampleArgsp,
@ -1188,13 +1189,12 @@ public:
void dumpJson(std::ostream& str) const override;
string name() const override VL_MT_STABLE { return m_name; }
void name(const string& name) override { m_name = name; }
bool isClass() const { return m_isClass; }
void isClass(bool flag) { m_isClass = flag; }
bool maybePointedTo() const override { return true; }
};
class AstCoverpointRef final : public AstNode {
// Reference to a coverpoint used in a cross
// @astgen ptr := m_coverpointp : Optional[AstCoverpoint]
string m_name;
const string m_name; // coverpoint name
public:
AstCoverpointRef(FileLine* fl, const string& name)

View File

@ -3499,14 +3499,9 @@ const char* AstNot::widthMismatch() const VL_MT_STABLE {
//######################################################################
// Functional coverage dump methods
void AstCovergroup::dump(std::ostream& str) const {
this->AstNode::dump(str);
str << " " << m_name;
if (m_isClass) str << " [class]";
}
void AstCovergroup::dump(std::ostream& str) const { this->AstNode::dump(str); }
void AstCovergroup::dumpJson(std::ostream& str) const {
dumpJsonBoolFuncIf(str, isClass);
dumpJsonGen(str);
}
@ -3516,13 +3511,12 @@ void AstCoverpoint::dumpJson(std::ostream& str) const { this->AstNodeFuncCovItem
void AstCoverBin::dump(std::ostream& str) const {
this->AstNode::dump(str);
str << " " << m_name << " " << m_type.ascii();
str << " " << 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\": \"" << m_type.ascii() << "\"";
if (m_isArray) str << ", \"isArray\": true";
}
@ -3539,7 +3533,6 @@ void AstCoverTransItem::dumpJson(std::ostream& str) const {
void AstCoverTransSet::dump(std::ostream& str) const {
this->AstNode::dump(str);
str << " trans_set";
}
void AstCoverTransSet::dumpJson(std::ostream& str) const { this->AstNode::dumpJson(str); }
@ -3548,15 +3541,9 @@ void AstCoverCross::dump(std::ostream& str) const { this->AstNodeFuncCovItem::du
void AstCoverCross::dumpJson(std::ostream& str) const { this->AstNodeFuncCovItem::dumpJson(str); }
void AstCoverCrossBins::dump(std::ostream& str) const {
this->AstNode::dump(str);
str << " " << m_name;
}
void AstCoverCrossBins::dump(std::ostream& str) const { this->AstNode::dump(str); }
void AstCoverCrossBins::dumpJson(std::ostream& str) const {
this->AstNode::dumpJson(str);
str << ", \"name\": " << VString::quotePercent(m_name);
}
void AstCoverCrossBins::dumpJson(std::ostream& str) const { this->AstNode::dumpJson(str); }
void AstCoverOption::dump(std::ostream& str) const {
this->AstNode::dump(str);
@ -3568,15 +3555,9 @@ void AstCoverOption::dumpJson(std::ostream& str) const {
str << ", \"optionType\": \"" << m_type.ascii() << "\"";
}
void AstCoverpointRef::dump(std::ostream& str) const {
this->AstNode::dump(str);
str << " " << m_name;
}
void AstCoverpointRef::dump(std::ostream& str) const { this->AstNode::dump(str); }
void AstCoverpointRef::dumpJson(std::ostream& str) const {
this->AstNode::dumpJson(str);
str << ", \"name\": " << VString::quotePercent(m_name);
}
void AstCoverpointRef::dumpJson(std::ostream& str) const { this->AstNode::dumpJson(str); }
void AstCoverSelectExpr::dump(std::ostream& str) const { this->AstNode::dump(str); }

View File

@ -120,7 +120,7 @@ class FunctionalCoverageVisitor final : public VNVisitor {
AstCoverBin* const cbinp = VN_CAST(binp, CoverBin);
AstNode* nextBinp = binp->nextp();
if (cbinp && cbinp->binsType() == VCoverBinsType::AUTO) {
if (cbinp && cbinp->binsType() == VCoverBinsType::BINS_AUTO) {
UINFO(4, " Expanding automatic bin: " << cbinp->name() << endl);
// Get array size - must be a constant
@ -477,14 +477,14 @@ class FunctionalCoverageVisitor final : public VNVisitor {
if (!cbinp) continue;
// Defer default bins to second pass
if (cbinp->binsType() == VCoverBinsType::DEFAULT) {
if (cbinp->binsType() == VCoverBinsType::BINS_DEFAULT) {
defaultBins.push_back(cbinp);
continue;
}
// Handle array bins: create separate bin for each value/transition
if (cbinp->isArray()) {
if (cbinp->binsType() == VCoverBinsType::TRANSITION) {
if (cbinp->binsType() == VCoverBinsType::BINS_TRANSITION) {
hasTransition = true;
generateTransitionArrayBins(coverpointp, cbinp, exprp, atLeastValue);
} else {
@ -520,7 +520,7 @@ class FunctionalCoverageVisitor final : public VNVisitor {
// Generate bin matching code in sample()
// Handle transition bins specially
if (cbinp->binsType() == VCoverBinsType::TRANSITION) {
if (cbinp->binsType() == VCoverBinsType::BINS_TRANSITION) {
hasTransition = true;
generateTransitionBinMatchCode(coverpointp, cbinp, exprp, varp);
} else {
@ -615,7 +615,7 @@ class FunctionalCoverageVisitor final : public VNVisitor {
if (!cbinp) continue;
// Skip default, ignore, and illegal bins
if (cbinp->binsType() == VCoverBinsType::DEFAULT
if (cbinp->binsType() == VCoverBinsType::BINS_DEFAULT
|| cbinp->binsType() == VCoverBinsType::BINS_IGNORE
|| cbinp->binsType() == VCoverBinsType::BINS_ILLEGAL) {
continue;
@ -1285,7 +1285,7 @@ class FunctionalCoverageVisitor final : public VNVisitor {
std::vector<AstCoverBin*> cpBins;
for (AstNode* binp = cpp->binsp(); binp; binp = binp->nextp()) {
AstCoverBin* const cbinp = VN_CAST(binp, CoverBin);
if (cbinp && cbinp->binsType() == VCoverBinsType::USER) {
if (cbinp && cbinp->binsType() == VCoverBinsType::BINS_USER) {
cpBins.push_back(cbinp);
}
}

View File

@ -340,7 +340,7 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public VNVisitorConst {
default: putfs(nodep, "bins "); break;
}
puts(nodep->name());
if (nodep->binsType() == VCoverBinsType::DEFAULT) {
if (nodep->binsType() == VCoverBinsType::BINS_DEFAULT) {
puts(" = default");
} else if (nodep->transp()) {
puts(" = ");

View File

@ -1162,9 +1162,10 @@ class LinkParseVisitor final : public VNVisitor {
}
}
// IEEE: option
// IEEE: option / type_option members allow external access (cg_inst.option.X)
// and require std:: types; std:: is kept because setUsesStdPackage() is called
// at parse time for every covergroup declaration.
{
v3Global.setUsesStdPackage();
AstVar* const varp
= new AstVar{nodep->fileline(), VVarType::MEMBER, "option", VFlagChildDType{},
new AstRefDType{nodep->fileline(), "vl_covergroup_options_t",
@ -1173,12 +1174,10 @@ class LinkParseVisitor final : public VNVisitor {
nullptr}};
nodep->addMembersp(varp);
}
// IEEE: type_option
{
v3Global.setUsesStdPackage();
AstVar* const varp
= new AstVar{nodep->fileline(), VVarType::MEMBER, "type_option", VFlagChildDType{},
= new AstVar{nodep->fileline(), VVarType::MEMBER, "type_option",
VFlagChildDType{},
new AstRefDType{nodep->fileline(), "vl_covergroup_type_options_t",
new AstClassOrPackageRef{nodep->fileline(), "std",
nullptr, nullptr},

View File

@ -250,10 +250,6 @@ class CodeMotionAnalysisVisitor final : public VNVisitorConst {
void analyzeVarRef(AstVarRef* nodep) {
const VAccess access = nodep->access();
AstVar* const varp = nodep->varp();
// Add null check - varp can be null in some contexts (e.g., SenTree VarRefs)
if (!varp) return;
// Skip if not in a statement context (m_propsp can be null)
if (!m_propsp) return;
// Gather read and written variables
if (access.isReadOrRW()) m_propsp->m_rdVars.insert(varp);
if (access.isWriteOrRW()) m_propsp->m_wrVars.insert(varp);

View File

@ -331,16 +331,6 @@ class OrderGraphBuilder final : public VNVisitor {
void visit(AstCoverToggle* nodep) override { //
iterateLogic(nodep);
}
void visit(AstStmtExpr* nodep) override {
// StmtExpr wraps expressions used as statements (e.g., method calls).
// If it's under an AstActive but not already in a logic context, treat it as logic.
// Otherwise just iterate normally.
if (!m_logicVxp && m_domainp) {
iterateLogic(nodep);
} else {
iterateChildren(nodep);
}
}
//--- Ignored nodes
void visit(AstVar*) override {}

View File

@ -240,9 +240,6 @@ class SchedGraphBuilder final : public VNVisitor {
void visit(AstNodeProcedure* nodep) override { visitLogic(nodep); }
void visit(AstNodeAssign* nodep) override { visitLogic(nodep); }
void visit(AstCoverToggle* nodep) override { visitLogic(nodep); }
void visit(AstStmtExpr* nodep) override {
visitLogic(nodep);
} // Handle statement expressions like method calls
// Pre and Post logic are handled separately
void visit(AstAlwaysPre* nodep) override {}

View File

@ -343,12 +343,6 @@ class TimingSuspendableVisitor final : public VNVisitor {
}
}
void visit(AstNodeCCall* nodep) override {
// Skip automatic covergroup sampling calls (marked with user3==1)
if (nodep->user3()) {
iterateChildren(nodep);
return;
}
AstCFunc* funcp = nodep->funcp();
if (!funcp) {
iterateChildren(nodep);

View File

@ -224,6 +224,7 @@ class WidthVisitor final : public VNVisitor {
const AstCell* m_cellp = nullptr; // Current cell for arrayed instantiations
const AstEnumItem* m_enumItemp = nullptr; // Current enum item
AstNodeFTask* m_ftaskp = nullptr; // Current function/task
AstClass* m_cgClassp = nullptr; // Current covergroup class
AstNodeModule* m_modep = nullptr; // Current module
const AstConstraint* m_constraintp = nullptr; // Current constraint
AstNodeProcedure* m_procedurep = nullptr; // Current final/always
@ -1726,17 +1727,7 @@ class WidthVisitor final : public VNVisitor {
}
void visit(AstCgOptionAssign* nodep) override {
// Extract covergroup option values and store in AstClass before deleting
// Find parent covergroup (AstClass with isCovergroup() == true)
AstClass* cgClassp = nullptr;
for (AstNode* parentp = nodep->backp(); parentp; parentp = parentp->backp()) {
if (AstClass* classp = VN_CAST(parentp, Class)) {
if (classp->isCovergroup()) {
cgClassp = classp;
break;
}
}
}
AstClass* const cgClassp = m_cgClassp;
if (cgClassp) {
// Process supported options
if (nodep->name() == "auto_bin_max" && !nodep->typeOption()) {
@ -3392,11 +3383,12 @@ class WidthVisitor final : public VNVisitor {
return AstEqWild::newTyped(itemp->fileline(), exprp, itemp->unlinkFrBack());
}
void visit(AstInsideRange* nodep) override {
// Just do each side; AstInside will rip these nodes out later
// When m_vup is null we are in a covergroup bin context (not an expression context).
// Fold constant arithmetic (e.g., NEGATE(100) -> -100) so the children have their
// types set before further tree processing. Use constifyEdit (not constifyParamsEdit)
// to avoid errors on any non-constant expressions.
// 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) {
V3Const::constifyEdit(nodep->lhsp()); // lhsp may change
V3Const::constifyEdit(nodep->rhsp()); // rhsp may change
@ -7473,6 +7465,8 @@ class WidthVisitor final : public VNVisitor {
// Must do extends first, as we may in functions under this class
// start following a tree of extends that takes us to other classes
userIterateAndNext(nodep->extendsp(), nullptr);
VL_RESTORER(m_cgClassp);
if (nodep->isCovergroup()) m_cgClassp = nodep;
userIterateChildren(nodep, nullptr); // First size all members
}
void visit(AstNodeModule* nodep) override {

View File

@ -158,10 +158,7 @@ static void process() {
}
// Convert parseref's to varrefs, and other directly post parsing fixups
// Note: must run before removeStd() as it may create std:: references (e.g. covergroups)
V3LinkParse::linkParse(v3Global.rootp());
// Remove std package if unused (must be after V3LinkParse which may set usesStdPackage)
v3Global.removeStd();
// Cross-link signal names
// Cross-link dotted hierarchical references
V3LinkDot::linkDotPrimary(v3Global.rootp());
@ -751,6 +748,7 @@ static bool verilate(const string& argString) {
// Read first filename
v3Global.readFiles();
v3Global.removeStd();
// Link, etc, if needed
if (!v3Global.opt.preprocOnly()) { //

View File

@ -6943,6 +6943,9 @@ covergroup_declaration<nodep>: // ==IEEE: covergroup_declaration
}
$$ = new AstCovergroup{$<fl>1, *$2, static_cast<AstVar*>($3),
static_cast<AstVar*>(sampleArgsp), $6, clockp};
// Every covergroup has option/type_option members (added by V3LinkParse)
// referencing std:: types, so mark std as needed at parse time.
v3Global.setUsesStdPackage();
GRAMMARP->endLabel($<fl>8, $$, $8); }
| yCOVERGROUP yEXTENDS idAny ';'
/*cont*/ coverage_spec_or_optionListE
@ -7124,15 +7127,15 @@ bins_or_options<nodep>: // ==IEEE: bins_or_options
// // cgexpr part of trans_list
| yBINS idAny/*bin_identifier*/ bins_orBraE '=' trans_list iffE
{ FileLine* isArray = $<fl>3;
$$ = new AstCoverBin{$<fl>2, *$2, static_cast<AstCoverTransSet*>($5), false, false, isArray != nullptr};
$$ = new AstCoverBin{$<fl>2, *$2, static_cast<AstCoverTransSet*>($5), VCoverBinsType{VCoverBinsType::BINS_TRANSITION}, isArray != nullptr};
DEL($6); }
| yIGNORE_BINS idAny/*bin_identifier*/ bins_orBraE '=' trans_list iffE
{ FileLine* isArray = $<fl>3;
$$ = new AstCoverBin{$<fl>2, *$2, static_cast<AstCoverTransSet*>($5), true, false, isArray != nullptr};
$$ = new AstCoverBin{$<fl>2, *$2, static_cast<AstCoverTransSet*>($5), VCoverBinsType{VCoverBinsType::BINS_IGNORE}, isArray != nullptr};
DEL($6); }
| yILLEGAL_BINS idAny/*bin_identifier*/ bins_orBraE '=' trans_list iffE
{ FileLine* isArray = $<fl>3;
$$ = new AstCoverBin{$<fl>2, *$2, static_cast<AstCoverTransSet*>($5), false, true, isArray != nullptr};
$$ = new AstCoverBin{$<fl>2, *$2, static_cast<AstCoverTransSet*>($5), VCoverBinsType{VCoverBinsType::BINS_ILLEGAL}, isArray != nullptr};
DEL($6); }
| yWILDCARD yBINS idAny/*bin_identifier*/ bins_orBraE '=' trans_list iffE
{ $$ = nullptr; BBCOVERIGN($<fl>1, "Ignoring unsupported: cover bin 'wildcard' trans list"); DEL($6, $7);}
@ -7142,7 +7145,7 @@ bins_or_options<nodep>: // ==IEEE: bins_or_options
{ $$ = nullptr; BBCOVERIGN($<fl>1, "Ignoring unsupported: cover bin 'wildcard' trans list"); DEL($6, $7);}
//
| yBINS idAny/*bin_identifier*/ bins_orBraE '=' yDEFAULT iffE
{ $$ = new AstCoverBin{$<fl>2, *$2, VCoverBinsType::DEFAULT};
{ $$ = new AstCoverBin{$<fl>2, *$2, VCoverBinsType::BINS_DEFAULT};
DEL($6); }
| yIGNORE_BINS idAny/*bin_identifier*/ bins_orBraE '=' yDEFAULT iffE
{ $$ = new AstCoverBin{$<fl>2, *$2, VCoverBinsType::BINS_IGNORE};