Fix function call expression coverage (#6589)

src/V3Coverage.cpp

@@ -29,6 +29,7 @@
 #include "V3Coverage.h"
 
 #include "V3EmitV.h"
+#include "V3UniqueNames.h"
 
 #include <list>
 #include <unordered_map>
@@ -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<VNRef<AstFuncRef>, 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* covExprp = nullptr;
-                AstNodeExpr* const termp = term.m_objective ? clonep : new AstLogNot{fl, clonep};
+                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;
 };
 

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;
+%000005         return ~x;
-+000012  point: comment=(x==0) => 1 hier=top.t
+-000004  point: comment=(x==0) => 1 hier=top.t
-+000015  point: comment=(x==1) => 0 hier=top.t
+-000005  point: comment=(x==1) => 0 hier=top.t
             endfunction
         
             function automatic bit and_oper(bit a, bit 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;
+%000005         return ~x;
-+000012  point: comment=(x==0) => 1 hier=top.t
+-000004  point: comment=(x==0) => 1 hier=top.t
-+000015  point: comment=(x==1) => 0 hier=top.t
+-000005  point: comment=(x==1) => 0 hier=top.t
             endfunction
         
             function automatic bit and_oper(bit a, bit 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;
+%000005         return ~x;
-+000012  point: comment=(x==0) => 1 hier=top.t
+-000004  point: comment=(x==0) => 1 hier=top.t
-+000015  point: comment=(x==1) => 0 hier=top.t
+-000005  point: comment=(x==1) => 0 hier=top.t
             endfunction
         
             function automatic bit and_oper(bit a, bit 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);
+%000009     function automatic bit invert(bit x);
-+000027  point: comment=block hier=top.t
+-000009  point: comment=block hier=top.t
- 000027         return ~x;
+%000009         return ~x;
-+000027  point: comment=block hier=top.t
+-000009  point: comment=block hier=top.t
-+000012  point: comment=(x==0) => 1 hier=top.t
+-000004  point: comment=(x==0) => 1 hier=top.t
-+000015  point: comment=(x==1) => 0 hier=top.t
+-000005  point: comment=(x==1) => 0 hier=top.t
             endfunction
         
 %000009     function automatic bit and_oper(bit a, bit b);