From 87d77f01338c0c2af145dc501d48bd90143c5ed4 Mon Sep 17 00:00:00 2001 From: Matthew Ballance Date: Tue, 7 Jul 2026 08:42:39 -0700 Subject: [PATCH 1/6] Fix internal error for coverpoints that reference a covergroup formal parameter (#7853 partial) (#7889) --- src/V3Covergroup.cpp | 55 ++++++++++--------- .../t/t_covergroup_embedded_unsup.out | 8 ++- test_regress/t/t_covergroup_embedded_unsup.v | 34 +++++++++--- 3 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/V3Covergroup.cpp b/src/V3Covergroup.cpp index f243b0c33..90bae71a2 100644 --- a/src/V3Covergroup.cpp +++ b/src/V3Covergroup.cpp @@ -1692,32 +1692,37 @@ class FunctionalCoverageVisitor final : public VNVisitor { } // VISITORS - AstNode* findEnclosingMemberRef(AstClass* cgClassp) { - // An embedded covergroup is lowered into a sibling AstClass that has no handle to - // the enclosing object. A coverpoint/iff/cross expression that references a - // (non-static) member of the enclosing class therefore emits C++ that accesses the - // member as if it were static ("invalid use of non-static data member"). Detect - // such references so the caller can skip lowering with a clean warning instead of - // producing uncompilable code. Returns the first offending node, or nullptr. - // Collect the covergroup class's own member variables (sample/constructor args); - // references to those are legitimate. + AstNode* findUnsupportedCoverpointRef(AstClass* cgClassp) { + // An embedded covergroup is lowered into a sibling AstClass that (currently) has + // no handle to the enclosing object. Identify refs to the containing context + // or a formal param and flag as unsupported std::set ownVars; for (AstNode* itemp = cgClassp->membersp(); itemp; itemp = itemp->nextp()) { if (const AstVar* const varp = VN_CAST(itemp, Var)) ownVars.insert(varp); } + // Flag non-static enclosing-class members reached without a handle as unsupported AstNode* offenderp = nullptr; - const auto scan = [&](AstNode* rootp) { + const auto scanEnclosing = [&](AstNode* rootp) { rootp->foreach([&](AstVarRef* refp) { if (offenderp) return; - const AstVar* const varp = refp->varp(); // Always set post-LinkDot - // A member of another class (the enclosing class) reached with no handle. - // Members of the covergroup class itself (sample/constructor args) are - // legitimate and excluded via ownVars. + const AstVar* const varp = refp->varp(); if (varp->isClassMember() && !ownVars.count(varp)) offenderp = refp; }); }; - for (AstCoverpoint* cpp : m_coverpoints) scan(cpp); - for (AstCoverCross* crossp : m_coverCrosses) scan(crossp); + for (AstCoverpoint* cpp : m_coverpoints) scanEnclosing(cpp); + for (AstCoverCross* crossp : m_coverCrosses) scanEnclosing(crossp); + if (offenderp) return offenderp; + // Flag references to covergroup formal parameters as currently unsupported + const auto scanHandleDeref = [&](AstNode* rootp) { + if (!rootp) return; + rootp->foreach([&](AstMemberSel* selp) { + if (!offenderp) offenderp = selp; + }); + }; + for (AstCoverpoint* cpp : m_coverpoints) { + scanHandleDeref(cpp->exprp()); + scanHandleDeref(cpp->iffp()); + } return offenderp; } @@ -1801,16 +1806,16 @@ class FunctionalCoverageVisitor final : public VNVisitor { iterateChildren(nodep); - // Option B safety net for embedded covergroups: if a coverpoint/iff/cross - // references a member of the enclosing class, lowering would emit uncompilable - // C++ (no handle to the enclosing instance). Skip this covergroup with a clean - // warning rather than crashing the C++ compile. (Full support - an enclosing - // back-pointer - is the planned follow-up.) - if (AstNode* const offenderp = findEnclosingMemberRef(nodep)) { + // Identify embedded covergroup refs to enclosing class members or + // covergroup formal parameters and flag as currently unsupported. + if (AstNode* const offenderp = findUnsupportedCoverpointRef(nodep)) { + const bool viaHandle = VN_IS(offenderp, MemberSel); offenderp->v3warn(COVERIGN, - "Unsupported: 'covergroup' coverpoint referencing enclosing " - "class member; ignoring covergroup " - << nodep->prettyNameQ()); + "Unsupported: 'covergroup' coverpoint " + << (viaHandle ? "dereferencing a class handle member " + "(parameterized covergroup)" + : "referencing enclosing class member") + << "; ignoring covergroup " << nodep->prettyNameQ()); for (AstCoverpoint* cpp : m_coverpoints) { VL_DO_DANGLING(pushDeletep(cpp->unlinkFrBack()), cpp); } diff --git a/test_regress/t/t_covergroup_embedded_unsup.out b/test_regress/t/t_covergroup_embedded_unsup.out index 93cf7a3a5..d74a490cf 100644 --- a/test_regress/t/t_covergroup_embedded_unsup.out +++ b/test_regress/t/t_covergroup_embedded_unsup.out @@ -1,7 +1,11 @@ -%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:27:34: Unsupported: 'covergroup' coverpoint referencing enclosing class member; ignoring covergroup '__vlAnonCG_cov_trans' +%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:23:34: Unsupported: 'covergroup' coverpoint referencing enclosing class member; ignoring covergroup '__vlAnonCG_cov_trans' : ... note: In instance 't' - 27 | trans_start_addr: coverpoint trans_collected.addr {option.auto_bin_max = 16;} + 23 | trans_start_addr: coverpoint trans_collected.addr {option.auto_bin_max = 16;} | ^~~~~~~~~~~~~~~ ... 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. +%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:46:23: Unsupported: 'covergroup' coverpoint dereferencing a class handle member (parameterized covergroup); ignoring covergroup '__vlAnonCG_cov_param' + : ... note: In instance 't' + 46 | cp: coverpoint st.test; + | ^~~~ %Error: Exiting due to diff --git a/test_regress/t/t_covergroup_embedded_unsup.v b/test_regress/t/t_covergroup_embedded_unsup.v index a35ffb840..8a81eb4ab 100644 --- a/test_regress/t/t_covergroup_embedded_unsup.v +++ b/test_regress/t/t_covergroup_embedded_unsup.v @@ -5,13 +5,9 @@ // SPDX-FileCopyrightText: 2026 Wilson Snyder // SPDX-License-Identifier: CC0-1.0 -// Test the graceful-degradation safety net for embedded covergroups (the dominant -// UVM pattern: a covergroup declared inside a class whose coverpoints reference the -// enclosing object's members). Such a covergroup is lowered into a sibling class -// with no handle to the enclosing instance, so emitting it would produce -// uncompilable C++ ("invalid use of non-static data member"). Until the enclosing -// back-pointer feature exists, Verilator must emit a clean COVERIGN warning and skip -// lowering the covergroup, rather than crashing the C++ compile. +// Test that two currently-unsupported coverpoint reference styles are properly flagged +// as COVIGN: references to containing-class members ; references to covergroup formal +// parameters class ubus_transfer; bit [15:0] addr; @@ -35,10 +31,34 @@ class ubus_master_monitor; endfunction endclass +class coverage_state; + bit [3:0] test; + bit [3:0] test2; +endclass + +class parameterized_monitor; + coverage_state cs; + + // Parameterized covergroup: the coverpoints dereference the class-handle argument 'st'. + // Two handle-dereferencing coverpoints ensure the safety net reports only the first + // offender (a second AstMemberSel is seen with the offender already latched). + covergroup cov_param(coverage_state st); + cp: coverpoint st.test; + cp2: coverpoint st.test2; + endgroup + + function new(); + cs = new; + cov_param = new(cs); + endfunction +endclass + module t; ubus_master_monitor m; + parameterized_monitor p; initial begin m = new; + p = new; $write("*-* All Finished *-*\n"); $finish; end From f1a8192a363595cb90929e2e8dbdd3604d7e85d5 Mon Sep 17 00:00:00 2001 From: Yilou Wang Date: Tue, 7 Jul 2026 20:26:19 +0200 Subject: [PATCH 2/6] Fix mixed-width inside and dist range bounds failing randomization (#7875) --- src/V3Randomize.cpp | 61 ++++--- src/V3Width.cpp | 33 +++- .../t/t_constraint_inside_impl_mixwidth.py | 21 +++ .../t/t_constraint_inside_impl_mixwidth.v | 155 ++++++++++++++++++ 4 files changed, 244 insertions(+), 26 deletions(-) create mode 100755 test_regress/t/t_constraint_inside_impl_mixwidth.py create mode 100644 test_regress/t/t_constraint_inside_impl_mixwidth.v diff --git a/src/V3Randomize.cpp b/src/V3Randomize.cpp index 2786ea305..c0c9e1707 100644 --- a/src/V3Randomize.cpp +++ b/src/V3Randomize.cpp @@ -4453,6 +4453,39 @@ class RandomizeVisitor final : public VNVisitor { return new AstConstraintIf{fl, condp, thenBodyp, nullptr}; } + static bool distBoundRefsRandVar(const AstNode* boundp) { + bool found = false; + boundp->foreach([&](const AstVarRef* vrefp) { + if (vrefp->varp()->rand().isRandomizable()) found = true; + }); + return found; + } + + // (distExpr >= lo) && (distExpr <= hi); signed comparisons for signed vars + static AstNodeExpr* newDistRangeMembership(AstDist* distp, const AstInsideRange* irp) { + FileLine* const fl = distp->fileline(); + const bool isSigned = distp->exprp()->isSigned(); + AstNodeExpr* const distExprGtep = distp->exprp()->cloneTreePure(false); + AstNodeExpr* const distExprLtep = distp->exprp()->cloneTreePure(false); + distExprGtep->user1(true); + distExprLtep->user1(true); + AstNodeExpr* const gep + = isSigned ? static_cast( + new AstGteS{fl, distExprGtep, irp->lhsp()->cloneTreePure(false)}) + : static_cast( + new AstGte{fl, distExprGtep, irp->lhsp()->cloneTreePure(false)}); + AstNodeExpr* const lep + = isSigned ? static_cast( + new AstLteS{fl, distExprLtep, irp->rhsp()->cloneTreePure(false)}) + : static_cast( + new AstLte{fl, distExprLtep, irp->rhsp()->cloneTreePure(false)}); + gep->user1(true); + lep->user1(true); + AstNodeExpr* const andp = new AstLogAnd{fl, gep, lep}; + andp->user1(true); + return andp; + } + // Replace AstDist with weighted bucket selection via AstConstraintIf chain. // Supports both constant and variable weight expressions. void lowerDistConstraints(AstTask* taskp, AstNode* constrItemsp, @@ -4577,25 +4610,7 @@ class RandomizeVisitor final : public VNVisitor { for (const auto& bucket : buckets) { AstNodeExpr* memberp; if (const AstInsideRange* const irp = VN_CAST(bucket.rangep, InsideRange)) { - // (distExpr >= lo) && (distExpr <= hi); signed comparisons for signed vars - const bool isSigned = distp->exprp()->isSigned(); - AstNodeExpr* const distExprGtep = distp->exprp()->cloneTreePure(false); - AstNodeExpr* const distExprLtep = distp->exprp()->cloneTreePure(false); - distExprGtep->user1(true); - distExprLtep->user1(true); - AstNodeExpr* const gep - = isSigned ? static_cast(new AstGteS{ - fl, distExprGtep, irp->lhsp()->cloneTreePure(false)}) - : static_cast(new AstGte{ - fl, distExprGtep, irp->lhsp()->cloneTreePure(false)}); - AstNodeExpr* const lep - = isSigned ? static_cast(new AstLteS{ - fl, distExprLtep, irp->rhsp()->cloneTreePure(false)}) - : static_cast(new AstLte{ - fl, distExprLtep, irp->rhsp()->cloneTreePure(false)}); - gep->user1(true); - lep->user1(true); - memberp = new AstLogAnd{fl, gep, lep}; + memberp = newDistRangeMembership(distp, irp); } else { // distExpr == val AstNodeExpr* const distExprCopyp = distp->exprp()->cloneTreePure(false); @@ -4673,7 +4688,13 @@ class RandomizeVisitor final : public VNVisitor { AstNode* chainp = nullptr; for (int i = static_cast(buckets.size()) - 1; i >= 0; --i) { AstNodeExpr* constraintExprp; - if (const AstInsideRange* const irp = VN_CAST(buckets[i].rangep, InsideRange)) { + const AstInsideRange* const irp = VN_CAST(buckets[i].rangep, InsideRange); + if (irp + && (distBoundRefsRandVar(irp->lhsp()) || distBoundRefsRandVar(irp->rhsp()))) { + // Bounds solved concurrently cannot pin a pre-solve value; softly + // prefer the symbolic range so the hard membership stays satisfiable + constraintExprp = newDistRangeMembership(distp, irp); + } else if (irp) { // Pick distExpr = lo + rand64() % (hi - lo + 1) for a uniform value in range AstNodeExpr* const distExprCopyp = distp->exprp()->cloneTreePure(false); distExprCopyp->user1(true); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 802fe155d..5eeaf2205 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3464,9 +3464,11 @@ class WidthVisitor final : public VNVisitor { for (AstNode *nextip, *itemp = nodep->itemsp(); itemp; itemp = nextip) { nextip = itemp->nextp(); itemp = VN_AS(itemp, DistItem)->rangep(); - // InsideRange will get replaced with Lte&Gte and finalized later - if (!VN_IS(itemp, InsideRange)) + if (VN_IS(itemp, InsideRange)) { + userIterate(itemp, WidthVP{subDTypep, FINAL}.p()); + } else { iterateCheck(nodep, "Dist Item", itemp, CONTEXT_DET, FINAL, subDTypep, EXTEND_EXP); + } } // Inside a constraint, V3Randomize handles dist lowering with proper weights, @@ -3541,10 +3543,12 @@ class WidthVisitor final : public VNVisitor { EXTEND_EXP); for (AstNode *nextip, *itemp = nodep->itemsp(); itemp; itemp = nextip) { nextip = itemp->nextp(); // iterate may cause the node to get replaced - // InsideRange will get replaced with Lte&Gte and finalized later - if (!VN_IS(itemp, InsideRange) && !itemp->dtypep()->isNonPackedArray()) + if (VN_IS(itemp, InsideRange)) { + userIterate(itemp, WidthVP{expDTypep, FINAL}.p()); + } else if (!itemp->dtypep()->isNonPackedArray()) { iterateCheck(nodep, "Inside Item", itemp, CONTEXT_DET, FINAL, expDTypep, EXTEND_EXP); + } } AstNodeExpr* exprp; @@ -3636,8 +3640,25 @@ class WidthVisitor final : public VNVisitor { V3Const::constifyEdit(nodep->lhsp()); // lhsp may change V3Const::constifyEdit(nodep->rhsp()); // rhsp may change } else { - userIterateAndNext(nodep->lhsp(), m_vup); - userIterateAndNext(nodep->rhsp(), m_vup); + if (m_vup->prelim()) { + userIterateAndNext(nodep->lhsp(), m_vup); + userIterateAndNext(nodep->rhsp(), m_vup); + } + if (m_vup->final()) { + AstNodeDType* const expDTypep = m_vup->dtypeOverridep(nodep->dtypep()); + // Warning waivers match visit_cmp_eq_gt on the lowered Gte/Lte + const int expWidth = expDTypep->width(); + const bool waiveLhs = expWidth == 32 + && !(expDTypep->isSigned() && nodep->lhsp()->isSigned()) + && expDTypep->widthMin() >= nodep->lhsp()->width(); + const bool waiveRhs = expWidth == 32 + && !(expDTypep->isSigned() && nodep->rhsp()->isSigned()) + && expWidth >= nodep->rhsp()->widthMin(); + iterateCheck(nodep, "Range LHS", nodep->lhsp(), CONTEXT_DET, FINAL, expDTypep, + EXTEND_EXP, !waiveLhs); + iterateCheck(nodep, "Range RHS", nodep->rhsp(), CONTEXT_DET, FINAL, expDTypep, + EXTEND_EXP, !waiveRhs); + } } nodep->dtypeFrom(nodep->lhsp()); } diff --git a/test_regress/t/t_constraint_inside_impl_mixwidth.py b/test_regress/t/t_constraint_inside_impl_mixwidth.py new file mode 100755 index 000000000..db1adb3f9 --- /dev/null +++ b/test_regress/t/t_constraint_inside_impl_mixwidth.py @@ -0,0 +1,21 @@ +#!/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 + +test.scenarios('simulator') + +if not test.have_solver: + test.skip("No constraint solver installed") + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_constraint_inside_impl_mixwidth.v b/test_regress/t/t_constraint_inside_impl_mixwidth.v new file mode 100644 index 000000000..25cd24ca9 --- /dev/null +++ b/test_regress/t/t_constraint_inside_impl_mixwidth.v @@ -0,0 +1,155 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); +// verilog_format: on + +// verilator lint_off WIDTHEXPAND +class Impl; + rand bit [63:0] x, y; + rand bit [31:0] g; + constraint c { + g inside {[1 : 10]}; + y == 64'h100; + y != 0 -> x inside {[y - g : y]}; + } +endclass + +class Neg; // inside under logical-not + rand bit [63:0] x, y; + rand bit [31:0] g; + constraint c { + g inside {[1 : 10]}; + y == 64'h100; + y != 0 -> !(x inside {[y - g : y - 1]}); + } +endclass + +class LAnd; // inside as a logical-and operand + rand bit [63:0] x, y; + rand bit [31:0] g; + constraint c { + g inside {[1 : 10]}; + y == 64'h100; + (x inside {[y - g : y]}) && (x[0] == 1'b0); + } +endclass + +class Nest; // nested implication a -> (b -> inside) + rand bit [63:0] x, y; + rand bit [31:0] g; + rand bit a, b; + constraint c { + g inside {[1 : 10]}; + y == 64'h100; + a == 1; + b == 1; + a -> (b -> x inside {[y - g : y]}); + } +endclass + +class CondCtx; // inside as a ?: condition + rand bit [63:0] x, y; + rand bit [31:0] g; + rand bit s; + constraint c { + g inside {[1 : 10]}; + y == 64'h100; + (x inside {[y - g : y]}) ? (s == 1'b1) : (s == 1'b0); + } +endclass + +class Ctl; // all-32-bit control + rand bit [31:0] x, y, g; + constraint c { + g inside {[1 : 10]}; + y == 32'h100; + y != 0 -> x inside {[y - g : y]}; + } +endclass + +class DistRange; // mixed-width dist range bound + rand bit [63:0] x, y; + rand bit [31:0] g; + constraint c { + g inside {[1 : 10]}; + y == 64'h100; + x dist { + [y - g : y] := 1, + 5 := 1 + }; + } +endclass + +class Bare; // bare narrow variable as a bound + rand bit [63:0] x, y; + rand bit [31:0] g; + constraint c { + g inside {[1 : 10]}; + y == 64'h100; + y != 0 -> x inside {[g : y]}; + } +endclass + +module t; + Impl im; + Neg ng; + LAnd la; + Nest ne; + CondCtx cx; + Ctl ct; + DistRange dr; + Bare br; + int ok; + initial begin + im = new; + ng = new; + la = new; + ne = new; + cx = new; + ct = new; + dr = new; + br = new; + for (int i = 0; i < 20; ++i) begin + ok = im.randomize(); + `checkd(ok, 1); + if (im.x < (64'h100 - im.g) || im.x > 64'h100) `checkd(0, 1); + + ok = ng.randomize(); + `checkd(ok, 1); + if (ng.x >= (64'h100 - ng.g) && ng.x <= 64'hFF) `checkd(0, 1); + + ok = la.randomize(); + `checkd(ok, 1); + if (la.x < (64'h100 - la.g) || la.x > 64'h100 || la.x[0] !== 1'b0) `checkd(0, 1); + + ok = ne.randomize(); + `checkd(ok, 1); + if (ne.x < (64'h100 - ne.g) || ne.x > 64'h100) `checkd(0, 1); + + ok = cx.randomize(); + `checkd(ok, 1); + if (cx.s !== ((cx.x >= (64'h100 - cx.g)) && (cx.x <= 64'h100))) `checkd(0, 1); + + ok = ct.randomize(); + `checkd(ok, 1); + if (ct.x < (32'h100 - ct.g) || ct.x > 32'h100) `checkd(0, 1); + + ok = dr.randomize(); + `checkd(ok, 1); + if (dr.x != 5 && (dr.x < (64'h100 - dr.g) || dr.x > 64'h100)) `checkd(0, 1); + + ok = br.randomize(); + `checkd(ok, 1); + if (br.x < {32'h0, br.g} || br.x > 64'h100) `checkd(0, 1); + end + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule +// verilator lint_on WIDTHEXPAND From e743838d09ef430a936f0f47b8212a96ff47d650 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 7 Jul 2026 20:51:40 +0100 Subject: [PATCH 3/6] Internals: Remove DfgGraph::clone (#7896) Only used by V3DfgBreakCycles, which today either improves the graph, or leaves it unchanged. Remove unnecessary complexity. --- src/V3Dfg.cpp | 174 --------------------------------------- src/V3Dfg.h | 3 - src/V3DfgBreakCycles.cpp | 69 ++++++---------- src/V3DfgContext.h | 4 +- src/V3DfgOptimizer.cpp | 18 ++-- src/V3DfgPasses.h | 11 +-- src/astgen | 24 ------ 7 files changed, 35 insertions(+), 268 deletions(-) diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index 2fd5afb95..b729e841a 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -34,180 +34,6 @@ DfgGraph::~DfgGraph() { forEachVertex([&](DfgVertex& vtx) { vtx.unlinkDelete(*this); }); } -std::unique_ptr DfgGraph::clone() const { - // Create the new graph - DfgGraph* const clonep = new DfgGraph{name()}; - - // Map from original vertex to clone - std::unordered_map vtxp2clonep(size() * 2); - - // Clone constVertices - for (const DfgConst& vtx : m_constVertices) { - DfgConst* const cp = new DfgConst{*clonep, vtx.fileline(), vtx.num()}; - vtxp2clonep.emplace(&vtx, cp); - } - // Clone variable vertices - for (const DfgVertexVar& vtx : m_varVertices) { - const DfgVertexVar* const vp = vtx.as(); - DfgVertexVar* cp = nullptr; - - switch (vtx.type()) { - case VDfgType::VarArray: { - cp = new DfgVarArray{*clonep, vp->vscp()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::VarPacked: { - cp = new DfgVarPacked{*clonep, vp->vscp()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - default: { - vtx.v3fatalSrc("Unhandled variable vertex type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - - if (AstVarScope* const tmpForp = vp->tmpForp()) cp->tmpForp(tmpForp); - } - // Clone ast reference vertices - for (const DfgVertexAst& vtx : m_astVertices) { // LCOV_EXCL_START - switch (vtx.type()) { - case VDfgType::AstRd: { - const DfgAstRd* const vp = vtx.as(); - DfgAstRd* const cp = new DfgAstRd{*clonep, vp->exprp(), vp->inSenItem(), vp->inLoop()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - default: { - vtx.v3fatalSrc("Unhandled ast reference vertex type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - } // LCOV_EXCL_STOP - // Clone operation vertices - for (const DfgVertex& vtx : m_opVertices) { - switch (vtx.type()) { -#include "V3Dfg__gen_clone_cases.h" // From ./astgen - case VDfgType::CReset: { // LCOV_EXCL_START - No algorithm actually hits this today - DfgCReset* const cp = new DfgCReset{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } // LCOV_EXCL_STOP - case VDfgType::MatchMasked: { - DfgMatchMasked* const cp = new DfgMatchMasked{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Sel: { - DfgSel* const cp = new DfgSel{*clonep, vtx.fileline(), vtx.dtype()}; - cp->lsb(vtx.as()->lsb()); - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Rep: { - DfgRep* const cp = new DfgRep{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::UnitArray: { - DfgUnitArray* const cp = new DfgUnitArray{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Mux: { - DfgMux* const cp = new DfgMux{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::SpliceArray: { - DfgSpliceArray* const cp = new DfgSpliceArray{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::SplicePacked: { - DfgSplicePacked* const cp = new DfgSplicePacked{*clonep, vtx.fileline(), vtx.dtype()}; - vtxp2clonep.emplace(&vtx, cp); - break; - } - case VDfgType::Logic: { - vtx.v3fatalSrc("DfgLogic cannot be cloned"); - VL_UNREACHABLE; - break; - } - case VDfgType::Unresolved: { - vtx.v3fatalSrc("DfgUnresolved cannot be cloned"); - VL_UNREACHABLE; - break; - } - case VDfgType::AstRd: // LCOV_EXCL_START - case VDfgType::Const: - case VDfgType::VarArray: - case VDfgType::VarPacked: { - vtx.v3fatalSrc("Vertex should have been handled above: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } // LCOV_EXCL_STOP - } - } - UASSERT(size() == clonep->size(), "Size of clone should be the same"); - - // Constants have no inputs - // Hook up inputs of cloned variables - for (const DfgVertexVar& vtx : m_varVertices) { - DfgVertexVar* const cp = vtxp2clonep.at(&vtx)->as(); - if (const DfgVertex* const srcp = vtx.srcp()) cp->srcp(vtxp2clonep.at(srcp)); - if (const DfgVertex* const defp = vtx.defaultp()) cp->defaultp(vtxp2clonep.at(defp)); - } - // Hook up inputs of cloned ast references - for (const DfgVertexAst& vtx : m_astVertices) { // LCOV_EXCL_START - switch (vtx.type()) { - case VDfgType::AstRd: { - const DfgAstRd* const vp = vtx.as(); - DfgAstRd* const cp = vtxp2clonep.at(&vtx)->as(); - if (const DfgVertex* const srcp = vp->srcp()) cp->srcp(vtxp2clonep.at(srcp)); - break; - } - default: { - vtx.v3fatalSrc("Unhandled DfgVertexAst sub type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - } // LCOV_EXCL_STOP - // Hook up inputs of cloned operation vertices - for (const DfgVertex& vtx : m_opVertices) { - if (vtx.is()) { - switch (vtx.type()) { - case VDfgType::SpliceArray: - case VDfgType::SplicePacked: { - const DfgVertexSplice* const vp = vtx.as(); - DfgVertexSplice* const cp = vtxp2clonep.at(vp)->as(); - vp->foreachDriver([&](const DfgVertex& src, uint32_t lo, FileLine* flp) { - cp->addDriver(vtxp2clonep.at(&src), lo, flp); - return false; - }); - break; - } - default: { - vtx.v3fatalSrc("Unhandled DfgVertexVariadic sub type: " + vtx.typeName()); - VL_UNREACHABLE; - break; - } - } - } else { - DfgVertex* const cp = vtxp2clonep.at(&vtx); - for (size_t i = 0; i < vtx.nInputs(); ++i) { - cp->inputp(i, vtxp2clonep.at(vtx.inputp(i))); - } - } - } - - return std::unique_ptr{clonep}; -} - void DfgGraph::mergeGraphs(std::vector>&& otherps) { if (otherps.empty()) return; diff --git a/src/V3Dfg.h b/src/V3Dfg.h index d80be4d84..c66d2d1a6 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -487,9 +487,6 @@ public: for (const DfgVertex& vtx : m_opVertices) f(vtx); } - // Return an identical, independent copy of this graph. Vertex and edge order might differ. - std::unique_ptr clone() const VL_MT_DISABLED; - // Merge contents of other graphs into this graph. Deletes the other graphs. // DfgVertexVar instances representing the same Ast variable are unified. void mergeGraphs(std::vector>&& otherps) VL_MT_DISABLED; diff --git a/src/V3DfgBreakCycles.cpp b/src/V3DfgBreakCycles.cpp index c1e3b28c5..9839beab7 100644 --- a/src/V3DfgBreakCycles.cpp +++ b/src/V3DfgBreakCycles.cpp @@ -1583,35 +1583,18 @@ public: } }; -std::pair, bool> // -breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { +bool breakCycles(DfgGraph& dfg, V3DfgBreakCyclesContext& ctx) { // Shorthand for dumping graph at given dump level const auto dump = [&](int level, const DfgGraph& dfg, const std::string& name) { if (dumpDfgLevel() >= level) dfg.dumpDotFilePrefixed("breakCycles-" + name); }; - // Can't do much with trivial things ('a = a' or 'a[1] = a[0]'), so bail - if (dfg.size() <= 2) { - UINFO(7, "Graph is trivial"); - dump(9, dfg, "trivial"); - ++ctx.m_breakCyclesContext.m_nTrivial; - return {nullptr, false}; - } - // AstNetlist/AstNodeModule user2 used as sequence numbers for temporaries const VNUser2InUse user2InUse; // Show input for debugging dump(7, dfg, "input"); - // We might fail to make any improvements, so first create a clone of the - // graph. This is what we will be working on, and return if successful. - // Do not touch the input graph. - std::unique_ptr resultp = dfg.clone(); - // Just shorthand for code below - DfgGraph& res = *resultp; - dump(9, res, "clone"); - // How many improvements have we made size_t nImprovements = 0; size_t prevNImprovements; @@ -1619,16 +1602,16 @@ breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { // Iterate while an improvement can be made and the graph is still cyclic do { // Compute SCCs - SccInfo sccInfo{res}; + SccInfo sccInfo{dfg}; // Fix up independent ranges in vertices UINFO(9, "New iteration after " << nImprovements << " improvements"); prevNImprovements = nImprovements; - const size_t nFixed = FixUp::apply(res, sccInfo); + const size_t nFixed = FixUp::apply(dfg, sccInfo); if (nFixed) { nImprovements += nFixed; - ctx.m_breakCyclesContext.m_nImprovements += nFixed; - dump(9, res, "FixUp"); + ctx.m_nImprovements += nFixed; + dump(9, dfg, "FixUp"); } // Validate SccInfo if in debug mode @@ -1637,42 +1620,38 @@ breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { // Congrats if it has become acyclic if (!sccInfo.isCyclic()) { UINFO(7, "Graph became acyclic after " << nImprovements << " improvements"); - dump(7, res, "result-acyclic"); - ++ctx.m_breakCyclesContext.m_nFixed; - return {std::move(resultp), true}; + dump(7, dfg, "result-acyclic"); + ++ctx.m_nFixed; + return true; } } while (nImprovements != prevNImprovements); + // Debug dump if (dumpDfgLevel() >= 9) { - const SccInfo sccInfo{res}; - res.dumpDotFilePrefixed("breakCycles-remaining", [&](const DfgVertex& vtx) { + const SccInfo sccInfo{dfg}; + dfg.dumpDotFilePrefixed("breakCycles-remaining", [&](const DfgVertex& vtx) { return sccInfo.get(vtx); // }); } - // If an improvement was made, return the still cyclic improved graph + // Accounting if (nImprovements) { UINFO(7, "Graph was improved " << nImprovements << " times"); - dump(7, res, "result-improved"); - ++ctx.m_breakCyclesContext.m_nImproved; - return {std::move(resultp), false}; + dump(7, dfg, "result-improved"); + ++ctx.m_nImproved; + } else { + UINFO(7, "Graph NOT improved"); + dump(7, dfg, "result-original"); + ++ctx.m_nUnchanged; } - - // No improvement was made - UINFO(7, "Graph NOT improved"); - dump(7, res, "result-original"); - ++ctx.m_breakCyclesContext.m_nUnchanged; - return {nullptr, false}; + return false; } } //namespace V3DfgBreakCycles -std::pair, bool> // -V3DfgPasses::breakCycles(const DfgGraph& dfg, V3DfgContext& ctx) { - auto pair = V3DfgBreakCycles::breakCycles(dfg, ctx); - if (pair.first) { - if (v3Global.opt.debugCheck()) V3DfgPasses::typeCheck(*pair.first); - V3DfgPasses::removeUnused(*pair.first); - } - return pair; +bool V3DfgPasses::breakCycles(DfgGraph& dfg, V3DfgContext& ctx) { + const bool res = V3DfgBreakCycles::breakCycles(dfg, ctx.m_breakCyclesContext); + if (v3Global.opt.debugCheck()) V3DfgPasses::typeCheck(dfg); + V3DfgPasses::removeUnused(dfg); + return res; } diff --git a/src/V3DfgContext.h b/src/V3DfgContext.h index 21e7af31a..ee34df5f6 100644 --- a/src/V3DfgContext.h +++ b/src/V3DfgContext.h @@ -100,9 +100,8 @@ class V3DfgBreakCyclesContext final : public V3DfgSubContext { public: // STATE VDouble0 m_nFixed; // Number of graphs that became acyclic - VDouble0 m_nImproved; // Number of graphs that were imporoved but still cyclic + VDouble0 m_nImproved; // Number of graphs that were improved but still cyclic VDouble0 m_nUnchanged; // Number of graphs that were left unchanged - VDouble0 m_nTrivial; // Number of graphs that were not changed VDouble0 m_nImprovements; // Number of changes made to graphs private: @@ -112,7 +111,6 @@ private: addStat("made acyclic", m_nFixed); addStat("improved", m_nImproved); addStat("left unchanged", m_nUnchanged); - addStat("trivial", m_nTrivial); addStat("changes applied", m_nImprovements); } }; diff --git a/src/V3DfgOptimizer.cpp b/src/V3DfgOptimizer.cpp index 07002049d..f18672598 100644 --- a/src/V3DfgOptimizer.cpp +++ b/src/V3DfgOptimizer.cpp @@ -126,19 +126,15 @@ class DataflowOptimize final { std::vector> madeAcyclicComponents; if (v3Global.opt.fDfgBreakCycles()) { for (auto it = cyclicComps.begin(); it != cyclicComps.end();) { - auto result = V3DfgPasses::breakCycles(**it, m_ctx); - if (!result.first) { - // No improvement, moving on. + const bool madeAcyclic = V3DfgPasses::breakCycles(**it, m_ctx); + // If not made acyclic, keep it in 'cyclicComps' + if (!madeAcyclic) { ++it; - } else if (!result.second) { - // Improved, but still cyclic. Replace the original cyclic component. - *it = std::move(result.first); - ++it; - } else { - // Result became acyclic. Move to madeAcyclicComponents, delete original. - madeAcyclicComponents.emplace_back(std::move(result.first)); - it = cyclicComps.erase(it); + continue; } + // Otherwise move to 'madeAcyclicComponents' + madeAcyclicComponents.emplace_back(std::move(*it)); + it = cyclicComps.erase(it); } } // Merge those that were made acyclic back to the graph, this enables optimizing more diff --git a/src/V3DfgPasses.h b/src/V3DfgPasses.h index d0d8e62d8..3392f0703 100644 --- a/src/V3DfgPasses.h +++ b/src/V3DfgPasses.h @@ -44,14 +44,9 @@ void synthesize(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; // Remove redundant selects void removeSelects(DfgGraph& dfg, V3DfgRemoveSelectsContext& ctx) VL_MT_DISABLED; // Attempt to make the given cyclic graph into an acyclic, or "less cyclic" -// equivalent. If the returned pointer is null, then no improvement was -// possible on the input graph. Otherwise the returned graph is an improvement -// on the input graph, with at least some cycles eliminated. The returned -// graph is always independent of the original. If an imporoved graph is -// returned, then the returned 'bool' flag indicated if the returned graph is -// acyclic (flag 'true'), or still cyclic (flag 'false'). -std::pair, bool> // -breakCycles(const DfgGraph&, V3DfgContext&) VL_MT_DISABLED; +// equivalent. Genuine combinational cycles can exist, so this might be +// unsuccessful. Returns true if the graph became acyclic, false otherwise. +bool breakCycles(DfgGraph&, V3DfgContext&) VL_MT_DISABLED; // Construct binary to oneHot decoders void binToOneHot(DfgGraph&, V3DfgBinToOneHotContext&) VL_MT_DISABLED; // Common subexpression elimination diff --git a/src/astgen b/src/astgen index 75f68d5fb..7359833ff 100755 --- a/src/astgen +++ b/src/astgen @@ -1279,29 +1279,6 @@ def write_dfg_auto_classes(filename): fh.write("\n") -def write_dfg_clone_cases(filename): - with open_file(filename) as fh: - - def emitBlock(pattern, **fmt): - fh.write(textwrap.dedent(pattern).format(**fmt)) - - for node in DfgVertexList: - # Only generate code for automatically derived leaf nodes - if (node.file is not None) or not node.isLeaf: - continue - - emitBlock('''\ - case VDfgType::{t}: {{ - Dfg{t}* const cp = new Dfg{t}{{*clonep, vtx.fileline(), vtx.dtype()}}; - vtxp2clonep.emplace(&vtx, cp); - break; - }} - ''', - t=node.name, - s=node.superClass.name) - fh.write("\n") - - def write_dfg_ast_to_dfg(filename): with open_file(filename) as fh: for node in DfgVertexList: @@ -1499,7 +1476,6 @@ if Args.classes: write_type_tests("Dfg", DfgVertexList) write_dfg_macros("V3Dfg__gen_macros.h") write_dfg_auto_classes("V3Dfg__gen_auto_classes.h") - write_dfg_clone_cases("V3Dfg__gen_clone_cases.h") write_dfg_ast_to_dfg("V3Dfg__gen_ast_to_dfg.h") write_dfg_dfg_to_ast("V3Dfg__gen_dfg_to_ast.h") From 6c20fdb7bd94db311f3055583c7bb7c74d287ba3 Mon Sep 17 00:00:00 2001 From: Artur Bieniek Date: Wed, 8 Jul 2026 01:26:50 +0200 Subject: [PATCH 4/6] Optimize assertion NFAs using bit-vector ring buffers (#7885) Signed-off-by: Artur Bieniek --- src/V3AssertNfa.cpp | 300 +++++++++++------- src/V3Subst.cpp | 1 + test_regress/t/t_property_delay_large.py | 21 ++ test_regress/t/t_property_delay_large.v | 73 +++++ test_regress/t/t_property_sexpr_multi.py | 2 +- test_regress/t/t_property_sexpr_range_delay.v | 3 + 6 files changed, 283 insertions(+), 117 deletions(-) create mode 100755 test_regress/t/t_property_delay_large.py create mode 100644 test_regress/t/t_property_delay_large.v diff --git a/src/V3AssertNfa.cpp b/src/V3AssertNfa.cpp index b96696cff..552eabd48 100644 --- a/src/V3AssertNfa.cpp +++ b/src/V3AssertNfa.cpp @@ -48,8 +48,8 @@ class SvaStateVertex; // Per-vertex algorithm data, stored via V3GraphVertex::userp() during lowering struct SvaVertexData final { AstVar* stateVarp = nullptr; // NBA state register for this vertex - AstVar* counterActiveVarp = nullptr; // Counter FSM active flag - AstVar* counterCountVarp = nullptr; // Counter FSM count register + AstVar* delayRingVarp = nullptr; // Bitset ring buffer + AstVar* delayRingIdxVarp = nullptr; // Next slot written in delayRingVarp AstVar* doneLVarp = nullptr; // SAnd LHS done-latch AstVar* doneRVarp = nullptr; // SAnd RHS done-latch AstNodeExpr* stateSigp = nullptr; // Combinational state signal (owned during lowering) @@ -64,10 +64,10 @@ public: bool m_isMatch = false; // Owned throughout-guard condition clones; IEEE 1800-2023 16.9.9 std::vector m_throughoutConds; - // Counter FSM vertex for ##[M:N] when N-M > kChainLimit - bool m_isCounter = false; - int m_counterMax - = 0; // Counter window maximum (min is always 0; pre-chain handles the M offset) + // Nonzero for a bitset ring-buffer vertex for ## delays. + bool m_isFixedDelayRing = false; + int m_delayRingSize = 0; // Fixed delay cycles. Range: max-min+1. + AstNodeExpr* m_delayRingClearCondp = nullptr; // local RHS for pure-boolean range // Liveness terminal (IEEE weak semantics): reject must not fire from this source bool m_isUnbounded = false; // Temporal sequence AND combiner; IEEE 1800-2023 16.9.5 @@ -88,15 +88,25 @@ public: : V3GraphVertex{graphp} {} ~SvaStateVertex() override { for (AstNodeExpr* cp : m_throughoutConds) VL_DO_DANGLING(cp->deleteTree(), cp); + if (m_delayRingClearCondp) + VL_DO_DANGLING(m_delayRingClearCondp->deleteTree(), m_delayRingClearCondp); if (m_andLhsCondp) VL_DO_DANGLING(m_andLhsCondp->deleteTree(), m_andLhsCondp); if (m_andRhsCondp) VL_DO_DANGLING(m_andRhsCondp->deleteTree(), m_andRhsCondp); } // METHODS - string name() const override { return "s" + cvtToStr(color()); } // LCOV_EXCL_LINE // LCOV_EXCL_START -- Graphviz dump only + string name() const override { + string name = "s" + cvtToStr(color()); + if (m_delayRingSize) { + name += "\\n"; + name += m_isFixedDelayRing ? "fixed chain " : "range chain "; + name += cvtToStr(m_delayRingSize) + " bits"; + } + return name; + } string dotColor() const override { if (m_isMatch) return "red"; - if (m_isCounter) return "blue"; + if (m_delayRingSize) return "blue"; if (m_isAndCombiner) return "purple"; return "black"; } @@ -524,14 +534,28 @@ class SvaNfaBuilder final { return m_graph.addClockedEdge(fromp, top, throughoutCond(nullptr, flp)); } - SvaStateVertex* addDelayChain(SvaStateVertex* startp, int n, FileLine* flp) { - SvaStateVertex* currentp = startp; - for (int i = 0; i < n; ++i) { + SvaStateVertex* addDelayChain(SvaStateVertex* startp, int size, FileLine* flp, + bool isFixed = true, AstNodeExpr* clearCondp = nullptr) { + if (isFixed && size == 0) return startp; + UASSERT_OBJ(size > 0, startp, "Delay chain needs at least one slot"); + if (isFixed && size == 1) { SvaStateVertex* const nextp = scopedCreateVertex(); - guardedEdge(currentp, nextp, flp); - currentp = nextp; + guardedEdge(startp, nextp, flp); + return nextp; } - return currentp; + SvaStateVertex* const ringVtxp = scopedCreateVertex(); + ringVtxp->m_isFixedDelayRing = isFixed; + ringVtxp->m_delayRingSize = size; + if (clearCondp) { + UASSERT_OBJ(!isFixed, startp, "Fixed delay cannot have a clear condition"); + ringVtxp->m_delayRingClearCondp = clearCondp->cloneTreePure(false); + } + if (isFixed) { + guardedEdge(startp, ringVtxp, flp); + } else { + guardedLink(startp, ringVtxp, flp); + } + return ringVtxp; } // Build NFA for an SExpr. finalCond = RHS (not yet added as a vertex). @@ -576,7 +600,7 @@ class SvaNfaBuilder final { const int range = maxDelay - minDelay; currentp = addDelayChain(currentp, minDelay, flp); // kChainLimit bounds per-attempt unrolled vertices. Above this, a - // counter FSM (constant-size state) is used instead, so the vertex + // ring buffer (constant-size state) is used instead, so the vertex // count is O(1) in range regardless of user input; no adversarial N // blowup is possible. constexpr int kChainLimit = 256; @@ -590,13 +614,8 @@ class SvaNfaBuilder final { return false; } if (range > kChainLimit) { - // Large range: counter FSM. Overlapping triggers during an active - // count are dropped (non-overlapping semantics only). - SvaStateVertex* const counterVtxp = scopedCreateVertex(); - counterVtxp->m_isCounter = true; - counterVtxp->m_counterMax = range; - guardedEdge(currentp, counterVtxp, flp); - currentp = counterVtxp; + currentp = addDelayChain(currentp, range + 1, flp, false, + rhsExprp->isMultiCycleSva() ? nullptr : rhsExprp); } else if (VN_IS(rhsExprp, SExpr)) { // Nested-SExpr RHS: merge all [M,N] positions. Candidate-local misses // are not assertion rejects while a later position can still match. @@ -1649,6 +1668,25 @@ class SvaNfaLowering final { if (!exprp) return nullptr; return new AstLogAnd{c.flp, exprp, notKillActive(c)}; } + static AstNodeExpr* nextRingIndex(FileLine* flp, AstVar* idxp, uint32_t size) { + const auto u32Const = [flp](uint32_t value) { + return new AstConst{flp, AstConst::WidthedValue{}, 32, value}; + }; + UASSERT(size > 1, "Delay ring index needs at least two slots"); + // idx == size - 1 ? 0 : idx + 1 + AstAdd* const addp = new AstAdd{flp, new AstVarRef{flp, idxp, VAccess::READ}, u32Const(1)}; + addp->dtypeFrom(idxp); + AstCond* const condp = new AstCond{ + flp, new AstEq{flp, new AstVarRef{flp, idxp, VAccess::READ}, u32Const(size - 1)}, + u32Const(0), addp}; + condp->dtypeFrom(idxp); + return condp; + } + static AstNodeExpr* delayRingBit(FileLine* flp, AstVar* ringp, AstNodeExpr* idxExprp, + VAccess access = VAccess::READ) { + // ring[idx] + return new AstSel{flp, new AstVarRef{flp, ringp, access}, idxExprp, 1}; + } // Phase 3 output signals struct SignalSet final { @@ -1659,12 +1697,14 @@ class SvaNfaLowering final { }; // Phase 2/2b/2c: Emit NBA state-update always blocks for registered vertices, - // counter FSMs, and SAnd combiner done-latches. + // delay rings, and SAnd combiner done-latches. // Phase 2: State register NBA always block. Each clocked-edge target // latches the OR of its incoming contributions. void emitStateRegisterNba(LowerCtx& c) { AstNode* bodyp = nullptr; + bool hasDelayRing = false; for (int i = 0; i < c.N; ++i) { + if (c.vtx[i]->datap()->delayRingVarp) hasDelayRing = true; if (!c.vtx[i]->datap()->stateVarp) continue; AstNodeExpr* nextStatep = nullptr; @@ -1693,100 +1733,104 @@ class SvaNfaLowering final { AstAssignDly* const assignp = new AstAssignDly{ c.flp, new AstVarRef{c.flp, c.vtx[i]->datap()->stateVarp, VAccess::WRITE}, nextStatep}; - if (!bodyp) { - bodyp = assignp; - } else { - bodyp->addNext(assignp); - } + bodyp = AstNode::addNextNull(bodyp, assignp); } - if (!bodyp) return; // Capture disableCnt in Phase-2 NBA before any reactive re-evaluation. // snapshotVarp and disableCntVarp are allocated together. - if (c.snapshotVarp) { + if (c.snapshotVarp && (bodyp || hasDelayRing)) { UASSERT_OBJ(c.disableCntVarp, c.senTreep, "snapshotVarp set without disableCntVarp"); - bodyp->addNext( - new AstAssignDly{c.flp, new AstVarRef{c.flp, c.snapshotVarp, VAccess::WRITE}, - new AstVarRef{c.flp, c.disableCntVarp, VAccess::READ}}); + // disable_snapshot <= disable_count; + AstAssignDly* const snapshotp + = new AstAssignDly{c.flp, new AstVarRef{c.flp, c.snapshotVarp, VAccess::WRITE}, + new AstVarRef{c.flp, c.disableCntVarp, VAccess::READ}}; + bodyp = AstNode::addNextNull(bodyp, snapshotp); } + if (!bodyp) return; m_modp->addStmtsp( new AstAlways{c.flp, VAlwaysKwd::ALWAYS, c.senTreep->cloneTree(false), bodyp}); } - // Phase 2b: Counter FSM always block. - // if (active) { if (done) active<=0; else counter<=counter+1; } - // else if (incoming) { active<=1; counter<=0; } - void emitCounterFsmNba(LowerCtx& c) { - for (int ci = 0; ci < c.N; ++ci) { - if (!c.vtx[ci]->datap()->counterActiveVarp) continue; - AstVar* const activep = c.vtx[ci]->datap()->counterActiveVarp; - AstVar* const cntp = c.vtx[ci]->datap()->counterCountVarp; - const uint32_t counterMax = static_cast(c.vtx[ci]->m_counterMax); + // Phase 2b: Bitset ring-buffer delay always block. + void emitDelayRingNba(LowerCtx& c) { + for (int ri = 0; ri < c.N; ++ri) { + SvaStateVertex* const vtxp = c.vtx[ri]; + if (!vtxp->datap()->delayRingVarp) continue; + AstVar* const ringp = vtxp->datap()->delayRingVarp; + AstVar* const idxp = vtxp->datap()->delayRingIdxVarp; + const uint32_t size = static_cast(vtxp->m_delayRingSize); - // Builder only adds clocked edges to counter vertices (guardedEdge - // in buildSExpr), so m_consumesCycle is always true here. AstNodeExpr* incomingp = nullptr; for (const SvaTransEdge* const tep : c.edges) { - const int toIdx = tep->toVtxp()->color(); - if (toIdx != ci) continue; + if (static_cast(tep->toVtxp()->color()) != ri) continue; + UASSERT_OBJ(tep->m_consumesCycle == vtxp->m_isFixedDelayRing, vtxp, + "Delay-ring incoming edge kind mismatch"); const int fi = tep->fromVtxp()->color(); UASSERT_OBJ(c.vtx[fi]->datap()->stateSigp, c.vtx[fi], - "Clocked edge source missing stateSig"); + "Delay-ring incoming source missing stateSig"); AstNodeExpr* contribp = c.vtx[fi]->datap()->stateSigp->cloneTreePure(false); contribp = andCond(c.flp, contribp, tep->m_condp); - if (c.disableExprp) { + if (c.disableExprp && !c.snapshotVarp) { AstNodeExpr* const notDisp = new AstLogNot{c.flp, c.disableExprp->cloneTreePure(false)}; contribp = new AstLogAnd{c.flp, contribp, notDisp}; } incomingp = orExprs(c.flp, incomingp, contribp); } - UASSERT_OBJ(incomingp, c.vtx[ci], "Counter vertex has no incoming contribution"); - - // Counter window is always [0, m_counterMax]; M offset is handled by - // the pre-chain in buildSExpr, so every tick inside an active count - // is in-window. - AstNodeExpr* inWindowp = new AstConst{c.flp, AstConst::BitTrue{}}; - AstNodeExpr* matchedNowp = nullptr; - if (c.matchCondp) { - matchedNowp - = new AstLogAnd{c.flp, inWindowp, sampled(c.matchCondp->cloneTreePure(false))}; - } else { // LCOV_EXCL_LINE -- no counter-FSM caller leaves matchCondp null - matchedNowp = inWindowp; // LCOV_EXCL_LINE + UASSERT_OBJ(incomingp, vtxp, "Delay ring has no incoming edge"); + AstNode* updateBodyp = nullptr; + if (vtxp->m_isFixedDelayRing) { + // ring[idx] <= incoming; + updateBodyp = new AstAssignDly{ + c.flp, + delayRingBit(c.flp, ringp, new AstVarRef{c.flp, idxp, VAccess::READ}, + VAccess::WRITE), + incomingp}; + } else { + // ring[next_idx] <= 1'b0; ring[idx] <= incoming; + AstAssignDly* const clearExpirep = new AstAssignDly{ + c.flp, + delayRingBit(c.flp, ringp, nextRingIndex(c.flp, idxp, size), VAccess::WRITE), + new AstConst{c.flp, AstConst::BitFalse{}}}; + AstAssignDly* const writeIncomingp = new AstAssignDly{ + c.flp, + delayRingBit(c.flp, ringp, new AstVarRef{c.flp, idxp, VAccess::READ}, + VAccess::WRITE), + incomingp}; + clearExpirep->addNext(writeIncomingp); + updateBodyp = clearExpirep; } - AstNodeExpr* const counterAtEndp - = new AstEq{c.flp, new AstVarRef{c.flp, cntp, VAccess::READ}, - new AstConst{c.flp, AstConst::WidthedValue{}, 32, counterMax}}; + AstNodeExpr* clearCondp = nullptr; + if (vtxp->m_delayRingClearCondp) { + clearCondp = sampled(vtxp->m_delayRingClearCondp->cloneTreePure(false)); + } + if (c.disableExprp && !c.snapshotVarp) { + clearCondp = orExprs(c.flp, clearCondp, c.disableExprp->cloneTreePure(false)); + } + AstNodeExpr* guardp = nullptr; + for (AstNodeExpr* const cp : vtxp->m_throughoutConds) { + AstNodeExpr* const sampledp = sampled(cp->cloneTreePure(false)); + guardp = guardp ? static_cast(new AstLogAnd{c.flp, guardp, sampledp}) + : sampledp; + } + if (guardp) clearCondp = orExprs(c.flp, clearCondp, new AstLogNot{c.flp, guardp}); + if (clearCondp) { + // if (clear) ring <= '0; + AstConst* const zerop = new AstConst{c.flp, AstConst::DTyped{}, ringp->dtypep()}; + zerop->num().setAllBits0(); + updateBodyp = new AstIf{ + c.flp, clearCondp, + new AstAssignDly{c.flp, new AstVarRef{c.flp, ringp, VAccess::WRITE}, zerop}, + updateBodyp}; + } + // idx <= next_idx; + updateBodyp->addNext(new AstAssignDly{c.flp, + new AstVarRef{c.flp, idxp, VAccess::WRITE}, + nextRingIndex(c.flp, idxp, size)}); - AstNodeExpr* const donep = new AstLogOr{ - c.flp, killActive(c), new AstLogOr{c.flp, matchedNowp, counterAtEndp}}; - - AstAssignDly* const clearActivep - = new AstAssignDly{c.flp, new AstVarRef{c.flp, activep, VAccess::WRITE}, - new AstConst{c.flp, AstConst::BitFalse{}}}; - AstAdd* const addExprp - = new AstAdd{c.flp, new AstVarRef{c.flp, cntp, VAccess::READ}, - new AstConst{c.flp, AstConst::WidthedValue{}, 32, 1u}}; - addExprp->dtypeFrom(cntp); - AstAssignDly* const incCountp - = new AstAssignDly{c.flp, new AstVarRef{c.flp, cntp, VAccess::WRITE}, addExprp}; - AstIf* const doneIfp = new AstIf{c.flp, donep, clearActivep, incCountp}; - - AstAssignDly* const setActivep - = new AstAssignDly{c.flp, new AstVarRef{c.flp, activep, VAccess::WRITE}, - new AstConst{c.flp, AstConst::BitTrue{}}}; - AstAssignDly* const resetCountp - = new AstAssignDly{c.flp, new AstVarRef{c.flp, cntp, VAccess::WRITE}, - new AstConst{c.flp, AstConst::WidthedValue{}, 32, 0u}}; - setActivep->addNext(resetCountp); - AstIf* const startIfp - = new AstIf{c.flp, gateNotKill(c, incomingp), setActivep, nullptr}; - AstIf* const topIfp = new AstIf{c.flp, new AstVarRef{c.flp, activep, VAccess::READ}, - doneIfp, startIfp}; - - m_modp->addStmtsp( - new AstAlways{c.flp, VAlwaysKwd::ALWAYS, c.senTreep->cloneTree(false), topIfp}); + m_modp->addStmtsp(new AstAlways{c.flp, VAlwaysKwd::ALWAYS, + c.senTreep->cloneTree(false), updateBodyp}); } } @@ -1889,16 +1933,22 @@ class SvaNfaLowering final { outPerMidSrcsp->push_back(perMidp); } - if (tep->fromVtxp()->m_isCounter) { + if (tep->fromVtxp()->m_delayRingSize && !tep->fromVtxp()->m_isFixedDelayRing) { sigs.terminalActivep = orExprs(c.flp, sigs.terminalActivep, srcSigp->cloneTreePure(false)); - AstNodeExpr* const atEndp = new AstEq{ - c.flp, - new AstVarRef{c.flp, c.vtx[fi]->datap()->counterCountVarp, VAccess::READ}, - new AstConst{c.flp, AstConst::WidthedValue{}, 32, - static_cast(tep->fromVtxp()->m_counterMax)}}; - AstNodeExpr* const expireContribp = new AstLogAnd{c.flp, srcSigp, atEndp}; + AstVar* const ringp = c.vtx[fi]->datap()->delayRingVarp; + AstVar* const idxp = c.vtx[fi]->datap()->delayRingIdxVarp; + const uint32_t size = static_cast(c.vtx[fi]->m_delayRingSize); + // reject |= ring[next_idx] && final_condition; + AstNodeExpr* expireContribp + = delayRingBit(c.flp, ringp, nextRingIndex(c.flp, idxp, size)); + expireContribp = andCond(c.flp, expireContribp, tep->m_condp); + if (snapshotOkp) { + expireContribp + = new AstLogAnd{c.flp, expireContribp, snapshotOkp->cloneTreePure(false)}; + } sigs.rejectBasep = orExprs(c.flp, sigs.rejectBasep, expireContribp); + VL_DO_DANGLING(srcSigp->deleteTree(), srcSigp); } else if (tep->fromVtxp()->m_isUnbounded || tep->fromVtxp()->m_isAndCombiner) { sigs.terminalActivep = orExprs(c.flp, sigs.terminalActivep, srcSigp); } else { @@ -1922,6 +1972,10 @@ class SvaNfaLowering final { AstNodeExpr* stateExprp = nullptr; if (c.vtx[i]->datap()->stateVarp) { stateExprp = new AstVarRef{c.flp, c.vtx[i]->datap()->stateVarp, VAccess::READ}; + } else if (c.vtx[i]->datap()->delayRingVarp && c.vtx[i]->m_isFixedDelayRing) { + // fixed_chain_active = |ring; + stateExprp = new AstRedOr{ + c.flp, new AstVarRef{c.flp, c.vtx[i]->datap()->delayRingVarp, VAccess::READ}}; } else { UASSERT_OBJ(c.vtx[i]->datap()->stateSigp, c.vtx[i], "Throughout-conds vertex missing state representation"); @@ -2036,10 +2090,18 @@ class SvaNfaLowering final { if (c.vtx[i]->datap()->stateVarp) { c.vtx[i]->datap()->stateSigp = new AstVarRef{c.flp, c.vtx[i]->datap()->stateVarp, VAccess::READ}; - } else if (c.vtx[i]->datap()->counterActiveVarp) { - // Counter window is always [0, m_counterMax]; see buildSExpr. - c.vtx[i]->datap()->stateSigp - = new AstVarRef{c.flp, c.vtx[i]->datap()->counterActiveVarp, VAccess::READ}; + } else if (c.vtx[i]->datap()->delayRingVarp) { + if (c.vtx[i]->m_isFixedDelayRing) { + // state = ring[idx]; + c.vtx[i]->datap()->stateSigp = delayRingBit( + c.flp, c.vtx[i]->datap()->delayRingVarp, + new AstVarRef{c.flp, c.vtx[i]->datap()->delayRingIdxVarp, VAccess::READ}); + } else { + // state = |ring; + c.vtx[i]->datap()->stateSigp = new AstRedOr{ + c.flp, + new AstVarRef{c.flp, c.vtx[i]->datap()->delayRingVarp, VAccess::READ}}; + } } } // Fixed-point propagation along zero-delay (Link) edges. @@ -2256,18 +2318,22 @@ public: vtx[i]->datap()->doneRVarp = rp; continue; } - if (vtx[i]->m_isCounter) { - const std::string base = baseName + "__c" + std::to_string(i); - AstVar* const activep = new AstVar{flp, VVarType::MODULETEMP, base + "_active", - m_modp->findBitDType()}; - activep->lifetime(VLifetime::STATIC_EXPLICIT); - m_modp->addStmtsp(activep); - vtx[i]->datap()->counterActiveVarp = activep; - AstVar* const cntp - = new AstVar{flp, VVarType::MODULETEMP, base + "_cnt", u32DTypep}; - cntp->lifetime(VLifetime::STATIC_EXPLICIT); - m_modp->addStmtsp(cntp); - vtx[i]->datap()->counterCountVarp = cntp; + if (vtx[i]->m_delayRingSize) { + const std::string base = baseName + "__d" + std::to_string(i); + // bit [size-1:0] ring; + AstNodeDType* const ringDTypep = m_modp->findBitDType( + vtx[i]->m_delayRingSize, vtx[i]->m_delayRingSize, VSigning::UNSIGNED); + AstVar* const ringp + = new AstVar{flp, VVarType::MODULETEMP, base + "_ring", ringDTypep}; + ringp->lifetime(VLifetime::STATIC_EXPLICIT); + m_modp->addStmtsp(ringp); + vtx[i]->datap()->delayRingVarp = ringp; + // int unsigned idx; + AstVar* const idxp + = new AstVar{flp, VVarType::MODULETEMP, base + "_idx", u32DTypep}; + idxp->lifetime(VLifetime::STATIC_EXPLICIT); + m_modp->addStmtsp(idxp); + vtx[i]->datap()->delayRingIdxVarp = idxp; continue; } if (!vtx[i]->datap()->needsReg) continue; @@ -2288,9 +2354,9 @@ public: // Phase 1: Resolve combinational Links via fixed-point propagation. resolveLinks(c, triggerExprp); - // Phase 2/2b/2c: Emit NBA state-update, counter FSM, and SAnd done-latch logic. + // Phase 2/2b/2c: Emit NBA state-update, delay-ring, and SAnd done-latch logic. emitStateRegisterNba(c); - emitCounterFsmNba(c); + emitDelayRingNba(c); emitAndCombinerDoneLatchNba(c); emitKillAckNba(c); @@ -3014,6 +3080,8 @@ class AssertNfaVisitor final : public VNVisitor { } UINFO(4, "NFA converted assertion at " << flp << endl); + + if (dumpGraphLevel() >= 6) graph.m_graph.dumpDotFilePrefixed("assert-nfa"); } // VISITORS diff --git a/src/V3Subst.cpp b/src/V3Subst.cpp index bdc62e5d5..8e995dfc6 100644 --- a/src/V3Subst.cpp +++ b/src/V3Subst.cpp @@ -183,6 +183,7 @@ class SubstValidVisitor final : public VNVisitorConst { } void visit(AstConst*) override {} // Accelerate + void visit(AstText*) override {} // CExpr/CStmt literal text has no variable dependencies void visit(AstNodeExpr* nodep) override { if (!m_valid) return; diff --git a/test_regress/t/t_property_delay_large.py b/test_regress/t/t_property_delay_large.py new file mode 100755 index 000000000..91edb539c --- /dev/null +++ b/test_regress/t/t_property_delay_large.py @@ -0,0 +1,21 @@ +#!/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 + +test.scenarios('simulator') + +test.sim_time = 2700 + +test.compile(timing_loop=True, + verilator_flags2=['--assert', '--timing', '--coverage-user', '--dumpi-graph', '6']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_property_delay_large.v b/test_regress/t/t_property_delay_large.v new file mode 100644 index 000000000..6e6752f42 --- /dev/null +++ b/test_regress/t/t_property_delay_large.v @@ -0,0 +1,73 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkd(gotv, expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0) +`define checkp(gotv,expv_s) do begin string gotv_s; gotv_s = $sformatf("%p", gotv); if ((gotv_s) != (expv_s)) begin $write("%%Error: %s:%0d: got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv_s), (expv_s)); `stop; end end while(0); +// verilog_format: on + +module t ( + input clk +); + localparam int BIG_DELAY = 1024; + localparam int END_CYC = BIG_DELAY + 300; + + int cyc = 0; + int fixed_pass_q[$]; + int fixed_fail_q[$]; + int range_pass_q[$]; + int range_fail_q[$]; + + // Fixed delay: starts 1..12. Odd starts pass; even starts fail. + wire fixed_a = (cyc >= 1 && cyc <= 12); + wire fixed_b = (cyc == BIG_DELAY + 1) || (cyc == BIG_DELAY + 3) + || (cyc == BIG_DELAY + 5) || (cyc == BIG_DELAY + 7) + || (cyc == BIG_DELAY + 9) || (cyc == BIG_DELAY + 11); + + // Range delay: single passing starts match 10 cycles later. Two blocks never + // match and expire 300 cycles after each start. + wire range_pass_a = (cyc == 10) || (cyc == 40) || (cyc == 70) || (cyc == 100) + || (cyc == 130) || (cyc == 600) || (cyc == 640) + || (cyc == 680) || (cyc == 720); + wire range_fail_a = (cyc >= 250 && cyc <= 257) || (cyc >= 820 && cyc <= 827); + wire range_a = range_pass_a || range_fail_a; + wire range_b = (cyc == 20) || (cyc == 50) || (cyc == 80) || (cyc == 110) + || (cyc == 140) || (cyc == 610) || (cyc == 650) || (cyc == 690) + || (cyc == 730); + + // Questa action blocks observe the cycle after the sampled property cycle. + cover property (@(posedge clk) fixed_a ##1024 fixed_b) fixed_pass_q.push_back($sampled(cyc) + 1); + + assert property (@(posedge clk) fixed_a |-> ##1024 fixed_b) + else fixed_fail_q.push_back($sampled(cyc) + 1); + + cover property (@(posedge clk) range_a ##[5:300] range_b) range_pass_q.push_back($sampled(cyc) + 1); + + assert property (@(posedge clk) range_a |-> ##[5:300] range_b) + else range_fail_q.push_back($sampled(cyc) + 1); + + assert property (@(posedge clk) + 1'b0 |-> (fixed_a throughout (range_a throughout (range_b ##40 fixed_b)))); + + assert property (@(posedge clk) 1'b0 |-> ##[5:300] (range_b ##1 fixed_b)); + + assert property (@(posedge clk) 1'b0 |-> (fixed_a throughout (range_a ##[5:300] range_b))); + + always_ff @(posedge clk) begin + cyc <= cyc + 1; + if (cyc == END_CYC) begin + `checkp(fixed_pass_q, "'{'h402, 'h404, 'h406, 'h408, 'h40a, 'h40c}"); + `checkp(fixed_fail_q, "'{'h403, 'h405, 'h407, 'h409, 'h40b, 'h40d}"); + `checkp(range_pass_q, "'{'h15, 'h33, 'h51, 'h6f, 'h8d, 'h263, 'h28b, 'h2b3, 'h2db}"); + `checkp(range_fail_q, + "'{'h227, 'h228, 'h229, 'h22a, 'h22b, 'h22c, 'h22d, 'h22e, 'h461, 'h462, 'h463, 'h464, 'h465, 'h466, 'h467, 'h468}"); + + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule diff --git a/test_regress/t/t_property_sexpr_multi.py b/test_regress/t/t_property_sexpr_multi.py index 5de1b48bd..7f63922fc 100755 --- a/test_regress/t/t_property_sexpr_multi.py +++ b/test_regress/t/t_property_sexpr_multi.py @@ -11,7 +11,7 @@ import vltest_bootstrap test.scenarios('simulator') -test.compile(timing_loop=True, verilator_flags2=['--assert', '--timing', '--dumpi-V3AssertProp 6']) +test.compile(timing_loop=True, verilator_flags2=['--assert', '--timing']) test.execute() diff --git a/test_regress/t/t_property_sexpr_range_delay.v b/test_regress/t/t_property_sexpr_range_delay.v index 85ad2dcc8..929568fda 100644 --- a/test_regress/t/t_property_sexpr_range_delay.v +++ b/test_regress/t/t_property_sexpr_range_delay.v @@ -87,6 +87,9 @@ module t ( assert property (@(posedge clk) disable iff (cyc < 2) a |-> ##[1:10000] (a | b | c | d | e)); + cover property (@(posedge clk) disable iff (cyc < 2) + ##[0:10000] 1'b1); + // Range with binary SExpr: nextStep has delay > 0 after range match assert property (@(posedge clk) disable iff (cyc < 2) a |-> b ##[1:2] (a | b | c | d | e) ##3 (a | b | c | d | e)) From 93d0443998624e95562255b1f0c67cf2a5f0d2e8 Mon Sep 17 00:00:00 2001 From: Jakub Michalski <143384197+jmichalski-ant@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:48:17 +0200 Subject: [PATCH 5/6] Fix memory leak in VerilatedFst::close() (#7899) Signed-off-by: Jakub Michalski --- include/verilated_fst_c.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/verilated_fst_c.cpp b/include/verilated_fst_c.cpp index 0e3706725..de54d9e8e 100644 --- a/include/verilated_fst_c.cpp +++ b/include/verilated_fst_c.cpp @@ -98,8 +98,10 @@ void VerilatedFst::close() VL_MT_SAFE_EXCLUDES(m_mutex) { const VerilatedLockGuard lock{m_mutex}; Super::closeBase(); emitTimeChangeMaybe(); - if (m_fst) m_fst->close(); // LCOV_EXCL_BR_LINE - m_fst = nullptr; + if (m_fst) { + m_fst->close(); + VL_DO_CLEAR(delete m_fst, m_fst = nullptr); + } } void VerilatedFst::flush() VL_MT_SAFE_EXCLUDES(m_mutex) { From 646dcd38384b8017e27d559f4fe093a446891ddc Mon Sep 17 00:00:00 2001 From: Jakub Michalski <143384197+jmichalski-ant@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:22:24 +0200 Subject: [PATCH 6/6] Support user-provided DPI-C function declarations (#7626) (#7893) Fixes #7626. --- docs/guide/extensions.rst | 19 +++++++++++++++++++ src/V3AstNodeOther.h | 8 ++++++++ src/V3EmitCBase.cpp | 4 ++++ src/V3EmitCSyms.cpp | 8 ++++++-- src/V3PreProc.cpp | 13 +++++++++++++ src/V3Task.cpp | 1 + src/verilog.l | 1 + src/verilog.y | 9 +++++++++ test_regress/t/t_dpi_decl.cpp | 22 ++++++++++++++++++++++ test_regress/t/t_dpi_decl.py | 19 +++++++++++++++++++ test_regress/t/t_dpi_decl.v | 20 ++++++++++++++++++++ test_regress/t/t_dpi_decl_bad.out | 5 +++++ test_regress/t/t_dpi_decl_bad.py | 16 ++++++++++++++++ test_regress/t/t_dpi_decl_bad.v | 14 ++++++++++++++ 14 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 test_regress/t/t_dpi_decl.cpp create mode 100755 test_regress/t/t_dpi_decl.py create mode 100644 test_regress/t/t_dpi_decl.v create mode 100644 test_regress/t/t_dpi_decl_bad.out create mode 100755 test_regress/t/t_dpi_decl_bad.py create mode 100644 test_regress/t/t_dpi_decl_bad.v diff --git a/docs/guide/extensions.rst b/docs/guide/extensions.rst index 8e8d5d183..3cc41ea46 100644 --- a/docs/guide/extensions.rst +++ b/docs/guide/extensions.rst @@ -294,6 +294,25 @@ or "`ifdef`"'s may break other tools. (if appropriate :vlopt:`--coverage` flags are passed) after being disabled earlier with :option:`/*verilator&32;coverage_off*/`. +.. option:: /*verilator&32;dpi_c_decl ""*/ + + Specifies C function declaration that will be emitted for given DPI-C + function into the Verilator-generated __Dpi.h header, replacing the declaration + that Verilator would build by default using the Verilog function signature. + + This enables use of C functions with types not specified in the + standard. For example, it enables use of functions that return ``char*``: + + .. code-block:: sv + + module t; + import "DPI-C" function string getenv(input string arg) /*verilator dpi_c_decl "char* getenv(const char*)"*/; + + initial begin + $display("%s", getenv("HOME")); + end + endmodule + .. option:: /*verilator&32;fargs */ For Verilator developers only. When a source file containing these `fargs` diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 3c25a9992..d2bbf21e4 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -94,6 +94,7 @@ class AstNodeFTask VL_NOT_FINAL : public AstNode { // @astgen op4 := scopeNamep : Optional[AstScopeName] string m_name; // Name of task string m_cname; // Name of task if DPI import + string m_dpiCDecl; // Custom DPI-C function declaration string m_ifacePortName; // Interface port name for out-of-block definition (IEEE 25.8) uint64_t m_dpiOpenParent = 0; // DPI import open array, if !=0, how many callees bool m_taskPublic : 1; // Public task @@ -199,6 +200,9 @@ public: void dpiOpenChild(bool flag) { m_dpiOpenChild = flag; } bool dpiTask() const { return m_dpiTask; } void dpiTask(bool flag) { m_dpiTask = flag; } + bool dpiCDeclOverride() const { return !m_dpiCDecl.empty(); } + const string& dpiCDecl() const { return m_dpiCDecl; } + void dpiCDecl(const string& cDecl) { m_dpiCDecl = cDecl; } bool isConstructor() const { return m_isConstructor; } void isConstructor(bool flag) { m_isConstructor = flag; } bool isHideLocal() const { return m_isHideLocal; } @@ -511,6 +515,7 @@ class AstCFunc final : public AstNode { string m_rtnType; // void, bool, or other return type string m_argTypes; // Argument types string m_ifdef; // #ifdef symbol around this function + string m_cDecl; // Custom DPI-C function declaration VBoolOrUnknown m_isConst; // Function is declared const (*this not changed) bool m_isStatic : 1; // Function is static (no need for a 'this' pointer) bool m_isTrace : 1; // Function is related to tracing @@ -640,6 +645,9 @@ public: void dpiImportPrototype(bool flag) { m_dpiImportPrototype = flag; } bool dpiImportWrapper() const { return m_dpiImportWrapper; } void dpiImportWrapper(bool flag) { m_dpiImportWrapper = flag; } + bool dpiCDeclOverride() const { return !m_cDecl.empty(); } + const string& dpiCDecl() const { return m_cDecl; } + void dpiCDecl(const string& cDecl) { m_cDecl = cDecl; } bool isCoroutine() const { return m_rtnType == "VlCoroutine"; } void recursive(bool flag) { m_recursive = flag; } bool recursive() const { return m_recursive; } diff --git a/src/V3EmitCBase.cpp b/src/V3EmitCBase.cpp index f1680a40a..2e93700a5 100644 --- a/src/V3EmitCBase.cpp +++ b/src/V3EmitCBase.cpp @@ -137,6 +137,10 @@ void EmitCBaseVisitorConst::emitCDefaultConstructor(const AstNodeModule* const m void EmitCBaseVisitorConst::emitCFuncHeader(const AstCFunc* funcp, const AstNodeModule* modp, bool withScope) { if (funcp->slow()) putns(funcp, "VL_ATTR_COLD "); + if (funcp->dpiCDeclOverride()) { + putns(funcp, funcp->dpiCDecl() + ";\n"); + return; + } if (!funcp->isDestructor()) { putns(funcp, funcp->rtnTypeVoid()); puts(" "); diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index bcf79c164..baf500103 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -1434,8 +1434,12 @@ void EmitCSyms::emitDpiHdr() { if (!firstImp++) puts("\n// DPI IMPORTS\n"); putsDecoration(nodep, "// DPI import" + sourceLoc + "\n"); } - putns(nodep, "extern " + nodep->rtnTypeVoid() + " " + nodep->nameProtect() + "(" - + cFuncArgs(nodep) + ");\n"); + if (nodep->dpiCDeclOverride()) { + putns(nodep, "extern " + nodep->dpiCDecl() + ";\n"); + } else { + putns(nodep, "extern " + nodep->rtnTypeVoid() + " " + nodep->nameProtect() + "(" + + cFuncArgs(nodep) + ");\n"); + } } puts("\n"); diff --git a/src/V3PreProc.cpp b/src/V3PreProc.cpp index 67e2cf3f9..342797a72 100644 --- a/src/V3PreProc.cpp +++ b/src/V3PreProc.cpp @@ -545,6 +545,19 @@ void V3PreProcImp::comment(const string& text) { if (arg.size() && baseCmd == "public_flat_rw_on") baseCmd += "_sns"; // different cmd for applying sensitivity if (!printed) insertUnreadback("/*verilator " + baseCmd + "*/ " + arg + " /**/"); + } else if (VString::startsWith(cmd, "dpi_c_decl")) { + // "/*verilator dpi_c_decl foo(bar) */" -> "/*verilator dpi_c_decl*/ \"foo(bar)\"" + string baseCmd = cmd.substr(0, std::strlen("dpi_c_decl")); + string::size_type startOfArg = baseCmd.size(); + while (std::isspace(cmd[startOfArg])) startOfArg++; + string arg = cmd.substr(startOfArg); + if (arg.empty()) { + fileline()->v3warn( + BADVLTPRAGMA, + "No function declaration provided in /*verilator dpi_c_decl*/ meta-comment"); + return; + } + if (!printed) insertUnreadback("/*verilator " + baseCmd + "*/ \"" + arg + "\" /**/"); } else { if (!printed) insertUnreadback("/*verilator " + cmd + "*/"); } diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 72e779d1e..28768e305 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -1060,6 +1060,7 @@ class TaskVisitor final : public VNVisitor { // Add DPI Import to top, since it's a global function m_topScopep->scopep()->addBlocksp(funcp); + funcp->dpiCDecl(nodep->dpiCDecl()); if (!makePortList(nodep, funcp)) return nullptr; return funcp; } diff --git a/src/verilog.l b/src/verilog.l index 36058b0a6..d274148ff 100644 --- a/src/verilog.l +++ b/src/verilog.l @@ -825,6 +825,7 @@ vnum {vnum1}|{vnum2}|{vnum3}|{vnum4}|{vnum5} "/*verilator coverage_block_off*/" { FL; return yVL_COVERAGE_BLOCK_OFF; } "/*verilator coverage_off*/" { FL_FWD; PARSEP->lexFileline()->coverageOn(false); FL_BRK; } "/*verilator coverage_on*/" { FL_FWD; PARSEP->lexFileline()->coverageOn(true); FL_BRK; } + "/*verilator dpi_c_decl*/" { FL; return yVL_DPI_C_DECL; } // The "foo(bar)" is converted by the preproc "/*verilator forceable*/" { FL; return yVL_FORCEABLE; } "/*verilator full_case*/" { FL; return yVL_FULL_CASE; } "/*verilator hier_block*/" { FL; return yVL_HIER_BLOCK; } diff --git a/src/verilog.y b/src/verilog.y index db145ea8e..378c4f402 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -780,6 +780,7 @@ BISONPRE_VERSION(3.7,%define api.header.include {"V3ParseBison.h"}) %token yVL_CLOCKER "/*verilator clocker*/" %token yVL_CLOCK_ENABLE "/*verilator clock_enable*/" %token yVL_COVERAGE_BLOCK_OFF "/*verilator coverage_block_off*/" +%token yVL_DPI_C_DECL "/*verilator dpi_c_decl*/" %token yVL_FORCEABLE "/*verilator forceable*/" %token yVL_FULL_CASE "/*verilator full_case*/" %token yVL_HIER_BLOCK "/*verilator hier_block*/" @@ -4928,6 +4929,14 @@ dpi_import_export: // ==IEEE: dpi_import_export $5->dpiPure($3 == iprop_PURE); $5->dpiImport(true); GRAMMARP->checkDpiVer($1, *$2); v3Global.dpi(true); } + | yIMPORT yaSTRING dpi_tf_import_propertyE dpi_importLabelE function_prototype yVL_DPI_C_DECL yaSTRING ';' + { $$ = $5; + if (*$4 != "") $5->cname(*$4); + $5->dpiContext($3 == iprop_CONTEXT); + $5->dpiPure($3 == iprop_PURE); + $5->dpiImport(true); + if (*$7 != "") $5->dpiCDecl(*$7); + GRAMMARP->checkDpiVer($1, *$2); v3Global.dpi(true); } | yIMPORT yaSTRING dpi_tf_import_propertyE dpi_importLabelE task_prototype ';' { $$ = $5; if (*$4 != "") $5->cname(*$4); diff --git a/test_regress/t/t_dpi_decl.cpp b/test_regress/t/t_dpi_decl.cpp new file mode 100644 index 000000000..5cf783a39 --- /dev/null +++ b/test_regress/t/t_dpi_decl.cpp @@ -0,0 +1,22 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +//************************************************************************* +// +// 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 Antmicro +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* +#include "Vt_dpi_decl__Dpi.h" + +char* func(const char* arg) { + static char str[] = "abc"; + return str; +} +// some functions (e.g. getenv() from glibc) may have additional specifier, lack of it in the +// declaration will cause build errors +char* func_with_specifier(const char* arg) throw() { + static char str[] = "efd"; + return str; +} diff --git a/test_regress/t/t_dpi_decl.py b/test_regress/t/t_dpi_decl.py new file mode 100755 index 000000000..50cb5e61f --- /dev/null +++ b/test_regress/t/t_dpi_decl.py @@ -0,0 +1,19 @@ +#!/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 + +test.scenarios('simulator') +test.pli_filename = "t/t_dpi_decl.cpp" + +test.compile(v_flags2=[test.pli_filename]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_dpi_decl.v b/test_regress/t/t_dpi_decl.v new file mode 100644 index 000000000..1f5ae21c7 --- /dev/null +++ b/test_regress/t/t_dpi_decl.v @@ -0,0 +1,20 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +`define stop $stop +`define checks(gotv,expv) do if ((gotv) != (expv)) begin $write("%%Error: %s:%0d: got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); + +module t; + import "DPI-C" function string func(input string arg) /*verilator dpi_c_decl "char* func(const char*)"*/; + import "DPI-C" function string func_with_specifier(input string arg) /*verilator dpi_c_decl "char* func_with_specifier(const char*) throw()"*/; + + initial begin + `checks(func("arg"), "abc"); + `checks(func_with_specifier("arg"), "efd"); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_dpi_decl_bad.out b/test_regress/t/t_dpi_decl_bad.out new file mode 100644 index 000000000..9a6dc9fde --- /dev/null +++ b/test_regress/t/t_dpi_decl_bad.out @@ -0,0 +1,5 @@ +%Error-BADVLTPRAGMA: t/t_dpi_decl_bad.v:8:57: No function declaration provided in /*verilator dpi_c_decl*/ meta-comment + 8 | import "DPI-C" function string func(input string arg) /*verilator dpi_c_decl*/; + | ^~~~~~~~~~~~~~~~~~~~~~~~ + ... For error description see https://verilator.org/warn/BADVLTPRAGMA?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_dpi_decl_bad.py b/test_regress/t/t_dpi_decl_bad.py new file mode 100755 index 000000000..bdec1e4cd --- /dev/null +++ b/test_regress/t/t_dpi_decl_bad.py @@ -0,0 +1,16 @@ +#!/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 + +test.scenarios('simulator') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_dpi_decl_bad.v b/test_regress/t/t_dpi_decl_bad.v new file mode 100644 index 000000000..1d6466065 --- /dev/null +++ b/test_regress/t/t_dpi_decl_bad.v @@ -0,0 +1,14 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +module t; + import "DPI-C" function string func(input string arg) /*verilator dpi_c_decl*/; + + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule