luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Internals: Make AstCAwait an AstNodeStmt (#6280) (#7078) 

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.
This commit is contained in:
Geza Lore 2026-02-14 20:15:32 +00:00 committed by GitHub
parent be27811a10
commit a0b89dde8e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 53 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/V3AstNodeExpr.h
View File

@ -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;

5
src/V3AstNodeOther.h
View File

@ -498,6 +498,8 @@ 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 +529,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 +603,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 {

20
src/V3AstNodeStmt.h
View File

@ -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

4
src/V3AstNodes.cpp
View File

@ -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);

1
src/V3EmitCFunc.h
View File

@ -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)) {

1
src/V3Life.cpp
View File

@ -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
}

14
src/V3SchedTrigger.cpp
View File

@ -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
// first and then return CAwait
VNRelinker relinker;
nodep->unlinkFrBack(&relinker);
AstExprStmt* const exprstmtp
= new AstExprStmt{flp, beforeTrigp->makeStmt(), nodep};
exprstmtp->setTimingControl();
relinker.relink(exprstmtp);
// Call the before-trigger function before the CAwait
nodep->addHereThisAsNext(beforeTrigp->makeStmt());
m_senTreeToSched.emplace(nodep->sentreep(), cMethodHardp->fromp());
}
}

10
src/V3SchedUtil.cpp
View File

@ -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};
awaitp->dtypeSetVoid();
ofuncp->addStmtsp(awaitp->makeStmt());
ofuncp->addStmtsp(new AstCAwait{flp, callp});
} else {
ofuncp->addStmtsp(callp->makeStmt());
}

37
src/V3Timing.cpp
View File

@ -759,9 +759,7 @@ class TimingControlVisitor final : public VNVisitor {
joinp->dtypeSetVoid();
addProcessInfo(joinp);
addDebugInfo(joinp);
AstCAwait* const awaitp = new AstCAwait{flp, joinp};
awaitp->dtypeSetVoid();
forkp->addNextHere(awaitp->makeStmt());
forkp->addNextHere(new AstCAwait{flp, joinp});
}
// `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;
nodep->unlinkFrBack(&relinker);
AstCAwait* const awaitp = new AstCAwait{nodep->fileline(), nodep};
awaitp->dtypeSetVoid();
relinker.relink(awaitp);
// Calls to suspendables are always void return type, hence parent must be StmtExpr
AstStmtExpr* const stmtp = VN_AS(nodep->backp(), StmtExpr);
stmtp->replaceWith(new AstCAwait{nodep->fileline(), nodep->unlinkFrBack()});
VL_DO_DANGLING(pushDeletep(stmtp), stmtp);
}
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()};
awaitp->dtypeSetVoid();
AstStmtExpr* const awaitStmtp = awaitp->makeStmt();
AstNode* const awaitp = new AstCAwait{flp, delayMethodp, getCreateDelaySenTree()};
// Relink child statements after the co_await
if (nodep->stmtsp()) {
AstNode::addNext<AstNode, AstNode>(awaitStmtp,
nodep->stmtsp()->unlinkFrBackWithNext());
if (AstNode* const stmtsp = nodep->stmtsp()) {
awaitp->addNext(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->makeStmt());
nodep->replaceWith(awaitp);
}
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->makeStmt());
nodep->replaceWith(awaitp);
if (stmtsp) VL_DO_DANGLING(stmtsp->deleteTree(), stmtsp);
VL_DO_DANGLING(condp->deleteTree(), condp);
} else {