diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 43228b494..5b3ee4a1c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -345,6 +345,7 @@ set(COMMON_SOURCES V3Sampled.cpp V3Sched.cpp V3SchedAcyclic.cpp + V3SchedCovergroup.cpp V3SchedPartition.cpp V3SchedReplicate.cpp V3SchedTiming.cpp diff --git a/src/Makefile_obj.in b/src/Makefile_obj.in index 1e3e60b6c..f076e1f8e 100644 --- a/src/Makefile_obj.in +++ b/src/Makefile_obj.in @@ -330,6 +330,7 @@ RAW_OBJS_PCH_ASTNOMT = \ V3Sampled.o \ V3Sched.o \ V3SchedAcyclic.o \ + V3SchedCovergroup.o \ V3SchedPartition.o \ V3SchedReplicate.o \ V3SchedTiming.o \ diff --git a/src/V3Order.cpp b/src/V3Order.cpp index e09637037..35c518395 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -102,13 +102,14 @@ void V3Order::orderOrderGraph(OrderGraph& graph, const std::string& tag) { AstCFunc* V3Order::order(AstNetlist* netlistp, // const std::vector& logic, // const V3Order::TrigToSenMap& trigToSen, + const V3Sched::CovergroupRefBindings& cgRefBindings, const string& tag, // bool parallel, // bool slow, // const ExternalDomainsProvider& externalDomains) { // Build the OrderGraph const std::unique_ptr graph - = buildOrderGraph(netlistp, logic, trigToSen, parallel); + = buildOrderGraph(netlistp, logic, trigToSen, cgRefBindings, parallel); // Order it orderOrderGraph(*graph, tag); // Assign sensitivity domains to combinational logic diff --git a/src/V3Order.h b/src/V3Order.h index 34d644e6e..e7c7ef000 100644 --- a/src/V3Order.h +++ b/src/V3Order.h @@ -32,6 +32,7 @@ class AstVarScope; namespace V3Sched { struct LogicByScope; +class CovergroupRefBindings; }; // namespace V3Sched //============================================================================ @@ -46,6 +47,7 @@ using TrigToSenMap = std::unordered_map; AstCFunc* order(AstNetlist* netlistp, // const std::vector& logic, // const TrigToSenMap& trigToSen, // + const V3Sched::CovergroupRefBindings& cgRefBindings, // const string& tag, // bool parallel, // bool slow, // diff --git a/src/V3OrderGraphBuilder.cpp b/src/V3OrderGraphBuilder.cpp index 0ecb89e35..9054dbbd0 100644 --- a/src/V3OrderGraphBuilder.cpp +++ b/src/V3OrderGraphBuilder.cpp @@ -112,6 +112,11 @@ class OrderGraphBuilder final : public VNVisitor { V3Sched::util::VarScopeSet m_forceReadEdgeIgnores; const bool m_parallel; // Ordering for multi-threaded execution (record variable accesses) + // What covergroup reference formal arguments are bound to at construction + const V3Sched::CovergroupRefBindings& m_cgRefBindings; + // Bindings reachable from the covergroup sample() being walked, nullptr when not in one + const V3Sched::CovergroupRefBindings::Bindings* m_cgRefBoundps = nullptr; + // METHODS void iterateLogic(AstNode* nodep) { @@ -198,18 +203,36 @@ class OrderGraphBuilder final : public VNVisitor { UASSERT_OBJ(m_logicVxp, nodep, "AstVarRef not under logic"); AstVarScope* const varscp = nodep->varScopep(); UASSERT_OBJ(varscp, nodep, "Var didn't get varscoped in V3Scope.cpp"); - - // Variable reference in logic. Add data dependency. - - // Record the raw access for the multi-threaded data hazard fixer - if (m_parallel) { - uint8_t recorded = 0; - if (nodep->access().isWriteOrRW()) recorded |= VA_WRITE; - if (nodep->access().isReadOrRW()) recorded |= VA_READ; - UASSERT_OBJ(recorded, nodep, "Unknown variable access type"); - // Accumulate access type, record the variable on first access only - if (!varscp->user4Or(recorded)) m_accessedVscps.push_back(varscp); + // Reading a covergroup 'ref' formal reads whatever it was bound to at construction. + // The formal itself is a pointer member fixed at construction, so it is not itself + // interesting to ordering. + const AstVar* const varp = nodep->varp(); + if (m_cgRefBoundps && varp->covergroupRefMember()) { + // Covergroup params are considered const-ref + UASSERT_OBJ(nodep->access().isReadOnly(), nodep, "covergroup ref argument is written"); + for (AstVarScope* const boundp : *m_cgRefBoundps) { + accountVarAccess(boundp, VAccess::READ, nodep); + } + } else { + accountVarAccess(varscp, nodep->access(), nodep); } + } + + // Record the raw access for the multi-threaded data hazard fixer + void recordRawAccess(AstVarScope* varscp, const VAccess& access, AstNode* nodep) { + if (!m_parallel) return; + uint8_t recorded = 0; + if (access.isWriteOrRW()) recorded |= VA_WRITE; + if (access.isReadOrRW()) recorded |= VA_READ; + UASSERT_OBJ(recorded, nodep, "Unknown variable access type"); + // Accumulate access type, record the variable on first access only + if (!varscp->user4Or(recorded)) m_accessedVscps.push_back(varscp); + } + + // Add the graph edges, and record the raw access, for one access of one variable + void accountVarAccess(AstVarScope* varscp, const VAccess& access, AstNode* nodep) { + // Variable reference in logic. Add data dependency. + recordRawAccess(varscp, access, nodep); // Check whether this variable was already generated/consumed in the same logic. We // don't want to add extra edges if the logic has many usages of the same variable, @@ -218,12 +241,11 @@ class OrderGraphBuilder final : public VNVisitor { const bool prevCon = varscp->user2() & VU_CON; // Compute whether the variable is produced (written) here - const bool gen - = !prevGen && nodep->access().isWriteOrRW() && !varscp->varp()->ignoreSchedWrite(); + const bool gen = !prevGen && access.isWriteOrRW() && !varscp->varp()->ignoreSchedWrite(); // Compute whether the value is consumed (read) here bool con = false; - if (!prevCon && nodep->access().isReadOrRW()) { + if (!prevCon && access.isReadOrRW()) { con = true; if (prevGen && !m_inClocked) { // Dangerous assumption: @@ -239,7 +261,11 @@ class OrderGraphBuilder final : public VNVisitor { // latch?). con = false; } - if (!m_inClocked && m_forceReadEdgeIgnores.count(varscp)) con = false; + if (!m_inClocked) { + // Ignored reads and references from within covergroups do not + // add to the combinational sensitivity of the block + if (m_forceReadEdgeIgnores.count(varscp) || m_cgRefBoundps) con = false; + } } // Note: See V3OrderGraph.h about the roles of the various vertex types @@ -323,7 +349,26 @@ class OrderGraphBuilder final : public VNVisitor { } } } - void visit(AstCCall* nodep) override { iterateChildren(nodep); } + // A covergroup sample() is not inlined and may read design signals through cross-scope + // references held by the covergroup. This attributes those references to the calling block. + void visit(AstCMethodCall* nodep) override { + iterateChildren(nodep); + AstCFunc* const funcp = nodep->funcp(); + if (!funcp->isCovergroupSample()) return; + // Since sample is a built-in, we never expect recursion. + UASSERT_OBJ(!m_cgRefBoundps, nodep, "Covergroup sample() calls another sample()"); + VL_RESTORER(m_cgRefBoundps); + // Reference formals are bound per covergroup object. If the call handle matches + // one that we recorded, use that info. If the call handle isn't something we + // recorded (eg array-element construction), use the union of references across + // the covergroup type. + const AstVarScope* instp = nullptr; + if (const AstVarRef* const fromRefp = VN_CAST(nodep->fromp(), VarRef)) { + instp = fromRefp->varScopep(); + } + m_cgRefBoundps = &m_cgRefBindings.forSample(instp, VN_AS(funcp->scopep()->modp(), Class)); + iterateChildren(funcp); + } //--- Logic akin to SystemVerilog Processes (AstNodeProcedure) void visit(AstInitial* nodep) override { // LCOV_EXCL_START @@ -384,9 +429,11 @@ class OrderGraphBuilder final : public VNVisitor { // CONSTRUCTOR OrderGraphBuilder(AstNetlist* /*nodep*/, const std::vector& coll, - const V3Order::TrigToSenMap& trigToSen, bool parallel) + const V3Order::TrigToSenMap& trigToSen, + const V3Sched::CovergroupRefBindings& cgRefBindings, bool parallel) : m_trigToSen{trigToSen} - , m_parallel{parallel} { + , m_parallel{parallel} + , m_cgRefBindings{cgRefBindings} { // Build the graph for (const V3Sched::LogicByScope* const lbsp : coll) { for (const auto& pair : *lbsp) { @@ -404,9 +451,10 @@ public: static std::unique_ptr apply(AstNetlist* nodep, const std::vector& coll, const V3Order::TrigToSenMap& trigToSen, + const V3Sched::CovergroupRefBindings& cgRefBindings, bool parallel) { return std::unique_ptr{ - OrderGraphBuilder{nodep, coll, trigToSen, parallel}.m_graphp}; + OrderGraphBuilder{nodep, coll, trigToSen, cgRefBindings, parallel}.m_graphp}; } }; @@ -414,6 +462,7 @@ std::unique_ptr V3Order::buildOrderGraph(AstNetlist* netlistp, // const std::vector& coll, // const V3Order::TrigToSenMap& trigToSen, // + const V3Sched::CovergroupRefBindings& cgRefBindings, // bool parallel) { - return OrderGraphBuilder::apply(netlistp, coll, trigToSen, parallel); + return OrderGraphBuilder::apply(netlistp, coll, trigToSen, cgRefBindings, parallel); } diff --git a/src/V3OrderInternal.h b/src/V3OrderInternal.h index facaab652..732ba224d 100644 --- a/src/V3OrderInternal.h +++ b/src/V3OrderInternal.h @@ -43,6 +43,7 @@ namespace V3Order { std::unique_ptr buildOrderGraph(AstNetlist* netlistp, // const std::vector& coll, // const TrigToSenMap& trigToSen, // + const V3Sched::CovergroupRefBindings& cgRefBindings, bool parallel); void orderOrderGraph(OrderGraph& graph, const std::string& tag); diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index ede728cd3..834b33c6f 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -391,7 +391,7 @@ void addVirtIfaceTriggerAssignments(AstNetlist* netlistp, AstCFunc* initFuncp, // Order the combinational logic to create the 'stl' region void createSettle(AstNetlist* netlistp, AstCFunc* const initFuncp, SenExprBuilder& senExprBulider, - LogicClasses& logicClasses) { + LogicClasses& logicClasses, const CovergroupRefBindings& cgRefBindings) { // Clone, because ordering is destructive, but we still need them for the other regions LogicByScope comb = logicClasses.m_comb.clone(); LogicByScope hybrid = logicClasses.m_hybrid.clone(); @@ -418,7 +418,7 @@ void createSettle(AstNetlist* netlistp, AstCFunc* const initFuncp, SenExprBuilde // Create and the body function AstCFunc* const stlFuncp = V3Order::order( - netlistp, {&comb, &hybrid}, trigToSen, "stl", false, true, + netlistp, {&comb, &hybrid}, trigToSen, cgRefBindings, "stl", false, true, [=](const AstVarScope*, std::vector& out) { out.push_back(inputChanged); }); util::splitCheck(stlFuncp); @@ -443,7 +443,8 @@ void createSettle(AstNetlist* netlistp, AstCFunc* const initFuncp, SenExprBuilde void createIcoRegion(AstNetlist* netlistp, AstCFunc* const initFuncp, SenExprBuilder& senExprBuilder, LogicByScope& logic, - const VirtIfaceTriggers& virtIfaceTriggers) { + const VirtIfaceTriggers& virtIfaceTriggers, + const CovergroupRefBindings& cgRefBindings) { // SystemC only: Any top level inputs feeding a combinational logic must be marked, // so we can make them sc_sensitive if (v3Global.opt.systemC()) { @@ -544,7 +545,7 @@ void createIcoRegion(AstNetlist* netlistp, AstCFunc* const initFuncp, // Create and Order the body function AstCFunc* const icoFuncp = V3Order::order( - netlistp, {&logic}, trigToSen, "ico", false, false, + netlistp, {&logic}, trigToSen, cgRefBindings, "ico", false, false, [&](const AstVarScope* vscp, std::vector& out) { AstVar* const varp = vscp->varp(); // If it has an explicit change detect trigger, use that, @@ -858,6 +859,9 @@ void schedule(AstNetlist* netlistp) { // Step 2: Prepare external domains for timing and virtual interfaces // Create extra triggers for virtual interfaces const auto& virtIfaceTriggers = makeVirtIfaceTriggers(netlistp); + // Resolve what covergroup reference formal arguments are bound to, which is visible here + // but not from V3Order, where the reads through them must be modeled + const CovergroupRefBindings& cgRefBindings = makeCovergroupRefBindings(netlistp); // Prepare timing-related logic and external domains TimingKit timingKit = prepareTiming(netlistp); @@ -898,7 +902,7 @@ void schedule(AstNetlist* netlistp) { SenExprBuilder senExprBuilder{scopeTopp}; // Step 6: Create 'settle' region that restores the combinational invariant - createSettle(netlistp, staticp, senExprBuilder, logicClasses); + createSettle(netlistp, staticp, senExprBuilder, logicClasses, cgRefBindings); if (v3Global.opt.stats()) V3Stats::statsStage("sched-settle"); // Step 7: Partition the clocked and combinational (including hybrid) logic into pre/act/nba. @@ -929,7 +933,8 @@ void schedule(AstNetlist* netlistp) { } // Step 9: Create the input combinational logic - createIcoRegion(netlistp, staticp, senExprBuilder, logicReplicas.m_ico, virtIfaceTriggers); + createIcoRegion(netlistp, staticp, senExprBuilder, logicReplicas.m_ico, virtIfaceTriggers, + cgRefBindings); if (v3Global.opt.stats()) V3Stats::statsStage("sched-create-ico"); // Step 10: Create the triggers @@ -998,7 +1003,8 @@ void schedule(AstNetlist* netlistp) { AstCFunc* const actFuncp = V3Order::order( netlistp, {&logicRegions.m_pre, &logicRegions.m_act, &logicReplicas.m_act}, trigToSenAct, - "act", false, false, [&](const AstVarScope* vscp, std::vector& out) { + cgRefBindings, "act", false, false, + [&](const AstVarScope* vscp, std::vector& out) { auto it = actTimingDomains.find(vscp); if (it != actTimingDomains.end()) out = it->second; if (vscp->varp()->isWrittenByDpi()) out.push_back(dpiExportTriggeredAct); @@ -1035,7 +1041,8 @@ void schedule(AstNetlist* netlistp) { const auto& timingDomains = timingKit.remapDomains(trigMap); AstCFunc* const funcp = V3Order::order( - netlistp, logic, trigToSen, name, name == "nba" && v3Global.opt.mtasks(), false, + netlistp, logic, trigToSen, cgRefBindings, name, + name == "nba" && v3Global.opt.mtasks(), false, [&](const AstVarScope* vscp, std::vector& out) { auto it = timingDomains.find(vscp); if (it != timingDomains.end()) out = it->second; @@ -1119,6 +1126,15 @@ void schedule(AstNetlist* netlistp) { // Step 18: Clean up netlistp->clearStlFirstIterationp(); + if (v3Global.opt.stats()) { + // A sample() call resolved to the union over its covergroup's constructions reads more + // than it can, which orders it against more logic than necessary + V3Stats::addStat("Scheduling, covergroup ref sample calls, per instance", + cgRefBindings.numExactCalls()); + V3Stats::addStat("Scheduling, covergroup ref sample calls, per type", + cgRefBindings.numUnionCalls()); + } + // Haven't split static initializer yet util::splitCheck(staticp); diff --git a/src/V3Sched.h b/src/V3Sched.h index 3dbe6b026..456947615 100644 --- a/src/V3Sched.h +++ b/src/V3Sched.h @@ -430,9 +430,59 @@ public: VirtIfaceTriggers& operator=(VirtIfaceTriggers&&) = default; }; +// Design variables bound to covergroup 'ref'/'const ref' formal arguments at construction. +// +// Such a formal becomes a pointer member, bound when the covergroup is constructed, so a +// sample() reading through it holds no AstVarRef naming the design variable. V3Order has to +// model those reads, but by the time it runs the constructions sit in logic it does not order, +// so they are collected before ordering and handed to it. +class CovergroupRefBindings final { +public: + // The design variables one covergroup sample() may read through its reference formals + using Bindings = std::vector; + +private: + // Bindings of one covergroup handle. Exact, and so only present for a handle whose every + // write is a construction we recognized -- see dropInstance(). + std::unordered_map m_byInstance; + // Bindings over every construction of a covergroup class, used where the handle a sample() + // call reaches is not known. An over-approximation, which is safe. + std::unordered_map m_byClass; + // How the sample() calls seen so far resolved. Statistics only, hence mutable: ordering + // runs once per scheduling region and must hold this by const reference. + mutable uint32_t m_numExactCalls = 0; + mutable uint32_t m_numUnionCalls = 0; + // Returned for a covergroup with no reference formal at all. + static const Bindings s_none; + +public: + // Record one construction of covergroup 'classp' binding reference formals to 'bindings'. + // 'instp' is the handle assigned, or nullptr if the destination could not be identified. + void addConstruction(const AstClass* classp, const AstVarScope* instp, + const Bindings& bindings); + // Forget the exact bindings of a handle, which turned out to be written by something other + // than a construction we recognized. Its sample() calls fall back to the class union. + void dropInstance(const AstVarScope* instp) { m_byInstance.erase(instp); } + // What a sample() of covergroup 'classp' may read through reference formals, when called on + // handle 'instp' (nullptr if the call is not on a plain handle reference) + const Bindings& forSample(const AstVarScope* instp, const AstClass* classp) const; + // Sample calls of a covergroup with a reference formal resolved to one handle's bindings + uint32_t numExactCalls() const { return m_numExactCalls; } + // ... and those that had to take the class union instead + uint32_t numUnionCalls() const { return m_numUnionCalls; } + + VL_UNCOPYABLE(CovergroupRefBindings); + CovergroupRefBindings() = default; + CovergroupRefBindings(CovergroupRefBindings&&) = default; + CovergroupRefBindings& operator=(CovergroupRefBindings&&) = default; +}; + // Creates trigger vars for signals driven via virtual interfaces VirtIfaceTriggers makeVirtIfaceTriggers(AstNetlist* nodep) VL_MT_DISABLED; +// Collects what covergroup reference formal arguments are bound to at construction +const CovergroupRefBindings makeCovergroupRefBindings(AstNetlist* nodep) VL_MT_DISABLED; + // Creates the timing kit and marks variables written by suspendables TimingKit prepareTiming(AstNetlist* const netlistp) VL_MT_DISABLED; diff --git a/src/V3SchedCovergroup.cpp b/src/V3SchedCovergroup.cpp new file mode 100644 index 000000000..14c6465c8 --- /dev/null +++ b/src/V3SchedCovergroup.cpp @@ -0,0 +1,191 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// DESCRIPTION: Verilator: Resolve covergroup reference formal arguments for +// scheduling +// +// Code available from: https://verilator.org +// +//************************************************************************* +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of either the GNU Lesser General Public License Version 3 +// or the Perl Artistic License Version 2.0. +// SPDX-FileCopyrightText: 2003-2026 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* +// V3SchedCovergroup's Transformations: +// +// None - this only gathers information. +// +// A covergroup 'ref'/'const ref' formal argument becomes a pointer member of the covergroup +// class, bound when the covergroup is constructed. A sample() reading through it therefore +// holds no AstVarRef naming the design variable it reads, and V3Order would see the sampling +// block as reading nothing. Record what each construction binds, so V3Order can attribute +// those reads to the blocks that call sample(). +// +// Bindings are keyed by the constructed handle where that is exact, which it is when every +// write of the handle is a construction of the recognized shape 'handle = new(...)'. Any other +// way for a covergroup object to reach a handle - an aliasing assignment, a temporary +// introduced by V3Task, an array element, passing the handle to a function by reference - is +// itself a write of that handle, and taints it. A tainted handle falls back to the union over +// all constructions of its class, which is an over-approximation, but safe. +// +//************************************************************************* + +#include "V3PchAstNoMT.h" // VL_MT_DISABLED_CODE_UNIT + +#include "V3AstNodeExpr.h" +#include "V3Sched.h" + +#include + +VL_DEFINE_DEBUG_FUNCTIONS; + +namespace V3Sched { + +const CovergroupRefBindings::Bindings CovergroupRefBindings::s_none; + +void CovergroupRefBindings::addConstruction(const AstClass* classp, const AstVarScope* instp, + const Bindings& bindings) { + Bindings& classBindings = m_byClass[classp]; + classBindings.insert(classBindings.end(), bindings.begin(), bindings.end()); + // Note this creates an entry even when 'bindings' is empty, so that a handle binding + // nothing stays distinguishable from a handle we know nothing about + if (instp) { + Bindings& instBindings = m_byInstance[instp]; + instBindings.insert(instBindings.end(), bindings.begin(), bindings.end()); + } +} + +const CovergroupRefBindings::Bindings& +CovergroupRefBindings::forSample(const AstVarScope* instp, const AstClass* classp) const { + if (instp) { + const auto it = m_byInstance.find(instp); + if (it != m_byInstance.end()) { + ++m_numExactCalls; + return it->second; + } + } + const auto it = m_byClass.find(classp); + if (it != m_byClass.end()) { + ++m_numUnionCalls; + return it->second; + } + // A covergroup with no reference formal at all + return s_none; +} + +class CovergroupRefBindVisitor final : public VNVisitorConst { + // STATE + CovergroupRefBindings m_bindings; // Result + // Covergroup handles written by something other than a construction we recognized + std::unordered_set m_tainted; + + // METHODS + + // The covergroup class this call constructs, or nullptr if it is not a covergroup + // construction + static AstClass* constructedCovergroup(const AstNodeCCall* nodep) { + const AstCFunc* const funcp = nodep->funcp(); + if (!funcp->isConstructor()) return nullptr; + // A constructor is a class method, so it is scoped and its scope is that of a class + AstClass* const classp = VN_AS(funcp->scopep()->modp(), Class); + return classp->isCovergroup() ? classp : nullptr; + } + + // True if this variable holds a handle to a covergroup object + static bool isCovergroupHandle(const AstVar* varp) { + const AstClassRefDType* const dtypep = VN_CAST(varp->dtypep()->skipRefp(), ClassRefDType); + return dtypep && dtypep->classp()->isCovergroup(); + } + + // True if this constructor takes a reference formal, and so has anything to bind + static bool hasRefFormal(const AstCFunc* funcp) { + for (AstNode* portp = funcp->argsp(); portp; portp = portp->nextp()) { + const AstVar* const varp = VN_AS(portp, Var); + if (varp->declDirection().isRef() || varp->declDirection().isConstRef()) return true; + } + return false; + } + + // Record one construction of 'classp' assigning to 'instp' (nullptr if not identified). + // A covergroup with no reference formal has nothing to bind, and is left out entirely, so + // that an entry with no bindings means only 'this handle binds nothing a sample() reads'. + void recordConstruction(AstNodeCCall* nodep, const AstClass* classp, + const AstVarScope* instp) { + if (!hasRefFormal(nodep->funcp())) return; + CovergroupRefBindings::Bindings bindings; + // Actuals correspond one to one, in order, with the function's argument variables. + // A constructor returns void, so none of them is a return value variable. + AstNode* actualp = nodep->argsp(); + for (AstNode* portp = nodep->funcp()->argsp(); portp; portp = portp->nextp()) { + AstNode* const thisActualp = actualp; + actualp = actualp->nextp(); + const AstVar* const varp = VN_AS(portp, Var); + if (!varp->declDirection().isRef() && !varp->declDirection().isConstRef()) continue; + // A 'ref' actual is an lvalue expression, so it need not be a plain variable + // reference. Bind every variable it names: for 'sigs[0]' that is 'sigs', which is + // exact rather than approximate, as V3Order models the whole array as one + // VarScope. An index expression contributes its own variables as well, which is an + // over-approximation, and so safe. + thisActualp->foreach([&](AstVarRef* refp) { + AstVarScope* const vscp = refp->varScopep(); + bindings.push_back(vscp); + }); + } + m_bindings.addConstruction(classp, instp, bindings); + } + + // VISITORS + void visit(AstNodeAssign* nodep) override { + AstCNew* const cnewp = VN_CAST(nodep->rhsp(), CNew); + AstVarRef* const lhsRefp = VN_CAST(nodep->lhsp(), VarRef); + AstClass* const classp = cnewp && lhsRefp ? constructedCovergroup(cnewp) : nullptr; + if (!classp) { + iterateChildrenConst(nodep); + return; + } + recordConstruction(cnewp, classp, lhsRefp->varScopep()); + // Deliberately not iterating the destination: this write is the one shape that does not + // taint the handle. Do iterate the arguments, which may write handles of their own. + iterateChildrenConst(cnewp); + } + + void visit(AstNodeCCall* nodep) override { + iterateChildrenConst(nodep); + // A construction reached only here is one whose destination we could not identify, so + // every handle of the class must assume it + if (const AstClass* const classp = constructedCovergroup(nodep)) { + recordConstruction(nodep, classp, nullptr); + } + } + + void visit(AstVarRef* nodep) override { + if (!nodep->access().isWriteOrRW()) return; + if (!isCovergroupHandle(nodep->varp())) return; + m_tainted.emplace(nodep->varScopep()); + } + + void visit(AstNode* nodep) override { iterateChildrenConst(nodep); } + +public: + // CONSTRUCTORS + explicit CovergroupRefBindVisitor(AstNetlist* nodep) { + iterateConst(nodep); + for (const AstVarScope* const vscp : m_tainted) m_bindings.dropInstance(vscp); + } + ~CovergroupRefBindVisitor() override = default; + + // METHODS + CovergroupRefBindings take_bindings() { return std::move(m_bindings); } +}; + +const CovergroupRefBindings makeCovergroupRefBindings(AstNetlist* nodep) { + UINFO(2, __FUNCTION__ << ":"); + CovergroupRefBindings bindings{}; + if (v3Global.useCovergroup()) bindings = CovergroupRefBindVisitor{nodep}.take_bindings(); + return bindings; +} + +} // namespace V3Sched diff --git a/test_regress/t/coverage_covergroup_common.py b/test_regress/t/coverage_covergroup_common.py index 7c33beddc..8524a7cc2 100644 --- a/test_regress/t/coverage_covergroup_common.py +++ b/test_regress/t/coverage_covergroup_common.py @@ -45,8 +45,11 @@ def covergroup_coverage_report(test, outfile=None): return outfile -def run(test, *, verilator_flags2=(), timing_loop=False): - test.compile(verilator_flags2=['--coverage', *verilator_flags2], timing_loop=timing_loop) +def run(test, *, verilator_flags2=(), timing_loop=False, threads=None): + compile_args = {} if threads is None else {'threads': threads} + test.compile(verilator_flags2=['--coverage', *verilator_flags2], + timing_loop=timing_loop, + **compile_args) test.execute() covergroup_coverage_report(test) test.files_identical(test.obj_dir + '/covergroup_report.txt', test.golden_filename) diff --git a/test_regress/t/t_covergroup_args.py b/test_regress/t/t_covergroup_args.py index 20a45138f..654c47626 100755 --- a/test_regress/t/t_covergroup_args.py +++ b/test_regress/t/t_covergroup_args.py @@ -11,6 +11,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_array_bins.py b/test_regress/t/t_covergroup_array_bins.py index 6c6b34a1d..ca81cf2c1 100755 --- a/test_regress/t/t_covergroup_array_bins.py +++ b/test_regress/t/t_covergroup_array_bins.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test, verilator_flags2=['--Wno-COVERIGN']) diff --git a/test_regress/t/t_covergroup_auto_bin_max.py b/test_regress/t/t_covergroup_auto_bin_max.py index 4c22a684c..7412b8231 100755 --- a/test_regress/t/t_covergroup_auto_bin_max.py +++ b/test_regress/t/t_covergroup_auto_bin_max.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_auto_sample_timing.py b/test_regress/t/t_covergroup_auto_sample_timing.py index 2a533c364..dc192fe8f 100755 --- a/test_regress/t/t_covergroup_auto_sample_timing.py +++ b/test_regress/t/t_covergroup_auto_sample_timing.py @@ -10,7 +10,7 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') # Use the same .v file as the non-timing test test.top_filename = "t/t_covergroup_clocked_sample.v" diff --git a/test_regress/t/t_covergroup_bin_counts.py b/test_regress/t/t_covergroup_bin_counts.py index b25c6b6b5..16258ed8c 100755 --- a/test_regress/t/t_covergroup_bin_counts.py +++ b/test_regress/t/t_covergroup_bin_counts.py @@ -10,7 +10,7 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_clocked_sample.py b/test_regress/t/t_covergroup_clocked_sample.py index be9b453d3..9f6b5465d 100755 --- a/test_regress/t/t_covergroup_clocked_sample.py +++ b/test_regress/t/t_covergroup_clocked_sample.py @@ -10,7 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -# Issue #7779 unstable with --vltmt -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_default_bins.py b/test_regress/t/t_covergroup_default_bins.py index 6b0f1a9e4..9f6b5465d 100755 --- a/test_regress/t/t_covergroup_default_bins.py +++ b/test_regress/t/t_covergroup_default_bins.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_embedded.py b/test_regress/t/t_covergroup_embedded.py index a5fa769b7..d0346a872 100755 --- a/test_regress/t/t_covergroup_embedded.py +++ b/test_regress/t/t_covergroup_embedded.py @@ -11,7 +11,7 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run( test, diff --git a/test_regress/t/t_covergroup_embedded_timing.py b/test_regress/t/t_covergroup_embedded_timing.py index c1ef59356..d54b7bd6a 100755 --- a/test_regress/t/t_covergroup_embedded_timing.py +++ b/test_regress/t/t_covergroup_embedded_timing.py @@ -11,6 +11,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test, verilator_flags2=['--timing'], timing_loop=True) diff --git a/test_regress/t/t_covergroup_empty.py b/test_regress/t/t_covergroup_empty.py index 27bc569af..36de5885f 100755 --- a/test_regress/t/t_covergroup_empty.py +++ b/test_regress/t/t_covergroup_empty.py @@ -6,7 +6,7 @@ import vltest_bootstrap -test.scenarios('vlt') +test.scenarios('vlt_all') test.compile() diff --git a/test_regress/t/t_covergroup_iff.py b/test_regress/t/t_covergroup_iff.py index 6b0f1a9e4..9f6b5465d 100755 --- a/test_regress/t/t_covergroup_iff.py +++ b/test_regress/t/t_covergroup_iff.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_ignore_bins.py b/test_regress/t/t_covergroup_ignore_bins.py index 6b0f1a9e4..9f6b5465d 100755 --- a/test_regress/t/t_covergroup_ignore_bins.py +++ b/test_regress/t/t_covergroup_ignore_bins.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_illegal_bins.py b/test_regress/t/t_covergroup_illegal_bins.py index 6b0f1a9e4..9f6b5465d 100755 --- a/test_regress/t/t_covergroup_illegal_bins.py +++ b/test_regress/t/t_covergroup_illegal_bins.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_in_class_namespace.py b/test_regress/t/t_covergroup_in_class_namespace.py index 20a45138f..654c47626 100755 --- a/test_regress/t/t_covergroup_in_class_namespace.py +++ b/test_regress/t/t_covergroup_in_class_namespace.py @@ -11,6 +11,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_member_event.py b/test_regress/t/t_covergroup_member_event.py index cbf529a7d..8bbec9235 100755 --- a/test_regress/t/t_covergroup_member_event.py +++ b/test_regress/t/t_covergroup_member_event.py @@ -9,7 +9,7 @@ import vltest_bootstrap -test.scenarios('vlt') +test.scenarios('vlt_all') test.compile(verilator_flags2=['--timing']) diff --git a/test_regress/t/t_covergroup_negative_ranges.py b/test_regress/t/t_covergroup_negative_ranges.py index ab1d96563..ecb91b4b1 100755 --- a/test_regress/t/t_covergroup_negative_ranges.py +++ b/test_regress/t/t_covergroup_negative_ranges.py @@ -7,6 +7,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_option.py b/test_regress/t/t_covergroup_option.py index c1bdd3d22..a2114653a 100755 --- a/test_regress/t/t_covergroup_option.py +++ b/test_regress/t/t_covergroup_option.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test, verilator_flags2=['--Wno-COVERIGN']) diff --git a/test_regress/t/t_covergroup_option_no_coverage.py b/test_regress/t/t_covergroup_option_no_coverage.py index 5f3bb6c6d..3fef0faee 100755 --- a/test_regress/t/t_covergroup_option_no_coverage.py +++ b/test_regress/t/t_covergroup_option_no_coverage.py @@ -9,7 +9,7 @@ import vltest_bootstrap -test.scenarios('vlt') +test.scenarios('vlt_all') test.top_filename = 't/t_covergroup_option.v' # runs without --coverage diff --git a/test_regress/t/t_covergroup_param_bins.py b/test_regress/t/t_covergroup_param_bins.py index ab1d96563..ecb91b4b1 100755 --- a/test_regress/t/t_covergroup_param_bins.py +++ b/test_regress/t/t_covergroup_param_bins.py @@ -7,6 +7,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_ref_bind.out b/test_regress/t/t_covergroup_ref_bind.out new file mode 100644 index 000000000..7fba95aa8 --- /dev/null +++ b/test_regress/t/t_covergroup_ref_bind.out @@ -0,0 +1,16 @@ +cg_w.cp_w.one: 1 +cg_w.cp_w.three: 2 +cg_w.cp_w.two: 1 +cg_w.cp_w.zero: 1 +cg_x.cp_x.one: 1 +cg_x.cp_x.three: 1 +cg_x.cp_x.two: 1 +cg_x.cp_x.zero: 2 +cg_y.cp_y.one: 3 +cg_y.cp_y.three: 0 +cg_y.cp_y.two: 1 +cg_y.cp_y.zero: 1 +cg_z.cp_z.one: 1 +cg_z.cp_z.three: 3 +cg_z.cp_z.two: 1 +cg_z.cp_z.zero: 5 diff --git a/test_regress/t/t_covergroup_ref_bind.py b/test_regress/t/t_covergroup_ref_bind.py new file mode 100755 index 000000000..4afc4d2c4 --- /dev/null +++ b/test_regress/t/t_covergroup_ref_bind.py @@ -0,0 +1,30 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# 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_all') + +# Sampling what a non-blocking assignment writes is a data race if left unordered, but whether +# it changes the histogram in any one run is timing dependent, so use ThreadSanitizer to detect +# it reliably. --no-threads-coarsen keeps the sample and the writer in separate MTasks. +test.enable_tsan() + +coverage_covergroup_common.run(test, + verilator_flags2=(['--stats'] + + (['--no-threads-coarsen'] if test.vltmt else [])), + threads=(2 if test.vltmt else 1)) + +# Both resolutions order the sample correctly, so only these distinguish them: x_inst and +# y_inst are sampled through the handle they were constructed into, z_alias, z_alias2 and +# w_stale are not. w_stale is the one that has a construction of its own, so it only lands in +# the union if the reassignment actually dropped that construction's binding. +test.file_grep(test.stats, r'Scheduling, covergroup ref sample calls, per instance\s+(\d+)', 2) +test.file_grep(test.stats, r'Scheduling, covergroup ref sample calls, per type\s+(\d+)', 3) diff --git a/test_regress/t/t_covergroup_ref_bind.v b/test_regress/t/t_covergroup_ref_bind.v new file mode 100644 index 000000000..87890a4c1 --- /dev/null +++ b/test_regress/t/t_covergroup_ref_bind.v @@ -0,0 +1,136 @@ +// DESCRIPTION: Verilator: Test covergroup 'ref' formal bindings resolved per instance +// A covergroup reference formal is bound at construction, so a sample() reading through it +// names no design signal. What each sample() may read is resolved from the handle it is +// called on where every write of that handle is a construction (x_inst, y_inst), and from the +// union over the covergroup type where it is not (z_alias, aliased from z_first). w_stale +// covers the case where a handle has a construction of its own and is reassigned anyway, so +// the binding that would resolve it exactly is stale and must be discarded. All must order +// the sample against the non-blocking writer of what they sample; only how much else they are +// ordered against differs. Runs under --vltmt, where an unordered sample is a data race. +// 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 + +module t ( + input clk +); + + logic [1:0] siga; + logic [1:0] sigb; + logic [1:0] sigc; + logic [1:0] sigd; + logic [1:0] sige; + logic [1:0] sigf; + + covergroup cg_x(ref logic [1:0] sig); + cp_x: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + covergroup cg_y(ref logic [1:0] sig); + cp_y: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + covergroup cg_z(ref logic [1:0] sig); + cp_z: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + covergroup cg_w(ref logic [1:0] sig); + cp_w: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + // Constructed and sampled through the same handle: bindings are known exactly + cg_x x_inst = new(siga); + cg_y y_inst = new(sigb); + + // Two instances of one type, each sampled through a handle that is assigned rather than + // constructed, so the sample must assume either of them + cg_z z_first = new(sigc); + cg_z z_second = new(sigd); + cg_z z_alias; + cg_z z_alias2; + + // Constructed into a handle of its own, so this construction would resolve w_stale to sige + // exactly. The assignment below is a write of the handle all the same, which makes that + // binding stale and must drop it, leaving the union over cg_w -- sige and sigf both. + cg_w w_stale = new(sige); + cg_w w_real = new(sigf); + cg_w w_stale_hold; + + initial begin + z_alias = z_first; + z_alias2 = z_second; + w_stale_hold = w_stale; + w_stale = w_real; + end + + always @(posedge clk) x_inst.sample(); + always @(posedge clk) y_inst.sample(); + always @(posedge clk) z_alias.sample(); + always @(posedge clk) z_alias2.sample(); + // Reads sigf, through the object the assignment above stored, not the sige of its own + // construction + always @(posedge clk) w_stale.sample(); + + int cyc = 0; + + always @(posedge clk) begin + cyc <= cyc + 1; + + case (cyc) + 0: begin + siga <= 2'b00; + sigb <= 2'b01; + sigc <= 2'b11; + sigd <= 2'b00; + sige <= 2'b11; + end + 1: begin + siga <= 2'b01; + sigb <= 2'b01; + sigc <= 2'b11; + sigd <= 2'b01; + sige <= 2'b10; + end + 2: begin + siga <= 2'b10; + sigb <= 2'b01; + sigc <= 2'b00; + sigd <= 2'b10; + sige <= 2'b00; + end + 3: begin + siga <= 2'b11; + sigb <= 2'b10; + sigc <= 2'b00; + sigd <= 2'b11; + sige <= 2'b01; + end + 4: begin + // Ensure w_stale_hold isn't optimized away. Clean-up after coverage registry lands. + if (w_stale_hold == null) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + endcase + end + + // What w_stale really reads, written in a block of its own so that ordering its sample + // against the stale binding above would leave this write unordered, and so a data race + always @(posedge clk) begin + case (cyc) + 0: sigf <= 2'b01; + 1: sigf <= 2'b10; + 2: sigf <= 2'b11; + 3: sigf <= 2'b11; + default: ; + endcase + end +endmodule diff --git a/test_regress/t/t_covergroup_ref_bind_lvalue.out b/test_regress/t/t_covergroup_ref_bind_lvalue.out new file mode 100644 index 000000000..48c1ccd5a --- /dev/null +++ b/test_regress/t/t_covergroup_ref_bind_lvalue.out @@ -0,0 +1,8 @@ +cg_a.cp_a.one: 1 +cg_a.cp_a.three: 4 +cg_a.cp_a.two: 2 +cg_a.cp_a.zero: 3 +cg_b.cp_b.one: 2 +cg_b.cp_b.three: 3 +cg_b.cp_b.two: 2 +cg_b.cp_b.zero: 3 diff --git a/test_regress/t/t_covergroup_ref_bind_lvalue.py b/test_regress/t/t_covergroup_ref_bind_lvalue.py new file mode 100755 index 000000000..b69d15553 --- /dev/null +++ b/test_regress/t/t_covergroup_ref_bind_lvalue.py @@ -0,0 +1,33 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# 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_all') + +# Sampling what a non-blocking assignment writes is a data race if left unordered, but whether +# it changes the histogram in any one run is timing dependent, so use ThreadSanitizer to detect +# it reliably. --no-threads-coarsen keeps the sample and the writer in separate MTasks. +test.enable_tsan() + +# -fno-lift-expr is what makes this test bite. With expression lifting on, V3LiftExpr rewrites +# 'arr[0] = new(sigs[0])' into '__VlemCall_0 = new(sigs[0]); arr[0] = __VlemCall_0', so the +# construction always assigns to a plain variable and the array element never reaches +# V3SchedCovergroup. With it off the raw form survives, and the binding is only seen at all +# because constructions are also collected where they are not a simple assignment. +coverage_covergroup_common.run(test, + verilator_flags2=(['--stats', '-fno-lift-expr'] + + (['--no-threads-coarsen'] if test.vltmt else [])), + threads=(2 if test.vltmt else 1)) + +# No sample names a covergroup object, so each resolves to the union over its type's two +# constructions rather than to one instance +test.file_grep(test.stats, r'Scheduling, covergroup ref sample calls, per instance\s+(\d+)', 0) +test.file_grep(test.stats, r'Scheduling, covergroup ref sample calls, per type\s+(\d+)', 4) diff --git a/test_regress/t/t_covergroup_ref_bind_lvalue.v b/test_regress/t/t_covergroup_ref_bind_lvalue.v new file mode 100644 index 000000000..22d249957 --- /dev/null +++ b/test_regress/t/t_covergroup_ref_bind_lvalue.v @@ -0,0 +1,114 @@ +// DESCRIPTION: Verilator: Test covergroup 'ref' bindings where no handle is a plain variable +// Companion to t_covergroup_ref_bind, which covers the resolvable shapes. Here nothing is a +// plain variable: the covergroup is constructed into an array element, sampled through an array +// element, and the 'ref' actual is an array element too. So neither the construction nor the +// sample names one covergroup object, and both must fall back to the union over the covergroup +// type -- which must still order every sample against the non-blocking writer of what any +// instance of that type reads. cg_b repeats that with a struct member as the handle instead of +// an array element. Runs under --vltmt, where an unordered sample is a data race, +// and with -fno-lift-expr, which leaves the construction assigning directly to the array +// element instead of to a lifted temporary. +// 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 + +module t ( + input clk +); + + logic [1:0] sigs[2]; + logic [1:0] sigt[2]; + + covergroup cg_a(ref logic [1:0] sig); + cp_a: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + covergroup cg_b(ref logic [1:0] sig); + cp_b: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + cg_a arr[2]; + + typedef struct { + cg_b cg; + } struct_t; + + struct_t s0; + struct_t s1; + + // The construction destination is an array element, so there is no handle to key the + // bindings on and every instance of cg_a must assume both of them. The 'ref' actual is an + // array element as well, and binds the array it selects from. + initial begin + arr[0] = new(sigs[0]); + arr[1] = new(sigs[1]); + end + + // Same shape again with a struct member holding the handle: the construction destination and + // the sample receiver are both member selects, so neither names one covergroup object + initial s0.cg = new(sigt[0]); + initial s1.cg = new(sigt[1]); + + // The sample receiver is an array element, so it names no one covergroup object either + always @(posedge clk) arr[0].sample(); + always @(posedge clk) arr[1].sample(); + always @(posedge clk) s0.cg.sample(); + always @(posedge clk) s1.cg.sample(); + + int cyc = 0; + + always @(posedge clk) begin + cyc <= cyc + 1; + + case (cyc) + 0: begin + sigs[0] <= 2'b01; + sigs[1] <= 2'b11; + end + 1: begin + sigs[0] <= 2'b10; + sigs[1] <= 2'b11; + end + 2: begin + sigs[0] <= 2'b11; + sigs[1] <= 2'b10; + end + 3: begin + sigs[0] <= 2'b11; + sigs[1] <= 2'b00; + end + 4: begin + $write("*-* All Finished *-*\n"); + $finish; + end + endcase + end + + // What the struct-held covergroups read, written in a block of its own so that leaving their + // samples unordered against it is a data race on its own + always @(posedge clk) begin + case (cyc) + 0: begin + sigt[0] <= 2'b01; + sigt[1] <= 2'b10; + end + 1: begin + sigt[0] <= 2'b10; + sigt[1] <= 2'b11; + end + 2: begin + sigt[0] <= 2'b11; + sigt[1] <= 2'b11; + end + 3: begin + sigt[0] <= 2'b00; + sigt[1] <= 2'b01; + end + default: ; + endcase + end +endmodule diff --git a/test_regress/t/t_covergroup_sample_comb_loop.out b/test_regress/t/t_covergroup_sample_comb_loop.out new file mode 100644 index 000000000..505bcd328 --- /dev/null +++ b/test_regress/t/t_covergroup_sample_comb_loop.out @@ -0,0 +1,8 @@ +cg_direct.cp_direct.one: 2 +cg_direct.cp_direct.three: 1 +cg_direct.cp_direct.two: 0 +cg_direct.cp_direct.zero: 3 +cg_ref.cp_ref.one: 2 +cg_ref.cp_ref.three: 1 +cg_ref.cp_ref.two: 1 +cg_ref.cp_ref.zero: 2 diff --git a/test_regress/t/t_covergroup_sample_comb_loop.py b/test_regress/t/t_covergroup_sample_comb_loop.py new file mode 100755 index 000000000..c310df07b --- /dev/null +++ b/test_regress/t/t_covergroup_sample_comb_loop.py @@ -0,0 +1,29 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# 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_all') + +# A sample() reached from combinational logic is deliberately left unordered against what it +# samples: calling sample() must not make the calling block behave as if sensitive to what the +# covergroup reads. Unordered is not the same as unsafe -- the access is still recorded for the +# MTask data hazard fixer -- so the run must still be race free. ThreadSanitizer is what checks +# that, and --no-threads-coarsen keeps the sample and the writer of what it samples in separate +# MTasks so there is something for it to catch. +test.enable_tsan() + +# Note on the golden: because these samples are unordered, which value each one observes follows +# from how ordering resolved the loop, not from anything the LRM pins down. The counts are +# reproducible (identical across vlt and vltmt), so they are worth locking down -- but a future +# diff here means the schedule changed, and wants understanding rather than regenerating. +coverage_covergroup_common.run(test, + verilator_flags2=(['--no-threads-coarsen'] if test.vltmt else []), + threads=(2 if test.vltmt else 1)) diff --git a/test_regress/t/t_covergroup_sample_comb_loop.v b/test_regress/t/t_covergroup_sample_comb_loop.v new file mode 100644 index 000000000..d4b03d696 --- /dev/null +++ b/test_regress/t/t_covergroup_sample_comb_loop.v @@ -0,0 +1,85 @@ +// DESCRIPTION: Verilator: Test covergroup sample() called from combinational logic +// A sample() reads what the covergroup holds, whether the covergroup reaches those signals +// through a 'ref' formal or names them directly, and those reads happen as part of the block +// that calls sample(). They must be recorded for the multi-threaded data hazard fixer, but +// they must not be given the consumer edge a combinational read normally gets: that edge is +// what a sensitivity list produces, and the calling block is not sensitive to what the +// covergroup samples. +// Here each calling block is combinational and drives the signal the sampled one is derived +// from, so giving those reads a combinational consumer edge closes a loop through the +// OrderGraph. Nothing before ordering sees such a loop, so it is not broken beforehand, and +// V3Order fails with 'Circular logic when ordering code' rather than coping with it. +// 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 + +module t ( + input clk +); + + int cyc = 0; + + // Sampled through a 'ref' formal, so the sampling block holds no reference to 'ref_gnt' + logic [1:0] ref_req; + logic [1:0] ref_gnt; + + // Sampled by a coverpoint naming it, so the reference is in sample(), not in the block + logic [1:0] dir_req; + logic [1:0] dir_gnt; + + covergroup cg_ref (ref logic [1:0] sig); + cp_ref: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + covergroup cg_direct; + cp_direct: coverpoint dir_gnt { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + cg_ref ref_cg = new(ref_gnt); + cg_direct direct_cg = new; + + // What is sampled, combinationally derived from what the sampling block drives + assign ref_gnt = ref_req ^ 2'b01; + + always_comb begin + dir_gnt = 2'b00; + if (dir_req[0]) dir_gnt = 2'b11; + if (dir_req[1]) dir_gnt = 2'b01; + end + + // Sampling from combinational logic. Neither block is sensitive to what its sample() reads, + // so both loops would be ones ordering introduces rather than ones the source describes. + // What each sample() observes here therefore follows from how ordering resolved the loop -- + // see the .py for what that means for the golden. + always_comb begin + case (cyc[1:0]) + 2'd0: ref_req = 2'b00; + 2'd1: ref_req = 2'b01; + 2'd2: ref_req = 2'b10; + default: ref_req = 2'b11; + endcase + ref_cg.sample(); + end + + always_comb begin + case (cyc[1:0]) + 2'd0: dir_req = 2'b00; + 2'd1: dir_req = 2'b01; + 2'd2: dir_req = 2'b10; + default: dir_req = 2'b11; + endcase + direct_cg.sample(); + end + + always @(posedge clk) begin + cyc <= cyc + 1; + if (cyc == 4) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule diff --git a/test_regress/t/t_covergroup_sample_order.out b/test_regress/t/t_covergroup_sample_order.out new file mode 100644 index 000000000..4b51eb9ff --- /dev/null +++ b/test_regress/t/t_covergroup_sample_order.out @@ -0,0 +1,12 @@ +cg_auto.cp_auto.one: 1 +cg_auto.cp_auto.three: 1 +cg_auto.cp_auto.two: 1 +cg_auto.cp_auto.zero: 2 +cg_derived.cp_derived.one: 2 +cg_derived.cp_derived.three: 1 +cg_derived.cp_derived.two: 1 +cg_derived.cp_derived.zero: 1 +cg_ref.cp_ref.one: 1 +cg_ref.cp_ref.three: 1 +cg_ref.cp_ref.two: 1 +cg_ref.cp_ref.zero: 2 diff --git a/test_regress/t/t_covergroup_sample_order.py b/test_regress/t/t_covergroup_sample_order.py new file mode 100755 index 000000000..716243e5b --- /dev/null +++ b/test_regress/t/t_covergroup_sample_order.py @@ -0,0 +1,23 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# 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_all') + +# An unordered sample() is a genuine data race, but whether it changes the +# histogram in any one run is timing dependent, so use ThreadSanitizer to detect +# it reliably. --no-threads-coarsen keeps the sample and the writer of what it +# samples in separate MTasks. +test.enable_tsan() + +coverage_covergroup_common.run(test, + verilator_flags2=(['--no-threads-coarsen'] if test.vltmt else []), + threads=(2 if test.vltmt else 1)) diff --git a/test_regress/t/t_covergroup_sample_order.v b/test_regress/t/t_covergroup_sample_order.v new file mode 100644 index 000000000..a4f3d8e00 --- /dev/null +++ b/test_regress/t/t_covergroup_sample_order.v @@ -0,0 +1,76 @@ +// DESCRIPTION: Verilator: Test covergroup sample() scheduling against non-blocking writers +// A coverpoint must observe the value its signal had before the sampling edge's non-blocking +// assignments commit, whether sampling is automatic, manual, through a 'ref' formal, or of a +// combinationally derived signal. Runs under --vltmt as well: none of these reads are +// visible at the sample() call site, so each is also a data race if left unordered. +// 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 + +module t ( + input clk +); + + logic [1:0] data; + logic [1:0] refsig; + logic [1:0] derived; + + assign derived = data + 2'b01; + + // Automatic sampling, coverpoint straight on a non-blocking driven signal + covergroup cg_auto @(posedge clk); + cp_auto: coverpoint data { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + // Automatic sampling, coverpoint on a combinationally derived signal + covergroup cg_derived @(posedge clk); + cp_derived: coverpoint derived { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + // Manual sampling, coverpoint reached through a 'ref' formal argument + covergroup cg_ref(ref logic [1:0] sig); + cp_ref: coverpoint sig { + bins zero = {2'b00}; bins one = {2'b01}; bins two = {2'b10}; bins three = {2'b11}; + } + endgroup + + cg_auto auto_cg = new; + cg_derived derived_cg = new; + cg_ref ref_cg = new(refsig); + + int cyc = 0; + + // Manual sample() from its own process, so it is not ordered by being in the writer's block + always @(posedge clk) ref_cg.sample(); + + always @(posedge clk) begin + cyc <= cyc + 1; + + case (cyc) + 0: begin + data <= 2'b00; + refsig <= 2'b00; + end + 1: begin + data <= 2'b01; + refsig <= 2'b01; + end + 2: begin + data <= 2'b10; + refsig <= 2'b10; + end + 3: begin + data <= 2'b11; + refsig <= 2'b11; + end + 4: begin + $write("*-* All Finished *-*\n"); + $finish; + end + endcase + end +endmodule diff --git a/test_regress/t/t_covergroup_static_coverage.py b/test_regress/t/t_covergroup_static_coverage.py index 4c22a684c..7412b8231 100755 --- a/test_regress/t/t_covergroup_static_coverage.py +++ b/test_regress/t/t_covergroup_static_coverage.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_trans_restart.py b/test_regress/t/t_covergroup_trans_restart.py index 6b0f1a9e4..9f6b5465d 100755 --- a/test_regress/t/t_covergroup_trans_restart.py +++ b/test_regress/t/t_covergroup_trans_restart.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test) diff --git a/test_regress/t/t_covergroup_wildcard_bins.py b/test_regress/t/t_covergroup_wildcard_bins.py index 6b0f1a9e4..9f6b5465d 100755 --- a/test_regress/t/t_covergroup_wildcard_bins.py +++ b/test_regress/t/t_covergroup_wildcard_bins.py @@ -10,6 +10,6 @@ import vltest_bootstrap import coverage_covergroup_common -test.scenarios('vlt') +test.scenarios('vlt_all') coverage_covergroup_common.run(test)