Internals: Make AstCAwait an AstNodeStmt (#6280)

AstCAwait is only ever uses in statement position, so model it as a
statement. We should never ever have a coroutine that returns a value.
There is no need for it in SV, nor should we rely on it for internals.

Also reworks the fix for V3Life incorrectly constant propagating the
beforeTrig functions (#7072). The property that upsets V3Life is that
a function:
1. Is called from multiple static call sites (multiple AstCCall)
2. Reads model state directly (AstVarRef to non-locals/arguments)

Such function can only be created internally after scheduling (V3Task
throws an unsupported error on a non-inlined function that reads model
state), so added a flag to AstCFunc to mark the dangerous ones for
V3Life.

src/V3AstNodeExpr.h

@@ -1349,7 +1349,6 @@ class AstExprStmt final : public AstNodeExpr {
     // @astgen op2 := resultp : AstNodeExpr
 private:
     bool m_hasResult = true;
-    bool m_containsTimingControl = false;
 
 public:
     AstExprStmt(FileLine* fl, AstNode* stmtsp, AstNodeExpr* resultp)
@@ -1370,8 +1369,6 @@ public:
     bool sameNode(const AstNode*) const override { return true; }
     bool hasResult() const { return m_hasResult; }
     void hasResult(bool flag) { m_hasResult = flag; }
-    void setTimingControl() { m_containsTimingControl = true; }
-    bool isTimingControl() const override { return m_containsTimingControl; }
 };
 class AstFError final : public AstNodeExpr {
     // @astgen op1 := filep : AstNode
@@ -4962,28 +4959,6 @@ public:
     int instrCount() const override { return INSTR_COUNT_DBL; }
     bool isSystemFunc() const override { return true; }
 };
-class AstCAwait final : public AstNodeUniop {
-    // Emit C++'s co_await expression
-    // @astgen alias op1 := exprp
-    //
-    // @astgen ptr := m_sentreep : Optional[AstSenTree]  // Sentree related to this await
-public:
-    AstCAwait(FileLine* fl, AstNodeExpr* exprp, AstSenTree* sentreep = nullptr)
-        : ASTGEN_SUPER_CAwait(fl, exprp)
-        , m_sentreep{sentreep} {}
-    ASTGEN_MEMBERS_AstCAwait;
-    bool isTimingControl() const override { return true; }
-    void dump(std::ostream& str) const override;
-    void dumpJson(std::ostream& str) const override;
-    AstSenTree* sentreep() const { return m_sentreep; }
-    void clearSentreep() { m_sentreep = nullptr; }
-    void numberOperate(V3Number& out, const V3Number& lhs) override { V3ERROR_NA; }
-    string emitVerilog() override { V3ERROR_NA_RETURN(""); }
-    string emitC() override { V3ERROR_NA_RETURN(""); }
-    bool cleanOut() const override { return true; }
-    bool cleanLhs() const override { return true; }
-    bool sizeMattersLhs() const override { return false; }
-};
 class AstCCast final : public AstNodeUniop {
     // Cast to C-based data type
     int m_size;

src/V3AstNodeOther.h

@@ -498,6 +498,7 @@ class AstCFunc final : public AstNode {
     bool m_dpiImportWrapper : 1;  // Wrapper for invoking DPI import prototype from generated code
     bool m_needProcess : 1;  // Needs access to VlProcess of the caller
     bool m_recursive : 1;  // Recursive or part of recursion
+    bool m_noLife: 1; // Disable V3Life on this function - has multiple calls, and reads Syms state
     int m_cost;  // Function call cost
 public:
     AstCFunc(FileLine* fl, const string& name, AstScope* scopep, const string& rtnType = "")
@@ -527,6 +528,7 @@ public:
         m_dpiImportPrototype = false;
         m_dpiImportWrapper = false;
         m_recursive = false;
+        m_noLife = false;
         m_cost = v3Global.opt.instrCountDpi();  // As proxy for unknown general DPI cost
     }
     ASTGEN_MEMBERS_AstCFunc;
@@ -600,6 +602,8 @@ public:
     bool isCoroutine() const { return m_rtnType == "VlCoroutine"; }
     void recursive(bool flag) { m_recursive = flag; }
     bool recursive() const { return m_recursive; }
+    void noLife(bool flag) { m_noLife = flag; }
+    bool noLife() const { return m_noLife; }
     void cost(int cost) { m_cost = cost; }
     // Special methods
     bool emptyBody() const {

src/V3AstNodeStmt.h

@@ -253,6 +253,26 @@ public:
     string verilogKwd() const override { return "break"; }
     bool isBrancher() const override { V3ERROR_NA_RETURN(true); }  // Node removed early
 };
+class AstCAwait final : public AstNodeStmt {
+    // C++'s co_await. While this is an expression in C++, in Verilator it is only used
+    // with void result types and always appears in statement position. There must never
+    // be a suspendable process that returns a value, so modeling as a statement instead.
+    // @astgen op1 := exprp : AstNodeExpr
+    //
+    // @astgen ptr := m_sentreep : Optional[AstSenTree]  // Sentree related to this await
+public:
+    AstCAwait(FileLine* fl, AstNodeExpr* exprp, AstSenTree* sentreep = nullptr)
+        : ASTGEN_SUPER_CAwait(fl)
+        , m_sentreep{sentreep} {
+        this->exprp(exprp);
+    }
+    ASTGEN_MEMBERS_AstCAwait;
+    void dump(std::ostream& str) const override;
+    void dumpJson(std::ostream& str) const override;
+    bool isTimingControl() const override { return true; }
+    AstSenTree* sentreep() const { return m_sentreep; }
+    void clearSentreep() { m_sentreep = nullptr; }
+};
 class AstCReturn final : public AstNodeStmt {
     // C++ return from a function
     // @astgen op1 := lhsp : AstNodeExpr

src/V3AstNodes.cpp

@@ -3236,6 +3236,7 @@ void AstCFunc::dump(std::ostream& str) const {
     if (isCoroutine()) str << " [CORO]";
     if (needProcess()) str << " [NPRC]";
     if (entryPoint()) str << " [ENTRY]";
+    if (noLife()) str << " [NOLIFE]";
 }
 void AstCFunc::dumpJson(std::ostream& str) const {
     dumpJsonBoolFuncIf(str, slow);
@@ -3250,11 +3251,12 @@ void AstCFunc::dumpJson(std::ostream& str) const {
     dumpJsonBoolFuncIf(str, isVirtual);
     dumpJsonBoolFuncIf(str, isCoroutine);
     dumpJsonBoolFuncIf(str, needProcess);
+    dumpJsonBoolFuncIf(str, noLife);
     dumpJsonGen(str);
     // TODO: maybe try to shorten these flags somehow
 }
 void AstCAwait::dump(std::ostream& str) const {
-    this->AstNodeUniop::dump(str);
+    this->AstNodeStmt::dump(str);
     if (sentreep()) {
         str << " => ";
         sentreep()->dump(str);

src/V3EmitCFunc.h

@@ -709,6 +709,7 @@ public:
     void visit(AstCAwait* nodep) override {
         putns(nodep, "co_await ");
         iterateConst(nodep->exprp());
+        puts(";\n");
     }
     void visit(AstCNew* nodep) override {
         if (VN_IS(nodep->dtypep(), VoidDType)) {

src/V3Life.cpp

@@ -398,6 +398,7 @@ class LifeVisitor final : public VNVisitor {
         if (!m_tracingCall && !nodep->entryPoint()) return;
         m_tracingCall = false;
         if (nodep->recursive()) setNoopt();
+        if (nodep->noLife()) setNoopt();
         if (nodep->dpiImportPrototype() && !nodep->dpiPure()) {
             m_sideEffect = true;  // If appears on assign RHS, don't ever delete the assignment
         }

src/V3SchedTrigger.cpp

@@ -859,6 +859,9 @@ class AwaitBeforeTrigVisitor final : public VNVisitor {
             AstVar* const tmpVarp = tmpp->varp()->unlinkFrBack();
             funcp->user1p(tmpp);
             funcp->addVarsp(tmpVarp);
+            // This function can be called multiple times, and accesses model state, which
+            // violates the assumption made in V3Life that there is no such function.
+            funcp->noLife(true);
             tmpVarp->funcLocal(true);
             tmpVarp->noReset(true);
 
@@ -884,20 +887,13 @@ class AwaitBeforeTrigVisitor final : public VNVisitor {
             if (cMethodHardp->method() == VCMethod::SCHED_TRIGGER) {
                 AstCCall* const beforeTrigp = getBeforeTriggerStmt(nodep->sentreep());
 
-                FileLine* const flp = nodep->fileline();
                 // Add eventDescription argument value to a CCall - it is used for --runtime-debug
                 AstNode* const pinp = cMethodHardp->pinsp()->nextp()->nextp();
                 UASSERT_OBJ(pinp, cMethodHardp, "No event description");
                 beforeTrigp->addArgsp(VN_AS(pinp, NodeExpr)->cloneTree(false));
 
-                // Change CAwait Expression into StmtExpr that calls to a before-trigger function
+                // Call the before-trigger function before the CAwait
-                // first and then return CAwait
+                nodep->addHereThisAsNext(beforeTrigp->makeStmt());
-                VNRelinker relinker;
-                nodep->unlinkFrBack(&relinker);
-                AstExprStmt* const exprstmtp
-                    = new AstExprStmt{flp, beforeTrigp->makeStmt(), nodep};
-                exprstmtp->setTimingControl();
-                relinker.relink(exprstmtp);
                 m_senTreeToSched.emplace(nodep->sentreep(), cMethodHardp->fromp());
             }
         }

src/V3SchedUtil.cpp

@@ -137,11 +137,11 @@ void splitCheckFinishSubFunc(AstCFunc* ofuncp, AstCFunc* subFuncp,
     }
 
     bool containsAwait = false;
-    subFuncp->foreach([&](AstNodeExpr* exprp) {
+    subFuncp->foreach([&](AstNode* nodep) {
         // Record if it has a CAwait
-        if (VN_IS(exprp, CAwait)) containsAwait = true;
+        if (VN_IS(nodep, CAwait)) containsAwait = true;
         // Redirect references to arguments to the clone in the sub-function
-        if (AstVarRef* const refp = VN_CAST(exprp, VarRef)) {
+        if (AstVarRef* const refp = VN_CAST(nodep, VarRef)) {
             if (AstVarScope* const vscp = VN_AS(refp->varp()->user3p(), VarScope)) {
                 refp->varp(vscp->varp());
                 refp->varScopep(vscp);
@@ -151,9 +151,7 @@ void splitCheckFinishSubFunc(AstCFunc* ofuncp, AstCFunc* subFuncp,
 
     if (ofuncp->isCoroutine() && containsAwait) {  // Wrap call with co_await
         subFuncp->rtnType("VlCoroutine");
-        AstCAwait* const awaitp = new AstCAwait{flp, callp};
+        ofuncp->addStmtsp(new AstCAwait{flp, callp});
-        awaitp->dtypeSetVoid();
-        ofuncp->addStmtsp(awaitp->makeStmt());
     } else {
         ofuncp->addStmtsp(callp->makeStmt());
     }

src/V3Timing.cpp

@@ -759,9 +759,7 @@ class TimingControlVisitor final : public VNVisitor {
         joinp->dtypeSetVoid();
         addProcessInfo(joinp);
         addDebugInfo(joinp);
-        AstCAwait* const awaitp = new AstCAwait{flp, joinp};
+        forkp->addNextHere(new AstCAwait{flp, joinp});
-        awaitp->dtypeSetVoid();
-        forkp->addNextHere(awaitp->makeStmt());
     }
 
     // `procp` shall be a NodeProcedure/CFunc/Begin and within it vars from `varsp` will be placed.
@@ -907,11 +905,10 @@ class TimingControlVisitor final : public VNVisitor {
     void visit(AstNodeCCall* nodep) override {
         if (nodep->funcp()->needProcess()) m_hasProcess = true;
         if (hasFlags(nodep->funcp(), T_SUSPENDEE) && !nodep->user1SetOnce()) {  // If suspendable
-            VNRelinker relinker;
+            // Calls to suspendables are always void return type, hence parent must be StmtExpr
-            nodep->unlinkFrBack(&relinker);
+            AstStmtExpr* const stmtp = VN_AS(nodep->backp(), StmtExpr);
-            AstCAwait* const awaitp = new AstCAwait{nodep->fileline(), nodep};
+            stmtp->replaceWith(new AstCAwait{nodep->fileline(), nodep->unlinkFrBack()});
-            awaitp->dtypeSetVoid();
+            VL_DO_DANGLING(stmtp->deleteTree(), stmtp);
-            relinker.relink(awaitp);
         }
         iterateChildren(nodep);
     }
@@ -948,15 +945,12 @@ class TimingControlVisitor final : public VNVisitor {
         addProcessInfo(delayMethodp);
         addDebugInfo(delayMethodp);
         // Create the co_await
-        AstCAwait* const awaitp = new AstCAwait{flp, delayMethodp, getCreateDelaySenTree()};
+        AstNode* const awaitp = new AstCAwait{flp, delayMethodp, getCreateDelaySenTree()};
-        awaitp->dtypeSetVoid();
-        AstStmtExpr* const awaitStmtp = awaitp->makeStmt();
         // Relink child statements after the co_await
-        if (nodep->stmtsp()) {
+        if (AstNode* const stmtsp = nodep->stmtsp()) {
-            AstNode::addNext<AstNode, AstNode>(awaitStmtp,
+            awaitp->addNext(stmtsp->unlinkFrBackWithNext());
-                                               nodep->stmtsp()->unlinkFrBackWithNext());
         }
-        nodep->replaceWith(awaitStmtp);
+        nodep->replaceWith(awaitp);
         VL_DO_DANGLING(nodep->deleteTree(), nodep);
     }
     void visit(AstEventControl* nodep) override {
@@ -989,7 +983,6 @@ class TimingControlVisitor final : public VNVisitor {
             // Create the co_await
             AstCAwait* const awaitEvalp
                 = new AstCAwait{flp, evalMethodp, getCreateDynamicTriggerSenTree()};
-            awaitEvalp->dtypeSetVoid();
             // Construct the sen expression for this sentree
             UASSERT_OBJ(m_senExprBuilderp, nodep, "No SenExprBuilder for this scope");
             auto* const assignp = new AstAssign{flp, new AstVarRef{flp, trigvscp, VAccess::WRITE},
@@ -1007,7 +1000,7 @@ class TimingControlVisitor final : public VNVisitor {
                 = new AstLogNot{flp, new AstVarRef{flp, trigvscp, VAccess::READ}};
             AstLoop* const loopp = new AstLoop{flp};
             loopp->addStmtsp(new AstLoopTest{flp, loopp, condp});
-            loopp->addStmtsp(awaitEvalp->makeStmt());
+            loopp->addStmtsp(awaitEvalp);
             // Put pre updates before the trigger check and assignment
             for (AstNodeStmt* const stmtp : senResults.m_preUpdates) loopp->addStmtsp(stmtp);
             // Then the trigger check and assignment
@@ -1024,14 +1017,14 @@ class TimingControlVisitor final : public VNVisitor {
             if (destructivePostUpdate(sentreep)) {
                 AstCAwait* const awaitPostUpdatep = awaitEvalp->cloneTree(false);
                 VN_AS(awaitPostUpdatep->exprp(), CMethodHard)->method(VCMethod::SCHED_POST_UPDATE);
-                loopp->addStmtsp(awaitPostUpdatep->makeStmt());
+                loopp->addStmtsp(awaitPostUpdatep);
             }
             // Put the post updates at the end of the loop
             for (AstNodeStmt* const stmtp : senResults.m_postUpdates) loopp->addStmtsp(stmtp);
             // Finally, await the resumption step in 'act'
             AstCAwait* const awaitResumep = awaitEvalp->cloneTree(false);
             VN_AS(awaitResumep->exprp(), CMethodHard)->method(VCMethod::SCHED_RESUMPTION);
-            AstNode::addNext<AstNodeStmt, AstNodeStmt>(loopp, awaitResumep->makeStmt());
+            AstNode::addNext<AstNodeStmt, AstNodeStmt>(loopp, awaitResumep);
             // Replace the event control with the loop
             nodep->replaceWith(loopp);
         } else {
@@ -1050,8 +1043,7 @@ class TimingControlVisitor final : public VNVisitor {
             addEventDebugInfo(triggerMethodp, sentreep);
             // Create the co_await
             AstCAwait* const awaitp = new AstCAwait{flp, triggerMethodp, sentreep};
-            awaitp->dtypeSetVoid();
+            nodep->replaceWith(awaitp);
-            nodep->replaceWith(awaitp->makeStmt());
         }
         VL_DO_DANGLING(nodep->deleteTree(), nodep);
     }
@@ -1250,8 +1242,7 @@ class TimingControlVisitor final : public VNVisitor {
                 // callstack
                 AstCExpr* const foreverp = new AstCExpr{flp, "VlForever{}"};
                 AstCAwait* const awaitp = new AstCAwait{flp, foreverp};
-                awaitp->dtypeSetVoid();
+                nodep->replaceWith(awaitp);
-                nodep->replaceWith(awaitp->makeStmt());
                 if (stmtsp) VL_DO_DANGLING(stmtsp->deleteTree(), stmtsp);
                 VL_DO_DANGLING(condp->deleteTree(), condp);
             } else {