From af2771e901b8a1e05fdd179e09a8966088aac341 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Wed, 29 Oct 2025 19:26:46 -0400 Subject: [PATCH] Fix function call expression coverage (#6589) --- src/V3Coverage.cpp | 46 ++++++++++++++++++++++++--- test_regress/t/t_cover_expr.out | 6 ++-- test_regress/t/t_cover_expr_max.out | 6 ++-- test_regress/t/t_cover_expr_trace.out | 6 ++-- test_regress/t/t_cover_line_expr.out | 12 +++---- 5 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/V3Coverage.cpp b/src/V3Coverage.cpp index 9c1b707e4..6a72acd00 100644 --- a/src/V3Coverage.cpp +++ b/src/V3Coverage.cpp @@ -29,6 +29,7 @@ #include "V3Coverage.h" #include "V3EmitV.h" +#include "V3UniqueNames.h" #include #include @@ -74,7 +75,7 @@ class ExprCoverageEligibleVisitor final : public VNVisitorConst { public: // CONSTRUCTORS - explicit ExprCoverageEligibleVisitor(AstNode* nodep) { iterateChildrenConst(nodep); } + explicit ExprCoverageEligibleVisitor(AstNode* nodep) { iterateConst(nodep); } ~ExprCoverageEligibleVisitor() override = default; bool eligible() { return m_eligible; } @@ -138,6 +139,9 @@ class CoverageVisitor final : public VNVisitor { // AstIf/AstLoopTest::user2() -> bool. True indicates coverage-generated const VNUser1InUse m_inuser1; const VNUser2InUse m_inuser2; + V3UniqueNames m_exprTempNames; // For generating unique temporary variable names used by + // expression coverage + std::unordered_map, AstVar*> m_funcTemps; // STATE - across all visitors int m_nextHandle = 0; @@ -145,6 +149,7 @@ class CoverageVisitor final : public VNVisitor { // STATE - for current visit position (use VL_RESTORER) CheckState m_state; // State save-restored on each new coverage scope/block AstNodeModule* m_modp = nullptr; // Current module to add statement to + AstNodeFTask* m_ftaskp = nullptr; // Current function/task AstNode* m_exprStmtsp = nullptr; // Node to add expr coverage to bool m_then = false; // Whether we're iterating the then or else branch // when m_exprStmtps is an AstIf @@ -271,6 +276,7 @@ class CoverageVisitor final : public VNVisitor { const AstNodeModule* const origModp = m_modp; VL_RESTORER(m_modp); VL_RESTORER(m_state); + VL_RESTORER(m_exprTempNames); createHandle(nodep); m_modp = nodep; m_state.m_inModOff @@ -325,6 +331,9 @@ class CoverageVisitor final : public VNVisitor { } void visit(AstNodeFTask* nodep) override { + VL_RESTORER(m_ftaskp); + VL_RESTORER(m_exprTempNames); + m_ftaskp = nodep; if (!nodep->dpiImport()) iterateProcedure(nodep); } @@ -774,8 +783,34 @@ class CoverageVisitor final : public VNVisitor { for (CoverTerm& term : expr) { comment += (first ? "" : " && ") + term.m_emitV + "==" + (term.m_objective ? "1" : "0"); - AstNodeExpr* const clonep = term.m_exprp->cloneTree(false); - AstNodeExpr* const termp = term.m_objective ? clonep : new AstLogNot{fl, clonep}; + AstNodeExpr* covExprp = nullptr; + if (AstFuncRef* const frefp = VN_CAST(term.m_exprp, FuncRef)) { + AstNodeDType* const dtypep = frefp->taskp()->fvarp()->dtypep(); + const auto pair = m_funcTemps.emplace(*frefp, nullptr); + AstVar* varp = pair.first->second; + if (pair.second) { + varp = new AstVar{fl, VVarType::MODULETEMP, m_exprTempNames.get(frefp), + dtypep}; + pair.first->second = varp; + if (m_ftaskp) { + varp->funcLocal(true); + varp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); + m_ftaskp->stmtsp()->addHereThisAsNext(varp); + } else { + m_modp->stmtsp()->addHereThisAsNext(varp); + } + VNRelinker relinkHandle; + frefp->unlinkFrBack(&relinkHandle); + relinkHandle.relink(new AstExprStmt{ + fl, new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, frefp}, + new AstVarRef{fl, varp, VAccess::READ}}); + } + covExprp = new AstVarRef{fl, varp, VAccess::READ}; + } else { + covExprp = term.m_exprp->cloneTree(false); + } + AstNodeExpr* const termp + = term.m_objective ? covExprp : new AstLogNot{fl, covExprp}; if (condp) { condp = new AstLogAnd{fl, condp, termp}; } else { @@ -1060,7 +1095,10 @@ class CoverageVisitor final : public VNVisitor { public: // CONSTRUCTORS - explicit CoverageVisitor(AstNetlist* rootp) { iterateChildren(rootp); } + explicit CoverageVisitor(AstNetlist* rootp) + : m_exprTempNames{"__VExpr"} { + iterateChildren(rootp); + } ~CoverageVisitor() override = default; }; diff --git a/test_regress/t/t_cover_expr.out b/test_regress/t/t_cover_expr.out index 16744d1df..336cf1c14 100644 --- a/test_regress/t/t_cover_expr.out +++ b/test_regress/t/t_cover_expr.out @@ -35,9 +35,9 @@ localparam bit ZERO = 1'b0; function automatic bit invert(bit x); - 000015 return ~x; -+000012 point: comment=(x==0) => 1 hier=top.t -+000015 point: comment=(x==1) => 0 hier=top.t +%000005 return ~x; +-000004 point: comment=(x==0) => 1 hier=top.t +-000005 point: comment=(x==1) => 0 hier=top.t endfunction function automatic bit and_oper(bit a, bit b); diff --git a/test_regress/t/t_cover_expr_max.out b/test_regress/t/t_cover_expr_max.out index 1dfc2e15a..68ce59c64 100644 --- a/test_regress/t/t_cover_expr_max.out +++ b/test_regress/t/t_cover_expr_max.out @@ -35,9 +35,9 @@ localparam bit ZERO = 1'b0; function automatic bit invert(bit x); - 000015 return ~x; -+000012 point: comment=(x==0) => 1 hier=top.t -+000015 point: comment=(x==1) => 0 hier=top.t +%000005 return ~x; +-000004 point: comment=(x==0) => 1 hier=top.t +-000005 point: comment=(x==1) => 0 hier=top.t endfunction function automatic bit and_oper(bit a, bit b); diff --git a/test_regress/t/t_cover_expr_trace.out b/test_regress/t/t_cover_expr_trace.out index 16744d1df..336cf1c14 100644 --- a/test_regress/t/t_cover_expr_trace.out +++ b/test_regress/t/t_cover_expr_trace.out @@ -35,9 +35,9 @@ localparam bit ZERO = 1'b0; function automatic bit invert(bit x); - 000015 return ~x; -+000012 point: comment=(x==0) => 1 hier=top.t -+000015 point: comment=(x==1) => 0 hier=top.t +%000005 return ~x; +-000004 point: comment=(x==0) => 1 hier=top.t +-000005 point: comment=(x==1) => 0 hier=top.t endfunction function automatic bit and_oper(bit a, bit b); diff --git a/test_regress/t/t_cover_line_expr.out b/test_regress/t/t_cover_line_expr.out index 5a6e1dfc4..24db5825f 100644 --- a/test_regress/t/t_cover_line_expr.out +++ b/test_regress/t/t_cover_line_expr.out @@ -37,12 +37,12 @@ localparam bit ONE = 1'b1; localparam bit ZERO = 1'b0; - 000027 function automatic bit invert(bit x); -+000027 point: comment=block hier=top.t - 000027 return ~x; -+000027 point: comment=block hier=top.t -+000012 point: comment=(x==0) => 1 hier=top.t -+000015 point: comment=(x==1) => 0 hier=top.t +%000009 function automatic bit invert(bit x); +-000009 point: comment=block hier=top.t +%000009 return ~x; +-000009 point: comment=block hier=top.t +-000004 point: comment=(x==0) => 1 hier=top.t +-000005 point: comment=(x==1) => 0 hier=top.t endfunction %000009 function automatic bit and_oper(bit a, bit b);