diff --git a/src/V3Begin.cpp b/src/V3Begin.cpp index 36e151053..2ac71c770 100644 --- a/src/V3Begin.cpp +++ b/src/V3Begin.cpp @@ -61,7 +61,6 @@ class BeginVisitor final : public VNVisitor { // NODE STATE // AstCase::user1 -> bool, if already purified - V3UniqueNames m_caseTempNames; // For generating unique temporary variable names used by cases // STATE - across all visitors BeginState* const m_statep; // Current global state @@ -75,7 +74,6 @@ class BeginVisitor final : public VNVisitor { string m_unnamedScope; // Name of begin blocks, including unnamed blocks int m_ifDepth = 0; // Current if depth bool m_keepBegins = false; // True if begins should not be inlined - VDouble0 m_statPurifiedCaseExpr; // Count of purified case expressions // METHODS @@ -132,7 +130,6 @@ class BeginVisitor final : public VNVisitor { } void visit(AstNodeModule* nodep) override { VL_RESTORER(m_modp); - VL_RESTORER(m_caseTempNames); m_modp = nodep; // Rename it (e.g. class under a generate) if (m_unnamedScope != "") { @@ -171,7 +168,6 @@ class BeginVisitor final : public VNVisitor { VL_RESTORER(m_liftedp); VL_RESTORER(m_namedScope); VL_RESTORER(m_unnamedScope); - VL_RESTORER(m_caseTempNames); m_displayScope = dot(m_displayScope, nodep->name()); m_namedScope = ""; m_unnamedScope = ""; @@ -415,29 +411,7 @@ class BeginVisitor final : public VNVisitor { // any BEGINs, but V3Coverage adds them all under the module itself. iterateChildren(nodep); } - void visit(AstCase* nodep) override { - // Introduce temporary variable for AstCase if needed - it is done here and not in V3Case - // because this phase is before V3Scope and V3Case is not. Doing it before V3Scope ensures - // that V3Scope will take care of a scope creation - if (!nodep->exprp()->isPure() && !nodep->user1SetOnce()) { - ++m_statPurifiedCaseExpr; - FileLine* const fl = nodep->exprp()->fileline(); - AstVar* const varp = new AstVar{fl, VVarType::XTEMP, m_caseTempNames.get(nodep), - nodep->exprp()->dtypep()}; - nodep->exprp(new AstExprStmt{fl, - new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, - nodep->exprp()->unlinkFrBack()}, - new AstVarRef{fl, varp, VAccess::READ}}); - if (m_ftaskp) { - varp->funcLocal(true); - varp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); - m_ftaskp->stmtsp()->addHereThisAsNext(varp); - } else { - m_modp->stmtsp()->addHereThisAsNext(varp); - } - } - iterateChildren(nodep); - } + void visit(AstCase* nodep) override { iterateChildren(nodep); } // VISITORS - LINT CHECK void visit(AstIf* nodep) override { // not AstNodeIf; other types not covered VL_RESTORER(m_keepBegins); @@ -464,10 +438,8 @@ class BeginVisitor final : public VNVisitor { public: // CONSTRUCTORS BeginVisitor(AstNetlist* nodep, BeginState* statep) - : m_caseTempNames{"__VCase"} - , m_statep{statep} { + : m_statep{statep} { iterate(nodep); - V3Stats::addStatSum("Impure case expressions", m_statPurifiedCaseExpr); } ~BeginVisitor() override = default; }; diff --git a/src/V3Case.cpp b/src/V3Case.cpp index 4eca73e01..6c969c791 100644 --- a/src/V3Case.cpp +++ b/src/V3Case.cpp @@ -147,7 +147,6 @@ class CaseVisitor final : public VNVisitor { bool m_caseNoOverlapsAllCovered = false; // Proven to be synopsys parallel_case compliant // For each possible value, the case branch we need std::array m_valueItem; - bool m_needToClearCache = false; // Whether cache needs to be cleared // METHODS //! Determine whether we should check case items are complete @@ -398,14 +397,6 @@ class CaseVisitor final : public VNVisitor { // CASEx(cexpr,.... // -> tree of IF(msb, IF(msb-1, 11, 10) // IF(msb-1, 01, 00)) - AstNodeExpr* cexprp; - AstExprStmt* cexprStmtp = nullptr; - if (nodep->exprp()->isPure()) { - cexprp = nodep->exprp()->unlinkFrBack(); - } else { - cexprStmtp = VN_AS(nodep->exprp()->unlinkFrBack(), ExprStmt); - cexprp = cexprStmtp->resultp()->cloneTreePure(false); - } if (debug() >= 9) { // LCOV_EXCL_START for (uint32_t i = 0; i < (1UL << m_caseWidth); ++i) { @@ -419,14 +410,7 @@ class CaseVisitor final : public VNVisitor { replaceCaseParallel(nodep, m_caseNoOverlapsAllCovered); AstNode::user3ClearTree(); - AstNode* ifrootp = replaceCaseFastRecurse(cexprp, m_caseWidth - 1, 0UL); - if (cexprStmtp) { - cexprStmtp->resultp()->unlinkFrBack()->deleteTree(); - AstIf* const ifp = VN_AS(ifrootp, If); - cexprStmtp->resultp(ifp->condp()->unlinkFrBack()); - ifp->condp(cexprStmtp); - m_needToClearCache = true; - } + AstNode* ifrootp = replaceCaseFastRecurse(nodep->exprp(), m_caseWidth - 1, 0UL); // Case expressions can't be linked twice, so clone them if (ifrootp && !ifrootp->user3()) ifrootp = ifrootp->cloneTree(true); @@ -437,7 +421,6 @@ class CaseVisitor final : public VNVisitor { nodep->unlinkFrBack(); } VL_DO_DANGLING(nodep->deleteTree(), nodep); - VL_DO_DANGLING(cexprp->deleteTree(), cexprp); UINFOTREE(9, ifrootp, "", "_simp"); } @@ -446,14 +429,7 @@ class CaseVisitor final : public VNVisitor { // -> IF((cexpr==icond1),istmts1, // IF((EQ (AND MASK cexpr) (AND MASK icond1) // ,istmts2, istmts3 - AstNodeExpr* cexprp; - AstExprStmt* cexprStmtp = nullptr; - if (nodep->exprp()->isPure()) { - cexprp = nodep->exprp()->unlinkFrBack(); - } else { - cexprStmtp = VN_AS(nodep->exprp(), ExprStmt)->unlinkFrBack(); - cexprp = cexprStmtp->resultp()->cloneTreePure(false); - } + AstNodeExpr* const cexprp = nodep->exprp(); // We'll do this in two stages. First stage, convert the conditions to // the appropriate IF AND terms. UINFOTREE(9, nodep, "", "_comp_IN::"); @@ -518,7 +494,6 @@ class CaseVisitor final : public VNVisitor { itemp->addCondsp(ifexprp); } } - VL_DO_DANGLING(cexprp->deleteTree(), cexprp); if (!hadDefault) { // If there was no default, add a empty one, this greatly simplifies below code // and constant propagation will just eliminate it for us later. @@ -582,12 +557,6 @@ class CaseVisitor final : public VNVisitor { if (grouprootp) { UINFOTREE(9, grouprootp, "", "_new"); nodep->replaceWith(grouprootp); - if (cexprStmtp) { - pushDeletep(cexprStmtp->resultp()->unlinkFrBack()); - cexprStmtp->resultp(grouprootp->condp()->unlinkFrBack()); - grouprootp->condp(cexprStmtp); - m_needToClearCache = true; - } } else { nodep->unlinkFrBack(); } @@ -622,6 +591,8 @@ class CaseVisitor final : public VNVisitor { { CaseLintVisitor{nodep}; } iterateChildren(nodep); UINFOTREE(9, nodep, "", "case_old"); + UASSERT_OBJ(nodep->exprp()->isPure(), nodep, + "Impure case expression should have been removed by V3LiftExpr"); if (isCaseTreeFast(nodep) && v3Global.opt.fCase()) { // It's a simple priority encoder or complete statement // we can make a tree of statements to avoid extra comparisons @@ -648,7 +619,6 @@ public: explicit CaseVisitor(AstNetlist* nodep) { for (auto& itr : m_valueItem) itr = nullptr; iterate(nodep); - if (m_needToClearCache) VIsCached::clearCacheTree(); } ~CaseVisitor() override { V3Stats::addStat("Optimizations, Cases parallelized", m_statCaseFast); diff --git a/test_regress/t/t_case_call_count.py b/test_regress/t/t_case_call_count.py index 4116217e8..df91d0e3c 100755 --- a/test_regress/t/t_case_call_count.py +++ b/test_regress/t/t_case_call_count.py @@ -15,6 +15,6 @@ test.compile(verilator_flags2=['--stats']) test.execute() -test.file_grep(test.stats, r'Impure case expressions\s+(\d+)', 2) +test.file_grep(test.stats, r'LiftExpr, lifted calls\s+(\d+)', 3) test.passes() diff --git a/test_regress/t/t_case_inside_call_count.py b/test_regress/t/t_case_inside_call_count.py index 4116217e8..df91d0e3c 100755 --- a/test_regress/t/t_case_inside_call_count.py +++ b/test_regress/t/t_case_inside_call_count.py @@ -15,6 +15,6 @@ test.compile(verilator_flags2=['--stats']) test.execute() -test.file_grep(test.stats, r'Impure case expressions\s+(\d+)', 2) +test.file_grep(test.stats, r'LiftExpr, lifted calls\s+(\d+)', 3) test.passes()