diff --git a/Changes b/Changes index dc07f520b..63b868510 100644 --- a/Changes +++ b/Changes @@ -19,6 +19,7 @@ Verilator 5.017 devel * Support resizing function call inout arguments (#4467). * Support converting parameters inside modules to localparams (#4511). [Anthony Donlon] * Change lint_off to not propagate upwards to files including where the lint_off is. +* Optimize empty expression statements (#4544). * Fix conversion of impure logical expressions to bit expressions (#487 partial) (#4437). [Ryszard Rozak, Antmicro Ltd.] * Fix enum functions in localparams (#3999). [Andrew Nolte] * Fix passing arguments by reference (#3385 partial) (#4489). [Ryszard Rozak, Antmicro Ltd.] diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 35f6606f9..3e26ad7cd 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -33,6 +33,8 @@ VL_DEFINE_DEBUG_FUNCTIONS; //====================================================================== // Statics +uint32_t VIsCached::s_cachedCntGbl = 1; + uint64_t AstNode::s_editCntLast = 0; uint64_t AstNode::s_editCntGbl = 0; // Hot cache line diff --git a/src/V3Ast.h b/src/V3Ast.h index a2fbe15cd..a6daaa584 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -169,15 +169,31 @@ inline std::ostream& operator<<(std::ostream& os, const VLifetime& rhs) { class VIsCached final { // Used in some nodes to cache results of boolean methods + // If cachedCnt == 0, not cached + // else if cachedCnt == s_cachedCntGbl, then m_state is if cached + uint32_t m_cachedCnt : 31; // Mark of when cache was computed + uint32_t m_state : 1; + static uint32_t s_cachedCntGbl; // Global computed count + static constexpr uint32_t MAX_CNT = (1UL << 31) - 1; // Max for m_cachedCnt + public: - enum en : uint8_t { NOT_CACHED, YES, NO }; - enum en m_e; VIsCached() - : m_e{NOT_CACHED} {} - bool isCached() const { return m_e != NOT_CACHED; } - bool get() const { return m_e == YES; } - void set(bool flag) { m_e = flag ? YES : NO; } - void clearCache() { m_e = NOT_CACHED; } + : m_cachedCnt{0} + , m_state{0} {} + bool isCached() const { return m_cachedCnt == s_cachedCntGbl; } + bool get() const { return m_state; } + void set(bool flag) { + m_cachedCnt = s_cachedCntGbl; + m_state = flag; + } + void clearCache() { + m_cachedCnt = 0; + m_state = 0; + } + static void clearCacheTree() { + ++s_cachedCntGbl; + UASSERT_STATIC(s_cachedCntGbl < MAX_CNT, "Overflow of cache counting"); + } }; // ###################################################################### diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 727cdca6e..002b980c5 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -1176,7 +1176,10 @@ public: string emitVerilog() override { V3ERROR_NA_RETURN(""); } string emitC() override { V3ERROR_NA_RETURN(""); } bool cleanOut() const override { return true; } - bool isPure() override { return false; } + bool isPure() override { + if (AstNode::afterCommentp(stmtsp())) return false; + return resultp()->isPure(); + } bool same(const AstNode*) const override { return true; } }; class AstFError final : public AstNodeExpr { diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 6432a7eb5..031269f78 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -2689,6 +2689,15 @@ private: << nodep->varp()->prettyNameQ()); } } + void visit(AstExprStmt* nodep) override { + iterateChildren(nodep); + if (!AstNode::afterCommentp(nodep->stmtsp())) { + UINFO(8, "ExprStmt(...) " << nodep << " " << nodep->resultp() << endl); + nodep->replaceWith(nodep->resultp()->unlinkFrBack()); + VL_DO_DANGLING(nodep->deleteTree(), nodep); + // Removing the ExprStmt might have made something impure above now pure + } + } void visit(AstEnumItemRef* nodep) override { iterateChildren(nodep); UASSERT_OBJ(nodep->itemp(), nodep, "Not linked"); @@ -3718,6 +3727,7 @@ public: } AstNode* mainAcceptEdit(AstNode* nodep) { + VIsCached::clearCacheTree(); // Avoid using any stale isPure // Operate starting at a random place return iterateSubtreeReturnEdits(nodep); } diff --git a/src/V3Dead.cpp b/src/V3Dead.cpp index 6a0f8ae7a..428538e1c 100644 --- a/src/V3Dead.cpp +++ b/src/V3Dead.cpp @@ -533,6 +533,7 @@ public: // We may have removed some datatypes, cleanup nodep->typeTablep()->repairCache(); + VIsCached::clearCacheTree(); // Removing assignments may affect isPure } ~DeadVisitor() override = default; }; diff --git a/src/V3Life.cpp b/src/V3Life.cpp index a20c17252..5fd69a27f 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -517,5 +517,6 @@ void V3Life::lifeAll(AstNetlist* nodep) { LifeState state; LifeTopVisitor{nodep, &state}; } // Destruct before checking + VIsCached::clearCacheTree(); // Removing assignments may affect isPure V3Global::dumpCheckGlobalTree("life", 0, dumpTreeLevel() >= 3); } diff --git a/test_regress/t/t_func_real_exprstmt.pl b/test_regress/t/t_func_real_exprstmt.pl index 859050d63..e8d0aa63b 100755 --- a/test_regress/t/t_func_real_exprstmt.pl +++ b/test_regress/t/t_func_real_exprstmt.pl @@ -11,8 +11,14 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); compile( + verilator_flags2 => ["--stats"], ); +if ($Self->{vlt_all}) { + # Check no EXPRSTMTs in final output - should get optimized away + file_grep_not($Self->{stats}, qr/Node count, EXPRSTMT/i); +} + execute( check_finished => 1, );