Fix removing if statement with side effect in condition (#3131).

Changes

@@ -14,6 +14,7 @@ Verilator 4.213 devel
 * Include processor information in verilator_gantt data file.
 * Fix verilator_profcfunc profile accounting (#3115).
 * Fix display has no time units on class function (#3116). [Damien Pretet]
+* Fix removing if statement with side effect in condition (#3131). [Alexander Grobman]
 
 
 Verilator 4.212 2021-09-01

src/V3AstNodes.h

@@ -4436,6 +4436,7 @@ public:
     virtual string emitC() override { V3ERROR_NA_RETURN(""); }
     virtual bool isGateOptimizable() const override { return false; }
     virtual bool isPredictOptimizable() const override { return false; }
+    virtual bool isPure() const override { return !outp(); }
     virtual bool cleanOut() const override { return true; }
     virtual bool same(const AstNode* samep) const override { return true; }
     AstNode* searchp() const { return op1p(); }  // op1 = Search expression

src/V3Const.cpp

@@ -1312,6 +1312,15 @@ private:
         // but for now can disable en-mass until V3Purify takes effect.
         return m_doShort || VN_IS(nodep, VarRef) || VN_IS(nodep, Const);
     }
+    bool isTreePureRecurse(AstNode* nodep) {
+        // Should memoize this if call commonly
+        if (!nodep->isPure()) return false;
+        if (nodep->op1p() && !isTreePureRecurse(nodep->op1p())) return false;
+        if (nodep->op2p() && !isTreePureRecurse(nodep->op2p())) return false;
+        if (nodep->op3p() && !isTreePureRecurse(nodep->op3p())) return false;
+        if (nodep->op4p() && !isTreePureRecurse(nodep->op4p())) return false;
+        return true;
+    }
 
     // Extraction checks
     bool warnSelect(AstSel* nodep) {
@@ -2783,10 +2792,13 @@ private:
                 }
                 VL_DO_DANGLING(nodep->deleteTree(), nodep);
             } else if (!afterComment(nodep->ifsp()) && !afterComment(nodep->elsesp())) {
+                if (!isTreePureRecurse(nodep->condp())) {
+                    // Condition has side effect - leave - perhaps in
+                    // future simplify to remove all but side effect terms
+                } else {
                     // Empty block, remove it
-                // Note if we support more C++ then there might be side
-                // effects in the condition itself
                     VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
+                }
             } else if (!afterComment(nodep->ifsp())) {
                 UINFO(4, "IF({x}) nullptr {...} => IF(NOT{x}}: " << nodep << endl);
                 AstNode* condp = nodep->condp();

test_regress/t/t_sys_plusargs.v

@@ -103,6 +103,11 @@ module t;
       $display("i='%d'",p_i);
       if (p_i !== 32'd1234) $stop;
 
+      // bug3131 - really "if" side effect test
+      p_i = 0;
+      if ($value$plusargs("INT=%d", p_i)) ;
+      if (p_i !== 32'd1234) $stop;
+
       $write("*-* All Finished *-*\n");
       $finish;
    end