diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 949ef9f79..43df3fac9 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -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) // diff --git a/src/V3Premit.cpp b/src/V3Premit.cpp index 199e6f5b7..50c23da38 100644 --- a/src/V3Premit.cpp +++ b/src/V3Premit.cpp @@ -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); diff --git a/test_regress/t/t_flag_expand_limit.py b/test_regress/t/t_flag_expand_limit.py index 8cacdb1ca..ec0cc54a6 100755 --- a/test_regress/t/t_flag_expand_limit.py +++ b/test_regress/t/t_flag_expand_limit.py @@ -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()