From 43975bcbdd65c06de7798ccd0e6825d2993f1897 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 13 Nov 2025 17:04:50 +0000 Subject: [PATCH] Optimize $past delayed variable reuse (#6689) As the TODO in the original code suggested, we can re-use delayed values of expressions if they appear in multiple $past calls. --- src/V3Assert.cpp | 96 +++++++++++++++++++++++++++++----------- test_regress/t/t_past.py | 2 +- test_regress/t/t_past.v | 14 ++++++ 3 files changed, 86 insertions(+), 26 deletions(-) diff --git a/src/V3Assert.cpp b/src/V3Assert.cpp index 448c08e8b..c89783dbe 100644 --- a/src/V3Assert.cpp +++ b/src/V3Assert.cpp @@ -18,6 +18,7 @@ #include "V3Assert.h" +#include "V3AstUserAllocator.h" #include "V3Stats.h" VL_DEFINE_DEBUG_FUNCTIONS; @@ -125,7 +126,10 @@ class AssertVisitor final : public VNVisitor { // NODE STATE/TYPES // Cleared on netlist // AstNode::user1() -> bool. True if processed - const VNUser1InUse m_inuser1; + // AstAlways::user2p() -> std::vector. Delayed variables via 'm_delayed' + const VNUser1InUse m_user1InUse; + const VNUser2InUse m_user2InUse; + AstUser2Allocator> m_delayed; // STATE AstNodeModule* m_modp = nullptr; // Last module @@ -146,6 +150,9 @@ class AssertVisitor final : public VNVisitor { AstNode* m_passsp = nullptr; // Current pass statement AstNode* m_failsp = nullptr; // Current fail statement bool m_underAssert = false; // Visited from assert + // Map from (expression, senTree) to AstAlways that computes delayed values of the expression + std::unordered_map, std::unordered_map, AstAlways*>> + m_modExpr2Sen2DelayedAlwaysp; // METHODS static AstNodeExpr* assertOnCond(FileLine* fl, VAssertType type, @@ -325,6 +332,65 @@ class AssertVisitor final : public VNVisitor { return bodysp; } + AstVar* createDelayedVar(const std::string& name, AstAlways* alwaysp, AstNodeExpr* exprp) { + FileLine* const flp = exprp->fileline(); + AstVar* const varp = new AstVar{flp, VVarType::MODULETEMP, name, exprp->dtypep()}; + // TODO: this lifetime seems nonsene (can't have NBAs to automatics), but is as before + varp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); + m_modp->addStmtsp(varp); + ++m_statPastVars; + // Actually set the delayed value + AstNodeExpr* const lhsp = new AstVarRef{flp, varp, VAccess::WRITE}; + AstAssignDly* const assignp = new AstAssignDly{flp, lhsp, exprp}; + if (!alwaysp->stmtsp()) { + alwaysp->addStmtsp(assignp); + } else { + alwaysp->stmtsp()->addHereThisAsNext(assignp); + } + return varp; + } + + AstAlways* getDelayedAlways(AstNodeExpr* exprp, AstSenTree* senTreep) { + AstAlways*& alwayspr = m_modExpr2Sen2DelayedAlwaysp[*exprp][*senTreep]; + if (!alwayspr) { + FileLine* const flp = exprp->fileline(); + // Create the always block that computes the delayed values + alwayspr = new AstAlways{flp, VAlwaysKwd::ALWAYS, senTreep, nullptr}; + m_modp->addStmtsp(alwayspr); + // Create the once-delayed variable + const std::string name = "_Vpast_" + cvtToStr(m_modPastNum++) + "_1"; + AstVar* const varp = createDelayedVar(name, alwayspr, exprp); + // Add it to delayed variable vector + m_delayed(alwayspr).emplace_back(varp); + } else { + // Reusing exiting, not needed + VL_DO_DANGLING(pushDeletep(exprp), exprp); + VL_DO_DANGLING(pushDeletep(senTreep), senTreep); + } + return alwayspr; + } + + AstNodeExpr* getPastValue(AstNodeExpr* exprp, AstSenTree* senTreep, uint32_t ticks) { + UASSERT_OBJ(ticks > 0, exprp, "Delay must be > 0"); + AstAlways* const alwaysp = getDelayedAlways(exprp, senTreep); + std::vector& delayedr = m_delayed(alwaysp); + // Ensure the required delay exists + while (delayedr.size() < ticks) { + AstVar* const firstp = delayedr.front(); + FileLine* const flp = firstp->fileline(); + // Create once more delayed value + std::string name = firstp->name(); + name.resize(name.size() - 1); + name += std::to_string(delayedr.size() + 1); + AstNodeExpr* const prevp = new AstVarRef{flp, delayedr.back(), VAccess::READ}; + AstVar* const varp = createDelayedVar(name, alwaysp, prevp); + // Add it to delayed variable vector + delayedr.emplace_back(varp); + } + // Return a reference to the appropriately delayed variable + return new AstVarRef{exprp->fileline(), delayedr.at(ticks - 1), VAccess::READ}; + } + void visitAssertionIterate(AstNodeCoverOrAssert* nodep, AstNode* failsp) { if (m_beginp && nodep->name() == "") nodep->name(m_beginp->name()); @@ -592,30 +658,8 @@ class AssertVisitor final : public VNVisitor { ticks = VN_AS(nodep->ticksp(), Const)->toUInt(); } UASSERT_OBJ(ticks >= 1, nodep, "0 tick should have been checked in V3Width"); - AstNodeExpr* const exprp = nodep->exprp()->unlinkFrBack(); - AstNodeExpr* inp = newSampledExpr(exprp); - AstVar* invarp = nullptr; - AstSenTree* const sentreep = nodep->sentreep()->unlinkFrBack(); - AstAlways* const alwaysp - = new AstAlways{nodep->fileline(), VAlwaysKwd::ALWAYS, sentreep, nullptr}; - m_modp->addStmtsp(alwaysp); - for (uint32_t i = 0; i < ticks; ++i) { - // TODO recognize AstVarRef is getting delayed and share variables between - // $pasts with same reference (or same expression). Saves downstream - // optimizations from identifying and removing duplication. - AstVar* const outvarp = new AstVar{ - nodep->fileline(), VVarType::MODULETEMP, - "_Vpast_" + cvtToStr(m_modPastNum++) + "_" + cvtToStr(i), inp->dtypep()}; - outvarp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); - ++m_statPastVars; - m_modp->addStmtsp(outvarp); - AstNode* const assp = new AstAssignDly{ - nodep->fileline(), new AstVarRef{nodep->fileline(), outvarp, VAccess::WRITE}, inp}; - alwaysp->addStmtsp(assp); - // UINFOTREE(9, assp, "", "ass"); - invarp = outvarp; - inp = new AstVarRef{nodep->fileline(), invarp, VAccess::READ}; - } + AstNodeExpr* const exprp = newSampledExpr(nodep->exprp()->unlinkFrBack()); + AstNodeExpr* inp = getPastValue(exprp, nodep->sentreep()->unlinkFrBack(), ticks); nodep->replaceWith(inp); VL_DO_DANGLING(pushDeletep(nodep), nodep); } @@ -856,9 +900,11 @@ class AssertVisitor final : public VNVisitor { VL_RESTORER(m_modp); VL_RESTORER(m_modPastNum); VL_RESTORER(m_modStrobeNum); + VL_RESTORER(m_modExpr2Sen2DelayedAlwaysp); m_modp = nodep; m_modPastNum = 0; m_modStrobeNum = 0; + m_modExpr2Sen2DelayedAlwaysp.clear(); iterateChildren(nodep); } void visit(AstNodeProcedure* nodep) override { diff --git a/test_regress/t/t_past.py b/test_regress/t/t_past.py index bd043aa76..72a44ba64 100755 --- a/test_regress/t/t_past.py +++ b/test_regress/t/t_past.py @@ -16,6 +16,6 @@ test.compile(verilator_flags2=['--stats']) test.execute() # Check that $past shared common variables -test.file_grep(test.stats, r'Assertions, \$past variables\s+(\d+)', 11) +test.file_grep(test.stats, r'Assertions, \$past variables\s+(\d+)', 8) test.passes() diff --git a/test_regress/t/t_past.v b/test_regress/t/t_past.v index 1a5e25ab3..a32858a45 100644 --- a/test_regress/t/t_past.v +++ b/test_regress/t/t_past.v @@ -59,6 +59,10 @@ module Test (/*AUTOARG*/ reg [31:0] dly1; reg [31:0] dly2; reg [31:0] dly3; + reg [31:0] dly0Inc; + reg [31:0] dly1Inc; + reg [31:0] dly2Inc; + reg [31:0] dly3Inc; // If called in an assertion, sequence, or property, the appropriate clocking event. // Otherwise, if called in a disable condition or a clock expression in an assertion, sequence, or prop, explicit. @@ -71,6 +75,10 @@ module Test (/*AUTOARG*/ dly1 <= dly0; dly2 <= dly1; dly3 <= dly2; + dly0Inc <= in + 1; + dly1Inc <= dly0Inc; + dly2Inc <= dly1Inc; + dly3Inc <= dly2Inc; if ($time > 40) begin // $past(expression, ticks, expression, clocking) // In clock expression @@ -79,12 +87,18 @@ module Test (/*AUTOARG*/ if (dly1 != $past(in, 2)) $stop; if (dly1 != $past(in, 2, )) $stop; if (dly1 != $past(in, 2, , )) $stop; + if (dly0Inc != $past(in + 1)) $stop; + if (dly0Inc != $past(in + 1,)) $stop; + if (dly1Inc != $past(in + 1, 2)) $stop; + if (dly1Inc != $past(in + 1, 2, )) $stop; + if (dly1Inc != $past(in + 1, 2, , )) $stop; // $sampled(expression) -> expression if (in != $sampled(in)) $stop; end end assert property (@(posedge clk) $time < 40 || dly0 == $past(in)); + assert property (@(posedge clk) $time < 40 || dly0Inc == $past(in + 1)); endmodule