Internals: Cleanup case condition lifting (#7768)

V3Begin used to lift impure case expressions. With V3LiftExpr, this is
now redundant.

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

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<AstNode*, 1 << CASE_OVERLAP_WIDTH> 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);

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()

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()