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

Optimize wide conditional expansion in V3Premit (#7691) 

V3Premit extracts wide sub-expressions via temporaries, which is needed
for emitting wide operations to C++ (calls to `VL_*_W`). The previous
version used to extract both branches of an AstCond unconditionally,
meaning both branches were fully evaluated. Rewriting the AstCond into
an AstIf instead enables evaluating only the required branch. While
this does limit V3Subst, overall the resulting code is ~3% faster,
and contains ~25% fewer branches on a large design.
This commit is contained in:
Geza Lore 2026-06-01 20:25:41 +01:00 committed by GitHub
parent 39b9901032
commit c079aa0b17
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 93 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/V3Const.cpp
View File

@ -1653,6 +1653,9 @@ class ConstVisitor final : public VNVisitor {
if (VN_IS(thensp->rhsp()->dtypep()->skipRefp(), UnpackArrayDType)) return false;
if (VN_IS(elsesp->rhsp()->dtypep()->skipRefp(), UnpackArrayDType)) return false;
if (m_underRecFunc) return false; // This optimization may lead to infinite recursion
if (m_doCpp) { // Do not combine ifs cretaed by V3Premit
if (VN_IS(thensp->lhsp(), VarRef) && thensp->lhsp()->isWide()) return false;
}
// Only do it if not calls and both pure, otherwise undoes V3LiftExpr
return !VN_IS(thensp->rhsp(), NodeFTaskRef) //
&& !VN_IS(elsesp->rhsp(), NodeFTaskRef) //

113
src/V3Premit.cpp
View File

@ -55,14 +55,48 @@ class PremitVisitor final : public VNVisitor {
bool m_assignLhs = false; // Inside assignment lhs, don't breakup extracts
// METHODS
void checkNode(AstNodeExpr* nodep) {
bool needsTemp(AstNodeExpr* nodep) {
// Consider adding a temp for this expression.
if (!m_stmtp) return; // Not under a statement
if (nodep->user1SetOnce()) return; // Already processed
if (!nodep->isWide()) return; // Not wide
if (m_assignLhs) return; // This is an lvalue!
if (!m_stmtp) return false; // Not under a statement
if (nodep->user1SetOnce()) return false; // Already processed
if (!nodep->isWide()) return false; // Not wide
if (m_assignLhs) return false; // This is an lvalue!
UASSERT_OBJ(!VN_IS(nodep->firstAbovep(), ArraySel), nodep, "Should have been ignored");
createTemp(nodep);
return true;
}
void checkNode(AstNodeExpr* nodep) {
if (needsTemp(nodep)) createTemp(nodep);
}
AstVarRef* isRhsOfAssignToVar(AstNodeExpr* nodep) {
// If enclosing statement is an assign
AstNodeAssign* const assignp = VN_CAST(m_stmtp, NodeAssign);
if (!assignp) return nullptr;
// of which 'nodep' is the RHS of
if (assignp->rhsp() != nodep) return nullptr;
// and the LHS is a sipmple variable reference
AstVarRef* const lRefp = VN_CAST(assignp->lhsp(), VarRef);
if (!lRefp) return nullptr;
AstVar* const varp = lRefp->varp();
// And the RHS does not use the same variable
if (nodep->exists([varp](const AstVarRef* refp) { return refp->varp() == varp; })) {
return nullptr;
}
// Then return the LHS reference
return lRefp;
}
// Create a new temporary that can hold the value of the given expression
AstVar* newTmpFor(AstNodeExpr* nodep) {
FileLine* const flp = nodep->fileline();
const std::string name = "__Vtemp_" + std::to_string(++m_tmpVarCnt);
AstVar* const varp = new AstVar{flp, VVarType::STMTTEMP, name, nodep->dtypep()};
varp->funcLocal(true);
varp->noReset(true);
m_cfuncp->addVarsp(varp);
++m_temporaryVarsCreated;
return varp;
}
AstVar* createTemp(AstNodeExpr* nodep) {
@ -89,16 +123,10 @@ class PremitVisitor final : public VNVisitor {
++m_extractedToConstPool;
} else {
// Keep as local temporary.
const std::string name = "__Vtemp_" + std::to_string(++m_tmpVarCnt);
varp = new AstVar{flp, VVarType::STMTTEMP, name, nodep->dtypep()};
varp->funcLocal(true);
varp->noReset(true);
m_cfuncp->addVarsp(varp);
++m_temporaryVarsCreated;
varp = newTmpFor(nodep);
// Assignment to put before the referencing statement
AstAssign* const assignp
= new AstAssign{flp, new AstVarRef{flp, varp, VAccess::WRITE}, nodep};
AstVarRef* const refp = new AstVarRef{flp, varp, VAccess::WRITE};
AstAssign* const assignp = new AstAssign{flp, refp, nodep};
// Insert before the statement
m_stmtp->addHereThisAsNext(assignp);
}
@ -189,9 +217,11 @@ class PremitVisitor final : public VNVisitor {
// Direct assignment to a simple variable
if (VN_IS(nodep->lhsp(), VarRef) && !AstVar::scVarRecurse(nodep->lhsp())) {
AstNode* const rhsp = nodep->rhsp();
// Rhs is already a var ref, so nothing to so
// Rhs is already a var ref, so nothing to do
if (VN_IS(rhsp, VarRef) && !AstVar::scVarRecurse(rhsp)) return;
if (!VN_IS(rhsp, Const)) {
if (VN_IS(rhsp, Cond)) {
// Do replace Cond on RHS, even if a simple assignment
} else if (!VN_IS(rhsp, Const)) {
// Don't replace the rhs, it's already a simple assignment
rhsp->user1(true);
} else if (rhsp->width() < STATIC_CONST_MIN_WIDTH) {
@ -210,6 +240,15 @@ class PremitVisitor final : public VNVisitor {
m_assignLhs = true; // Restored by VL_RESTORER in START_STATEMENT_OR_RETURN
iterateAndNextNull(nodep->lhsp());
// It's possible we end up with an 'a' = 'a' after expanding an AstCond
if (AstVarRef* const rhsp = VN_CAST(nodep->rhsp(), VarRef)) {
if (AstVarRef* const lhsp = VN_CAST(nodep->lhsp(), VarRef)) {
if (lhsp->varp() == rhsp->varp()) {
VL_DO_DANGLING(pushDeletep(nodep->unlinkFrBack()), nodep);
}
}
}
}
void visit(AstDisplay* nodep) override {
START_STATEMENT_OR_RETURN(nodep);
@ -309,15 +348,41 @@ class PremitVisitor final : public VNVisitor {
checkNode(nodep);
}
void visit(AstCond* nodep) override {
iterateChildren(nodep);
if (nodep->thenp()->isWide() && !VN_IS(nodep->condp(), Const)
&& !VN_IS(nodep->condp(), VarRef)) {
// We're going to need the expression several times in the expanded code,
// so might as well make it a common expression
createTemp(nodep->condp());
// Convert AstCond to AstIf in order to avoid evaluating
// sub-expressions in both branches unconditionally.
if (needsTemp(nodep)) {
// Check if LHS variable could be used directly
AstVarRef* const lRefp = isRhsOfAssignToVar(nodep);
// If not, create a new temporary variable
AstVar* varp = lRefp ? lRefp->varp() : newTmpFor(nodep);
// Can't substitute across basic blocks
varp->noSubst(true);
FileLine* const flp = nodep->fileline();
// Create 'then' assignment
AstVarRef* const thenRefp = new AstVarRef{flp, varp, VAccess::WRITE};
if (lRefp) thenRefp->selfPointer(lRefp->selfPointer());
AstNodeExpr* const thenExprp = nodep->thenp()->unlinkFrBack();
AstAssign* const thenAsspp = new AstAssign{flp, thenRefp, thenExprp};
// Create 'else' assignment
AstVarRef* const elseRefp = new AstVarRef{flp, varp, VAccess::WRITE};
if (lRefp) elseRefp->selfPointer(lRefp->selfPointer());
AstNodeExpr* const elseExprp = nodep->elsep()->unlinkFrBack();
AstAssign* const elseAsspp = new AstAssign{flp, elseRefp, elseExprp};
// Creae 'if' and insert it before the statement
AstNodeExpr* const condp = nodep->condp()->unlinkFrBack();
AstIf* const ifp = new AstIf{flp, condp, thenAsspp, elseAsspp};
m_stmtp->addHereThisAsNext(ifp);
// Replace the AstCond with the result variable
nodep->replaceWith(new AstVarRef{flp, varp, VAccess::READ});
VL_DO_DANGLING(pushDeletep(nodep), nodep);
// Splitting to multiple statements can change purity
VIsCached::clearCacheTree();
// Iterate the resulting assignments
iterate(ifp);
return;
}
checkNode(nodep);
iterateChildren(nodep);
}
void visit(AstSFormatF* nodep) override {
iterateChildren(nodep);

2
test_regress/t/t_flag_expand_limit.py
View File

@ -13,6 +13,6 @@ test.scenarios('vlt')
test.compile(verilator_flags2=['--expand-limit 1 --stats -fno-dfg'])
test.file_grep(test.stats, r'Optimizations, expand limited\s+(\d+)', 8)
test.file_grep(test.stats, r'Optimizations, expand limited\s+(\d+)', 7)
test.passes()