Internals: Make V3LiftExpr non optional (#8369)

src/V3LiftExpr.cpp

@@ -49,9 +49,8 @@
 //      __VleLogAnd_0 = __VleCall_0;
 //    }
 //    z = __VleLogAnd_1;
-// Similar patterns are used for AstLogOr and AstCond to preserve the
-// short-circuiting semantics and side effects. All AstLogIf should have
-// been converted to AstLogOr earlier by V3Const.
+// Similar patterns are used for AstLogOr, AstLogIf and AstCond to preserve
+// the short-circuiting semantics and side effects.
 //
 // Care must be taken for impure LValues as well. While, all LValue expressions
 // permitted by IEEE-1800 are themselves pure (except possibly for the non-lvalue
@@ -108,6 +107,7 @@ class LiftExprVisitor final : public VNVisitor {
     VDouble0 m_statLiftedConds;
     VDouble0 m_statLiftedLogAnds;
     VDouble0 m_statLiftedLogOrs;
+    VDouble0 m_statLiftedLogIfs;
     VDouble0 m_statLiftedExprStmts;
     VDouble0 m_statTemporariesCreated;
     VDouble0 m_statTemporariesReused;
@@ -432,7 +432,27 @@ class LiftExprVisitor final : public VNVisitor {
     }
     void visit(AstLogIf* nodep) override {
         if (!m_lift) return;
-        nodep->v3fatalSrc("AstLogIf should have been folded by V3Const");
+
+        // Lift from LHS
+        iterate(nodep->lhsp());
+
+        // Lift from RHS, if nothing lifted, then nothing to do
+        AstNode* const rhsStmtps = lift(nodep->rhsp());
+        if (!rhsStmtps) return;
+
+        // Otherwise convert to an AstIf with a temporary variable
+        ++m_statLiftedLogIfs;
+        FileLine* const flp = nodep->fileline();
+        AstVar* varp = getExistingVar(nodep->rhsp());
+        if (!varp) varp = newVar("LogIf", nodep);
+        addStmtps(new AstAssign{flp, new AstVarRef{flp, varp, VAccess::WRITE},
+                                new AstLogNot{flp, nodep->lhsp()->unlinkFrBack()}});
+        AstIf* const ifp = new AstIf{flp, new AstVarRef{flp, varp, VAccess::READ}};
+        addStmtps(ifp);
+        ifp->addElsesp(rhsStmtps);
+        ifp->addElsesp(assignIfDifferent(flp, varp, nodep->rhsp()));
+        nodep->replaceWith(new AstVarRef{flp, varp, VAccess::READ});
+        VL_DO_DANGLING(nodep->deleteTree(), nodep);
     }
     void visit(AstExprStmt* nodep) override {
         if (!m_lift) return;
@@ -476,6 +496,7 @@ public:
         V3Stats::addStat("LiftExpr, lifted Cond", m_statLiftedConds);
         V3Stats::addStat("LiftExpr, lifted LogAnd", m_statLiftedLogAnds);
         V3Stats::addStat("LiftExpr, lifted LogOr", m_statLiftedLogOrs);
+        V3Stats::addStat("LiftExpr, lifted LogIf", m_statLiftedLogIfs);
         V3Stats::addStat("LiftExpr, lifted ExprStmt", m_statLiftedExprStmts);
         V3Stats::addStat("LiftExpr, temporaries created", m_statTemporariesCreated);
         V3Stats::addStat("LiftExpr, temporaries reused", m_statTemporariesReused);

src/V3Options.cpp

@@ -1518,7 +1518,9 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc,
     DECL_OPTION("-finline-funcs-eager", FOnOff, &m_fInlineFuncsEager);
     DECL_OPTION("-flife", FOnOff, &m_fLife);
     DECL_OPTION("-flife-post", FOnOff, &m_fLifePost);
-    DECL_OPTION("-flift-expr", FOnOff, &m_fLiftExpr);
+    DECL_OPTION("-flift-expr", CbFOnOff, [fl](bool) {
+        fl->v3warn(DEPRECATED, "Option '-fno-lift-expr' is deprecated and has no effect");
+    });
     DECL_OPTION("-flocalize", FOnOff, &m_fLocalize);
     DECL_OPTION("-fmerge-cond", FOnOff, &m_fMergeCond);
     DECL_OPTION("-fmerge-cond-motion", FOnOff, &m_fMergeCondMotion);

src/V3Options.h

@@ -424,7 +424,6 @@ private:
     bool m_fInlineFuncsEager = true;  // main switch: -fno-inline-funcs-eager: don't inline eagerly
     bool m_fLife;        // main switch: -fno-life: variable lifetime
     bool m_fLifePost;    // main switch: -fno-life-post: delayed assignment elimination
-    bool m_fLiftExpr = true;   // main switch: -fno-lift-expr: lift expressions out of statements
     bool m_fLocalize;    // main switch: -fno-localize: convert temps to local variables
     bool m_fMergeCond;   // main switch: -fno-merge-cond: merge conditionals
     bool m_fMergeCondMotion = true; // main switch: -fno-merge-cond-motion: perform code motion
@@ -765,7 +764,6 @@ public:
     bool fInlineFuncsEager() const { return m_fInlineFuncsEager; }
     bool fLife() const { return m_fLife; }
     bool fLifePost() const { return m_fLifePost; }
-    bool fLiftExpr() const { return m_fLiftExpr; }
     bool fLocalize() const { return m_fLocalize; }
     bool fMergeCond() const { return m_fMergeCond; }
     bool fMergeCondMotion() const { return m_fMergeCondMotion; }

src/Verilator.cpp

@@ -307,7 +307,7 @@ static void process() {
 
         if (!v3Global.opt.serializeOnly()) {
             // Lift expressions out of statements.
-            if (v3Global.opt.fLiftExpr()) V3LiftExpr::liftExprAll(v3Global.rootp());
+            V3LiftExpr::liftExprAll(v3Global.rootp());
 
             // Move assignments from X into MODULE temps.
             // (Before flattening, so each new X variable is shared between all scopes of that