diff --git a/src/script_opt/ExprOptInfo.h b/src/script_opt/ExprOptInfo.h index b12cde6d97..274e9cd1f8 100644 --- a/src/script_opt/ExprOptInfo.h +++ b/src/script_opt/ExprOptInfo.h @@ -9,12 +9,12 @@ namespace zeek::detail { class ExprSideEffects { public: - ExprSideEffects(bool _has_side_effects) : has_side_effects(_has_side_effects) {} + ExprSideEffects(bool _has_side_effects) : has_side_effects(_has_side_effects) {} - bool HasSideEffects() const { return has_side_effects; } + bool HasSideEffects() const { return has_side_effects; } protected: - bool has_side_effects; + bool has_side_effects; }; class ExprOptInfo { diff --git a/src/script_opt/ProfileFunc.cc b/src/script_opt/ProfileFunc.cc index 0a5c3d456f..70b907614c 100644 --- a/src/script_opt/ProfileFunc.cc +++ b/src/script_opt/ProfileFunc.cc @@ -985,6 +985,21 @@ void ProfileFuncs::ComputeSideEffects() { auto a = ea.first; auto at = a->Tag(); if ( at == ATTR_DEFAULT || at == ATTR_DEFAULT_INSERT || at == ATTR_ON_CHANGE ) { + if ( at == ATTR_DEFAULT ) { + for ( auto t : ea.second ) { + if ( t->Tag() != TYPE_TABLE ) + continue; + + auto y = t->AsTableType()->Yield(); + + if ( y && IsAggr(y->Tag()) ) { + aggr_tbls_analyzed[t] = true; + for ( auto ta : type_aliases[t] ) + aggr_tbls_analyzed[ta] = true; + } + } + } + // Weed out very-common-and-completely-safe expressions. if ( ! DefinitelyHasNoSideEffects(a->GetExpr()) ) candidates.insert(a); @@ -995,6 +1010,8 @@ void ProfileFuncs::ComputeSideEffects() { } } + // ### + std::vector> side_effects; while ( ! candidates.empty() ) { @@ -1253,12 +1270,29 @@ bool ProfileFuncs::AssessSideEffects(const SideEffectsOp* se, SideEffectsOp::Acc return false; } +bool ProfileFuncs::IsTableWithDefaultAggr(const Type* t) { + auto analy = aggr_tbls_analyzed.find(t); + if ( analy != aggr_tbls_analyzed.end() ) + return analy->second; + + if ( t->AsTableType()->Yield() ) { + for ( auto& at : aggr_tbls_analyzed ) + if ( same_type(at.first, t) ) { + aggr_tbls_analyzed[t] = at.second; + return at.second; + } + } + + aggr_tbls_analyzed[t] = false; + return false; +} + bool ProfileFuncs::GetSideEffects(SideEffectsOp::AccessType access, const Type* t) const { IDSet nli; std::unordered_set aggrs; if ( GetSideEffects(access, t, nli, aggrs) ) - return true; + return true; return ! nli.empty() || ! aggrs.empty(); } diff --git a/src/script_opt/ProfileFunc.h b/src/script_opt/ProfileFunc.h index ca48f13cf9..fae9b650fd 100644 --- a/src/script_opt/ProfileFunc.h +++ b/src/script_opt/ProfileFunc.h @@ -228,11 +228,11 @@ protected: // the same type can be seen numerous times. std::unordered_set types; - std::unordered_map> type_aliases; - // The same, but in a deterministic order, with duplicates removed. std::vector ordered_types; + std::unordered_map> type_aliases; + // Script functions that this script calls. Includes calls made // by lambdas and when bodies, as the goal is to identify recursion. std::unordered_set script_calls; @@ -320,6 +320,8 @@ public: // types. // ### const auto& ExprAttrs() const { return expr_attrs; } + bool IsTableWithDefaultAggr(const Type* t); + // ### // true = unknown bool GetSideEffects(SideEffectsOp::AccessType access, const Type* t) const; @@ -447,6 +449,9 @@ protected: // the attribute appears. std::unordered_map> expr_attrs; + // ### + std::unordered_map aggr_tbls_analyzed; + std::unordered_map>> attr_side_effects; std::unordered_map>> record_constr_with_side_effects; diff --git a/src/script_opt/Reduce.cc b/src/script_opt/Reduce.cc index 207e2da22c..cfcb9132cd 100644 --- a/src/script_opt/Reduce.cc +++ b/src/script_opt/Reduce.cc @@ -429,27 +429,31 @@ bool Reducer::ExprValid(const ID* id, const Expr* e1, const Expr* e2) const { // If so, then it's never safe to reuse its associated identifier in lieu // of e2. std::optional& e1_se = e1->GetOptInfo()->SideEffects(); - if ( ! e1_se ) - { - bool has_side_effects; + if ( ! e1_se ) { + bool has_side_effects = false; - if ( e1->Tag() == EXPR_INDEX ) - has_side_effects = pfs.GetSideEffects(SideEffectsOp::READ, e1->GetOp1()->GetType().get()); + if ( e1->Tag() == EXPR_INDEX ) { + auto aggr = e1->GetOp1(); + auto aggr_t = aggr->GetType(); - else if ( e1->Tag() == EXPR_RECORD_CONSTRUCTOR || e1->Tag() == EXPR_RECORD_COERCE ) - has_side_effects = pfs.GetSideEffects(SideEffectsOp::CONSTRUCTION, e1->GetType().get()); - else - has_side_effects = false; + if ( pfs.GetSideEffects(SideEffectsOp::READ, aggr_t.get()) ) + has_side_effects = true; - e1_se = ExprSideEffects(has_side_effects); - } + else if ( aggr_t->Tag() == TYPE_TABLE && pfs.IsTableWithDefaultAggr(aggr_t.get()) ) + has_side_effects = true; + } - if ( e1_se->HasSideEffects() ) - { - // We already know that e2 is structurally identical to e1. + else if ( e1->Tag() == EXPR_RECORD_CONSTRUCTOR || e1->Tag() == EXPR_RECORD_COERCE ) + has_side_effects = pfs.GetSideEffects(SideEffectsOp::CONSTRUCTION, e1->GetType().get()); + + e1_se = ExprSideEffects(has_side_effects); + } + + if ( e1_se->HasSideEffects() ) { + // We already know that e2 is structurally identical to e1. e2->GetOptInfo()->SideEffects() = ExprSideEffects(true); - return false; - } + return false; + } // Here are the considerations for expression validity. // @@ -1045,7 +1049,8 @@ bool CSE_ValidityChecker::CheckID(const ID* id, bool ignore_orig) const { if ( id_t && same_type(id_t, i->GetType()) ) { // Same-type aggregate. - // if ( ! ignore_orig ) printf("identifier %s (%d), start %s, end %s\n", id->Name(), ignore_orig, obj_desc(start_e).c_str(), obj_desc(end_e).c_str()); + // if ( ! ignore_orig ) printf("identifier %s (%d), start %s, end %s\n", id->Name(), ignore_orig, + // obj_desc(start_e).c_str(), obj_desc(end_e).c_str()); if ( ignore_orig ) return true; }