From 769a3d958a88708cf3c184e5a54c47a610191b87 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:29:26 -0700 Subject: [PATCH 01/10] some minor tidying of -O gen-C++ sources --- src/script_opt/CPP/Driver.cc | 7 ++--- src/script_opt/CPP/Exprs.cc | 2 +- src/script_opt/CPP/Func.h | 14 ++-------- src/script_opt/CPP/InitsInfo.h | 25 ++++++++++++------ src/script_opt/CPP/RuntimeOps.cc | 2 +- src/script_opt/CPP/RuntimeOps.h | 44 +++++++++++++++++++------------- src/script_opt/CPP/RuntimeVec.cc | 2 +- src/script_opt/CPP/Stmts.cc | 2 +- src/script_opt/ScriptOpt.cc | 4 ++- 9 files changed, 56 insertions(+), 46 deletions(-) diff --git a/src/script_opt/CPP/Driver.cc b/src/script_opt/CPP/Driver.cc index 3dc7e991fc..95061a1f7c 100644 --- a/src/script_opt/CPP/Driver.cc +++ b/src/script_opt/CPP/Driver.cc @@ -37,11 +37,12 @@ void CPPCompile::Compile(bool report_uncompilable) { // previously compiled instances of those if present. for ( auto& func : funcs ) { const auto& f = func.Func(); + auto& body = func.Body(); auto& ofiles = analysis_options.only_files; auto allow_cond = analysis_options.allow_cond; - string fn = func.Body()->GetLocationInfo()->filename; + string fn = body->GetLocationInfo()->filename; if ( ! allow_cond && ! func.ShouldSkip() && ! ofiles.empty() && files_with_conditionals.count(fn) > 0 ) { if ( report_uncompilable ) @@ -184,8 +185,8 @@ void CPPCompile::GenProlog() { Emit("namespace CPP_%s { // %s\n", Fmt(total_hash), string(working_dir)); // The following might-or-might-not wind up being populated/used. - Emit("std::vector field_mapping;"); - Emit("std::vector enum_mapping;"); + Emit("std::vector field_mapping;"); + Emit("std::vector enum_mapping;"); NL(); const_info[TYPE_BOOL] = CreateConstInitInfo("Bool", "ValPtr", "bool"); diff --git a/src/script_opt/CPP/Exprs.cc b/src/script_opt/CPP/Exprs.cc index 40eba322b6..9628801e48 100644 --- a/src/script_opt/CPP/Exprs.cc +++ b/src/script_opt/CPP/Exprs.cc @@ -1248,7 +1248,7 @@ string CPPCompile::GenEnum(const TypePtr& t, const ValPtr& ev) { if ( ! et->HasRedefs() ) // Can use direct access. - return std::to_string(v); + return "zeek_int_t(" + std::to_string(v) + ")"; // Need to dynamically map the access. int mapping_slot; diff --git a/src/script_opt/CPP/Func.h b/src/script_opt/CPP/Func.h index 5b37866783..4642c2fdaa 100644 --- a/src/script_opt/CPP/Func.h +++ b/src/script_opt/CPP/Func.h @@ -8,9 +8,7 @@ #include "zeek/Func.h" #include "zeek/script_opt/ProfileFunc.h" -namespace zeek { - -namespace detail { +namespace zeek::detail { // A subclass of Func used for lambdas that the compiler creates for // complex initializations (expressions used in type attributes). @@ -42,11 +40,6 @@ public: const std::string& Name() { return name; } - // Sets/returns a hash associated with this statement. A value - // of 0 means "not set". - p_hash_type GetHash() const { return hash; } - void SetHash(p_hash_type h) { hash = h; } - // The following only get defined by lambda bodies. virtual void SetLambdaCaptures(Frame* f) {} virtual std::vector SerializeLambdaCaptures() const { return std::vector{}; } @@ -64,7 +57,6 @@ protected: TraversalCode Traverse(TraversalCallback* cb) const override { return TC_CONTINUE; } std::string name; - p_hash_type hash = 0ULL; // A pseudo AST "call" node, used to support error localization. CallExprPtr ce; @@ -117,6 +109,4 @@ extern std::unordered_map standalone_callbacks; // Callbacks to finalize initialization of standalone compiled scripts. extern std::vector standalone_finalizations; -} // namespace detail - -} // namespace zeek +} // namespace zeek::detail diff --git a/src/script_opt/CPP/InitsInfo.h b/src/script_opt/CPP/InitsInfo.h index cbdaaf806d..c2bba9fbd6 100644 --- a/src/script_opt/CPP/InitsInfo.h +++ b/src/script_opt/CPP/InitsInfo.h @@ -18,7 +18,7 @@ // standalone globals (for example, one for each BiF that a compiled script // may call). // -// For each of these types of initialization, our general approach is to a +// For each of these types of initialization, our general approach is to have a // class that manages a single instance of that type, and an an object that // manages all of those instances collectively. The latter object will, for // example, attend to determining the offset into the run-time vector associated @@ -48,8 +48,15 @@ // safely use cohort(X) = cohort(Y).) We then execute run-time initialization // in waves, one cohort at a time. // +// Many forms of initialization are specified in terms of indices into globals +// that hold items of various types. Thus, the most common initialization +// information is a vector of integers/indices. These data structures can +// be recursive, too, namely we sometimes associate an index with a vector +// of integers/indices and then we can track multiple such vectors using +// another vector of integers/indices. +// // Because C++ compilers can struggle when trying to optimize large quantities -// of code - clang in particular could take many CPU *hours* back when our +// of code - clang in particular could take many CPU *hours* back when the // compiler just generated C++ code snippets for each initialization - rather // than producing code that directly executes each given initialization, we // instead employ a table-driven approach. The C++ initializers for the @@ -58,12 +65,14 @@ // cohort at a time) to obtain the information needed to initialize any given // item. // -// Many forms of initialization are specified in terms of indices into globals -// that hold items of various types. Thus, the most common initialization -// information is a vector of integers/indices. These data structures can -// be recursive, too, namely we sometimes associate an index with a vector -// of integers/indices and then we can track multiple such vectors using -// another vector of integers/indices. +// Even this has headaches for very large initializations: both clang and g++ +// are *much* slower to initialize large vectors of simple template types +// (such as std::pair) than non-template types (such as a struct with two +// fields, which is all std::pair is, at the end of the day). A similar problem +// holds for initializing vectors-of-vectors-of-vectors, so we reduce these +// cases to simpler forms (structs for the first example, a single vector +// with information embedded within it for how to expand its values into +// a vector-of-vector-of-vector fr the second). #include "zeek/File.h" #include "zeek/Val.h" diff --git a/src/script_opt/CPP/RuntimeOps.cc b/src/script_opt/CPP/RuntimeOps.cc index 680f4648cc..b6bfde35b8 100644 --- a/src/script_opt/CPP/RuntimeOps.cc +++ b/src/script_opt/CPP/RuntimeOps.cc @@ -91,7 +91,7 @@ ValPtr when_index_slice__CPP(VectorVal* vec, const ListVal* lv) { return v; } -ValPtr when_invoke__CPP(Func* f, std::vector args, Frame* frame, void* caller_addr) { +ValPtr when_invoke__CPP(Func* f, ValVec args, Frame* frame, void* caller_addr) { auto trigger = frame->GetTrigger(); if ( trigger ) { diff --git a/src/script_opt/CPP/RuntimeOps.h b/src/script_opt/CPP/RuntimeOps.h index 5ef5ba0efa..1722f4c9bc 100644 --- a/src/script_opt/CPP/RuntimeOps.h +++ b/src/script_opt/CPP/RuntimeOps.h @@ -10,6 +10,8 @@ namespace zeek { +using IntVec = std::vector; +using ValVec = std::vector; using SubNetValPtr = IntrusivePtr; namespace detail { @@ -27,21 +29,21 @@ extern bool str_in__CPP(const String* s1, const String* s2); // Converts a vector of individual ValPtr's into a single ListValPtr // suitable for indexing an aggregate. -extern ListValPtr index_val__CPP(std::vector indices); +extern ListValPtr index_val__CPP(ValVec indices); // Returns the value corresponding to indexing the given table/vector/string // with the given set of indices. These are functions rather than something // generated directly so that they can package up the error handling for // the case where there's no such index. "patstr" refers to indexing a // table[pattern] of X with a string value. -extern ValPtr index_table__CPP(const TableValPtr& t, std::vector indices); -extern ValPtr index_patstr_table__CPP(const TableValPtr& t, std::vector indices); +extern ValPtr index_table__CPP(const TableValPtr& t, ValVec indices); +extern ValPtr index_patstr_table__CPP(const TableValPtr& t, ValVec indices); extern ValPtr index_vec__CPP(const VectorValPtr& vec, int index); -extern ValPtr index_string__CPP(const StringValPtr& svp, std::vector indices); +extern ValPtr index_string__CPP(const StringValPtr& svp, ValVec indices); // The same, but for indexing happening inside a "when" clause. -extern ValPtr when_index_table__CPP(const TableValPtr& t, std::vector indices); -extern ValPtr when_index_patstr__CPP(const TableValPtr& t, std::vector indices); +extern ValPtr when_index_table__CPP(const TableValPtr& t, ValVec indices); +extern ValPtr when_index_patstr__CPP(const TableValPtr& t, ValVec indices); extern ValPtr when_index_vec__CPP(const VectorValPtr& vec, int index); // For vector slices, we use the existing index_slice(), but we need a @@ -50,7 +52,7 @@ extern ValPtr when_index_slice__CPP(VectorVal* vec, const ListVal* lv); // Calls out to the given script or BiF function, which does not return // a value. -inline ValPtr invoke_void__CPP(Func* f, std::vector args, Frame* frame) { return f->Invoke(&args, frame); } +inline ValPtr invoke_void__CPP(Func* f, ValVec args, Frame* frame) { return f->Invoke(&args, frame); } // Used for error propagation by failed calls. class CPPInterpreterException : public InterpreterException {}; @@ -58,7 +60,7 @@ class CPPInterpreterException : public InterpreterException {}; // Calls out to the given script or BiF function. A separate function because // of the need to (1) construct the "args" vector using {} initializers, // but (2) needing to have the address of that vector. -inline ValPtr invoke__CPP(Func* f, std::vector args, Frame* frame) { +inline ValPtr invoke__CPP(Func* f, ValVec args, Frame* frame) { auto v = f->Invoke(&args, frame); if ( ! v ) throw CPPInterpreterException(); @@ -71,7 +73,7 @@ inline ValPtr invoke__CPP(Func* f, std::vector args, Frame* frame) { // last argument is the address of the calling function; we just need // it to be distinct to the call, so we can associate a Trigger cache // with it. -extern ValPtr when_invoke__CPP(Func* f, std::vector args, Frame* frame, void* caller_addr); +extern ValPtr when_invoke__CPP(Func* f, ValVec args, Frame* frame, void* caller_addr); // Thrown when a call inside a "when" delays. class CPPDelayedCallException : public InterpreterException {}; @@ -201,29 +203,35 @@ inline VectorValPtr vector_coerce__CPP(const ValPtr& v, const TypePtr& t) { return make_intrusive(cast_intrusive(t)); } +// Takes parallel vectors of attribute tags and values and returns a +// collective AttributesPtr corresponding to those instantiated attributes. +// For attributes that don't have associated expressions, the corresponding +// value should be nil. + +extern AttributesPtr build_attrs__CPP(IntVec attr_tags, std::vector attr_vals); + // Constructs a set of the given type, containing the given elements, and // with the associated attributes. -extern TableValPtr set_constructor__CPP(std::vector elements, TableTypePtr t, std::vector attr_tags, - std::vector attr_vals); +extern TableValPtr set_constructor__CPP(ValVec elements, TableTypePtr t, IntVec attr_tags, ValVec attr_vals); // Constructs a table of the given type, containing the given elements // (specified as parallel index/value vectors), and with the associated // attributes. -extern TableValPtr table_constructor__CPP(std::vector indices, std::vector vals, TableTypePtr t, - std::vector attr_tags, std::vector attr_vals); +extern TableValPtr table_constructor__CPP(ValVec indices, ValVec vals, TableTypePtr t, IntVec attr_tags, + ValVec attr_vals); // Assigns a set of attributes to an identifier. -extern void assign_attrs__CPP(IDPtr id, std::vector attr_tags, std::vector attr_vals); +extern void assign_attrs__CPP(IDPtr id, IntVec attr_tags, ValVec attr_vals); // Constructs a record of the given type, whose (ordered) fields are // assigned to the corresponding elements of the given vector of values. -extern RecordValPtr record_constructor__CPP(std::vector vals, RecordTypePtr t); +extern RecordValPtr record_constructor__CPP(ValVec vals, RecordTypePtr t); // Same, but with a map when using a named constructor. -extern RecordValPtr record_constructor_map__CPP(std::vector vals, std::vector map, RecordTypePtr t); +extern RecordValPtr record_constructor_map__CPP(ValVec vals, IntVec map, RecordTypePtr t); // Constructs a vector of the given type, populated with the given values. -extern VectorValPtr vector_constructor__CPP(std::vector vals, VectorTypePtr t); +extern VectorValPtr vector_constructor__CPP(ValVec vals, VectorTypePtr t); // For patterns, executes p1 += p2. inline PatternValPtr re_append__CPP(const PatternValPtr& p1, const PatternValPtr& p2) { @@ -234,7 +242,7 @@ inline PatternValPtr re_append__CPP(const PatternValPtr& p1, const PatternValPtr // Schedules an event to occur at the given absolute time, parameterized // with the given set of values. A separate function to facilitate avoiding // the scheduling if Zeek is terminating. -extern ValPtr schedule__CPP(double dt, EventHandlerPtr event, std::vector args); +extern ValPtr schedule__CPP(double dt, EventHandlerPtr event, ValVec args); // Simple helper functions for supporting absolute value. inline zeek_uint_t iabs__CPP(zeek_int_t v) { return v < 0 ? -v : v; } diff --git a/src/script_opt/CPP/RuntimeVec.cc b/src/script_opt/CPP/RuntimeVec.cc index 4a06800de3..62eed2dde2 100644 --- a/src/script_opt/CPP/RuntimeVec.cc +++ b/src/script_opt/CPP/RuntimeVec.cc @@ -109,7 +109,7 @@ VEC_OP1(comp, ~, ) } // Analogous to VEC_OP1, instantiates a function for a given binary operation, -// with customimzable kernels for "int" and "double" operations. +// with customizable kernels for "int" and "double" operations. // This version is for operations whose result type is the same as the // operand type. #define VEC_OP2(name, op, int_kernel, double_kernel, zero_check, is_bool) \ diff --git a/src/script_opt/CPP/Stmts.cc b/src/script_opt/CPP/Stmts.cc index 06837fcf87..fe8d66fbe0 100644 --- a/src/script_opt/CPP/Stmts.cc +++ b/src/script_opt/CPP/Stmts.cc @@ -352,7 +352,7 @@ void CPPCompile::GenWhenStmt(const WhenStmt* w) { if ( ret_type && ret_type->Tag() != TYPE_VOID ) { // Note, ret_type can be active but we *still* don't have - // a return type, due to the faked-up "any" return type + // a return value, due to the faked-up "any" return type // associated with "when" lambdas, so check for that case. Emit("if ( curr_t )"); StartBlock(); diff --git a/src/script_opt/ScriptOpt.cc b/src/script_opt/ScriptOpt.cc index d09e9dd25b..bc3dd6d1bc 100644 --- a/src/script_opt/ScriptOpt.cc +++ b/src/script_opt/ScriptOpt.cc @@ -401,7 +401,6 @@ static void use_CPP() { ++num_used; auto b = s->second.body; - b->SetHash(hash); // We may have already updated the body if // we're using code compiled for standalone. @@ -532,6 +531,9 @@ static void analyze_scripts_for_ZAM() { } void clear_script_analysis() { + if ( analysis_options.gen_CPP ) + return; + IDOptInfo::ClearGlobalInitExprs(); // We need to explicitly clear out the optimization information From 03347e235b2a120851973a443edcaec37265e03b Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:32:23 -0700 Subject: [PATCH 02/10] fix for script optimization of "in" operations --- src/script_opt/Expr.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/script_opt/Expr.cc b/src/script_opt/Expr.cc index 353604f1a7..b727171ee3 100644 --- a/src/script_opt/Expr.cc +++ b/src/script_opt/Expr.cc @@ -115,6 +115,9 @@ bool Expr::IsReducedConditional(Reducer* c) const { return NonReduced(this); if ( op1->Tag() == EXPR_LIST ) { + if ( ! op1->IsReduced(c) ) + return NonReduced(this); + auto l1 = op1->AsListExpr(); auto& l1_e = l1->Exprs(); From 5a3b519fb4ae9ea20539d2700dd2da724aedffe0 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:33:00 -0700 Subject: [PATCH 03/10] fix for script optimization of constants of type "opaque" --- src/Expr.h | 11 ++++++++++- src/script_opt/Expr.cc | 6 ++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Expr.h b/src/Expr.h index 73929a2114..c03bf4a76f 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -474,7 +474,16 @@ public: // Optimization-related: ExprPtr Duplicate() override; - ValPtr FoldVal() const override { return val; } + + ValPtr FoldVal() const override { + if ( type->Tag() == TYPE_OPAQUE ) + // Aggressive constant propagation can lead to the appearance of + // opaque "constants". Don't consider these as foldable because + // they're problematic to generate independently. + return nullptr; + + return val; + } protected: void ExprDescribe(ODesc* d) const override; diff --git a/src/script_opt/Expr.cc b/src/script_opt/Expr.cc index b727171ee3..84e70e6543 100644 --- a/src/script_opt/Expr.cc +++ b/src/script_opt/Expr.cc @@ -475,7 +475,8 @@ ExprPtr UnaryExpr::Reduce(Reducer* c, StmtPtr& red_stmt) { auto op_val = op->FoldVal(); if ( op_val ) { auto fold = Fold(op_val.get()); - return TransformMe(make_intrusive(fold), c, red_stmt); + if ( fold->GetType()->Tag() != TYPE_OPAQUE ) + return TransformMe(make_intrusive(fold), c, red_stmt); } if ( c->Optimizing() ) @@ -523,7 +524,8 @@ ExprPtr BinaryExpr::Reduce(Reducer* c, StmtPtr& red_stmt) { auto op2_fold_val = op2->FoldVal(); if ( op1_fold_val && op2_fold_val ) { auto fold = Fold(op1_fold_val.get(), op2_fold_val.get()); - return TransformMe(make_intrusive(fold), c, red_stmt); + if ( fold->GetType()->Tag() != TYPE_OPAQUE ) + return TransformMe(make_intrusive(fold), c, red_stmt); } if ( c->Optimizing() ) From 77c34787f3fcc346eb75cd1d34e4d24e8e1b0860 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:37:06 -0700 Subject: [PATCH 04/10] header tweaks to provide gen-C++ script optimization with more flexibility --- src/Func.h | 6 +++--- src/Type.h | 2 ++ src/script_opt/CPP/RuntimeOps.cc | 8 ++------ src/script_opt/CPP/RuntimeOps.h | 11 ++++++++++- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Func.h b/src/Func.h index 8b31976e69..f6808484d5 100644 --- a/src/Func.h +++ b/src/Func.h @@ -291,6 +291,9 @@ protected: */ virtual void SetCaptures(Frame* f); + // Captures when using ZVal block instead of a Frame. + std::unique_ptr> captures_vec; + private: size_t frame_size = 0; @@ -304,9 +307,6 @@ private: OffsetMap* captures_offset_mapping = nullptr; - // Captures when using ZVal block instead of a Frame. - std::unique_ptr> captures_vec; - // The most recently added/updated body ... StmtPtr current_body; diff --git a/src/Type.h b/src/Type.h index c5c52bc095..4f1f4e702d 100644 --- a/src/Type.h +++ b/src/Type.h @@ -35,6 +35,7 @@ class CompositeHash; class Expr; class ListExpr; class ZAMCompiler; +class CPPRuntime; using ExprPtr = IntrusivePtr; using ListExprPtr = IntrusivePtr; @@ -752,6 +753,7 @@ private: class CreationInitsOptimizer; friend zeek::RecordVal; friend zeek::detail::ZAMCompiler; + friend zeek::detail::CPPRuntime; const auto& DeferredInits() const { return deferred_inits; } const auto& CreationInits() const { return creation_inits; } diff --git a/src/script_opt/CPP/RuntimeOps.cc b/src/script_opt/CPP/RuntimeOps.cc index b6bfde35b8..03973a2e74 100644 --- a/src/script_opt/CPP/RuntimeOps.cc +++ b/src/script_opt/CPP/RuntimeOps.cc @@ -194,11 +194,7 @@ void remove_element__CPP(TableValPtr aggr, ListValPtr indices) { check_iterators__CPP(iterators_invalidated); } -// A helper function that takes a parallel vectors of attribute tags -// and values and returns a collective AttributesPtr corresponding to -// those instantiated attributes. For attributes that don't have -// associated expressions, the corresponding value should be nil. -static AttributesPtr build_attrs__CPP(vector attr_tags, vector attr_vals) { +AttributesPtr build_attrs__CPP(IntVec attr_tags, vector attr_vals) { vector attrs; int nattrs = attr_tags.size(); for ( auto i = 0; i < nattrs; ++i ) { @@ -243,7 +239,7 @@ TableValPtr table_constructor__CPP(vector indices, vector vals, return aggr; } -void assign_attrs__CPP(IDPtr id, std::vector attr_tags, std::vector attr_vals) { +void assign_attrs__CPP(IDPtr id, IntVec attr_tags, ValVec attr_vals) { id->SetAttrs(build_attrs__CPP(std::move(attr_tags), std::move(attr_vals))); } diff --git a/src/script_opt/CPP/RuntimeOps.h b/src/script_opt/CPP/RuntimeOps.h index 1722f4c9bc..6a31f8c1a7 100644 --- a/src/script_opt/CPP/RuntimeOps.h +++ b/src/script_opt/CPP/RuntimeOps.h @@ -18,7 +18,16 @@ namespace detail { class CPPRuntime { public: - static auto RawOptField(const RecordValPtr& rv, int field) { return rv->RawOptField(field); } + static auto& RawField(const RecordValPtr& rv, int field) { return rv->RawField(field); } + static auto& RawField(RecordVal* rv, int field) { return rv->RawField(field); } + static auto& RawOptField(const RecordValPtr& rv, int field) { return rv->RawOptField(field); } + static auto& RawOptField(RecordVal* rv, int field) { return rv->RawOptField(field); } + + static const auto& GetCreationInits(const RecordType* rt) { return rt->CreationInits(); } + + static RecordVal* BuildRecordVal(RecordTypePtr t, std::vector> init_vals) { + return new RecordVal(std::move(t), std::move(init_vals)); + } }; // Returns the concatenation of the given strings. From a93a69ba628c14c3598173278fec0e361ff9c1a2 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:41:51 -0700 Subject: [PATCH 05/10] -O gen-C++ fix for dealing with use of more than one module qualifier --- src/script_opt/CPP/Vars.cc | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/script_opt/CPP/Vars.cc b/src/script_opt/CPP/Vars.cc index 4aa967c3f8..44fbd103cd 100644 --- a/src/script_opt/CPP/Vars.cc +++ b/src/script_opt/CPP/Vars.cc @@ -111,10 +111,30 @@ string CPPCompile::LocalName(const ID* l) const { auto n = l->Name(); auto without_module = strstr(n, "::"); - if ( without_module ) - return Canonicalize(without_module + 2); - else - return Canonicalize(n); + while ( without_module ) { + n = without_module + 2; + without_module = strstr(n, "::"); + } + + return Canonicalize(n); +} + +string CPPCompile::CaptureName(const ID* l) const { + // We want to strip both the module and any inlining appendage. + auto n = l->Name(); + auto without_module = strstr(n, "::"); + + while ( without_module ) { + n = without_module + 2; + without_module = strstr(n, "::"); + } + + auto appendage = strchr(n, '.'); + + if ( appendage ) + return string(n, appendage - n) + "_"; + + return string(n) + "_"; } string CPPCompile::Canonicalize(const char* name) const { @@ -127,7 +147,7 @@ string CPPCompile::Canonicalize(const char* name) const { if ( c == '<' || c == '>' ) continue; - if ( c == ':' || c == '-' ) + if ( c == ':' || c == '-' || c == '.' ) c = '_'; cname += c; From 6daf9d5b8836db2e59886fba5bdc30d057bb8ec3 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:42:25 -0700 Subject: [PATCH 06/10] fixes for -O gen-C++ generation of floating point constants --- src/script_opt/CPP/Util.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/script_opt/CPP/Util.cc b/src/script_opt/CPP/Util.cc index 5ad4846aa3..15fc981ede 100644 --- a/src/script_opt/CPP/Util.cc +++ b/src/script_opt/CPP/Util.cc @@ -17,6 +17,16 @@ string Fmt(double d) { if ( d == 0.0 && signbit(d) ) return "-0.0"; + if ( isinf(d) ) { + string infty = "std::numeric_limits::infinity()"; + if ( d < 0.0 ) + infty = "-" + infty; + return infty; + } + + if ( isnan(d) ) + return "std::numeric_limits::quiet_NaN()"; + // Unfortunately, to_string(double) is hardwired to use %f with // default of 6 digits precision. char buf[8192]; From 0ca2f9a8b27e5add83476d42db12adc135de0c6b Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:43:17 -0700 Subject: [PATCH 07/10] speedups for compilation of initializers in -O gen-C++ generated code --- src/script_opt/CPP/Compile.h | 2 + src/script_opt/CPP/Inits.cc | 2 +- src/script_opt/CPP/InitsInfo.cc | 89 +++++++++++++++++++++++------- src/script_opt/CPP/InitsInfo.h | 19 +++++-- src/script_opt/CPP/RuntimeInits.cc | 20 ++++++- src/script_opt/CPP/RuntimeInits.h | 57 ++++++++++++------- 6 files changed, 139 insertions(+), 50 deletions(-) diff --git a/src/script_opt/CPP/Compile.h b/src/script_opt/CPP/Compile.h index e825b1a0a9..975956d79d 100644 --- a/src/script_opt/CPP/Compile.h +++ b/src/script_opt/CPP/Compile.h @@ -143,6 +143,7 @@ public: // cohort associated with a given type. int TypeOffset(const TypePtr& t) { return GI_Offset(RegisterType(t)); } int TypeCohort(const TypePtr& t) { return GI_Cohort(RegisterType(t)); } + int TypeFinalCohort(const TypePtr& t) { return GI_FinalCohort(RegisterType(t)); } // Tracks a Zeek ValPtr used as a constant value. These occur // in two contexts: directly as constant expressions, and indirectly @@ -963,6 +964,7 @@ private: // associated with an initialization. int GI_Offset(const std::shared_ptr& gi) const { return gi ? gi->Offset() : -1; } int GI_Cohort(const std::shared_ptr& gi) const { return gi ? gi->InitCohort() : 0; } + int GI_FinalCohort(const std::shared_ptr& gi) const { return gi ? gi->FinalInitCohort() : 0; } // Generate code to initialize the mappings for record field // offsets for field accesses into regions of records that diff --git a/src/script_opt/CPP/Inits.cc b/src/script_opt/CPP/Inits.cc index 4e7a30b145..ba30011745 100644 --- a/src/script_opt/CPP/Inits.cc +++ b/src/script_opt/CPP/Inits.cc @@ -166,7 +166,7 @@ void CPPCompile::InitializeConsts() { StartBlock(); for ( const auto& c : consts ) - Emit("CPP_ValElem(%s, %s),", TypeTagName(c.first), Fmt(c.second)); + Emit("{%s, %s},", TypeTagName(c.first), Fmt(c.second)); EndBlock(true); } diff --git a/src/script_opt/CPP/InitsInfo.cc b/src/script_opt/CPP/InitsInfo.cc index b4c03bf541..848c38e15b 100644 --- a/src/script_opt/CPP/InitsInfo.cc +++ b/src/script_opt/CPP/InitsInfo.cc @@ -7,6 +7,7 @@ #include "zeek/ZeekString.h" #include "zeek/script_opt/CPP/Attrs.h" #include "zeek/script_opt/CPP/Compile.h" +#include "zeek/script_opt/CPP/RuntimeInits.h" using namespace std; @@ -38,6 +39,13 @@ void CPP_InitsInfo::GenerateInitializers(CPPCompile* c) { c->Emit("%s %s = %s(%s, %s,", gt, InitializersName(), gt, base_name, Fmt(offset_set)); c->IndentUp(); + GenerateCohorts(c); + c->IndentDown(); + + c->Emit(");"); +} + +void CPP_InitsInfo::GenerateCohorts(CPPCompile* c) { c->Emit("{"); int n = 0; @@ -47,7 +55,7 @@ void CPP_InitsInfo::GenerateInitializers(CPPCompile* c) { if ( ++n > 1 ) c->Emit(""); - if ( cohort.size() == 1 && ! IsCompound() ) + if ( cohort.size() == 1 && ! UsesCompoundVectors() ) BuildCohort(c, cohort); else { c->Emit("{"); @@ -57,8 +65,6 @@ void CPP_InitsInfo::GenerateInitializers(CPPCompile* c) { } c->Emit("}"); - c->IndentDown(); - c->Emit(");"); } void CPP_InitsInfo::BuildOffsetSet(CPPCompile* c) { @@ -80,25 +86,25 @@ void CPP_InitsInfo::BuildOffsetSet(CPPCompile* c) { offset_set = c->IndMgr().AddIndices(offsets_vec); } -void CPP_InitsInfo::BuildCohort(CPPCompile* c, std::vector>& cohort) { - int n = 0; +static std::string describe_initializer(const Obj* o) { + auto od = obj_desc(o); + // Escape any embedded comment characters. + od = regex_replace(od, std::regex("/\\*"), "<>"); + od = regex_replace(od, std::regex("\\*/"), "<>"); + + return od; +} + +void CPP_InitsInfo::BuildCohort(CPPCompile* c, std::vector>& cohort) { for ( auto& co : cohort ) { vector ivs; auto o = co->InitObj(); - if ( o ) { - auto od = obj_desc(o); - - // Escape any embedded comment characters. - od = regex_replace(od, std::regex("/\\*"), "<>"); - od = regex_replace(od, std::regex("\\*/"), "<>"); - - c->Emit("/* #%s: Initializing %s: */", Fmt(co->Offset()), od); - } + if ( o ) + c->Emit("/* #%s: Initializing %s: */", Fmt(co->Offset()), describe_initializer(o)); co->InitializerVals(ivs); BuildCohortElement(c, co->InitializerType(), ivs); - ++n; } } @@ -117,12 +123,50 @@ void CPP_InitsInfo::BuildCohortElement(CPPCompile* c, string init_type, vectorEmit("std::make_shared<%s>(%s),", init_type, full_init); } +void CPP_CompoundInitsInfo::GenerateInitializers(CPPCompile* c) { + c->Emit(""); + c->Emit("static int %s_init[] = {", tag); + int n = 0; + + c->IndentUp(); + + for ( auto& cohort : instances ) { + if ( ++n > 1 ) + c->Emit(""); + + // Figure out the size of the cohort. + for ( auto& co : cohort ) { + auto o = co->InitObj(); + if ( o ) + c->Emit("/* #%s: Initializing %s: */", Fmt(co->Offset()), describe_initializer(o)); + + vector ivs; + co->InitializerVals(ivs); + c->Emit(Fmt(int(ivs.size())) + ","); + BuildCohortElement(c, co->InitializerType(), ivs); + } + + static const auto end_of_vv = Fmt(END_OF_VEC_VEC) + ","; + c->Emit(end_of_vv); + } + + static const auto end_of_vvv = Fmt(END_OF_VEC_VEC_VEC) + ","; + c->Emit(end_of_vvv); + + c->IndentDown(); + c->Emit("};"); + + CPP_InitsInfo::GenerateInitializers(c); +} + +void CPP_CompoundInitsInfo::GenerateCohorts(CPPCompile* c) { c->Emit("%s_init", tag); } + void CPP_CompoundInitsInfo::BuildCohortElement(CPPCompile* c, string init_type, vector& ivs) { string init_line; for ( auto& iv : ivs ) - init_line += iv + ", "; + init_line += iv + ","; - c->Emit("{ %s},", init_line); + c->Emit("%s", init_line); } void CPP_BasicConstInitsInfo::BuildCohortElement(CPPCompile* c, string init_type, vector& ivs) { @@ -174,7 +218,7 @@ PatternConstInfo::PatternConstInfo(CPPCompile* c, ValPtr v) : CPP_InitInfo(v) { CompoundItemInfo::CompoundItemInfo(CPPCompile* _c, ValPtr v) : CPP_InitInfo(v), c(_c) { auto& t = v->GetType(); type = c->TypeOffset(t); - init_cohort = c->TypeCohort(t) + 1; + init_cohort = c->TypeFinalCohort(t) + 1; } ListConstInfo::ListConstInfo(CPPCompile* _c, ValPtr v) : CompoundItemInfo(_c) { @@ -400,7 +444,11 @@ void TypeTypeInfo::AddInitializerVals(std::vector& ivs) const { } VectorTypeInfo::VectorTypeInfo(CPPCompile* _c, TypePtr _t) : AbstractTypeInfo(_c, std::move(_t)) { - yield = t->Yield(); + auto vt = t->AsVectorType(); + if ( vt->IsUnspecifiedVector() ) + yield = base_type(TYPE_VOID); + else + yield = t->Yield(); auto gi = c->RegisterType(yield); if ( gi ) init_cohort = gi->InitCohort(); @@ -552,7 +600,8 @@ void IndicesManager::Generate(CPPCompile* c) { c->Emit(line); } - c->Emit("-1"); + static const auto end_of_vv = Fmt(END_OF_VEC_VEC); + c->Emit(end_of_vv); c->EndBlock(true); } diff --git a/src/script_opt/CPP/InitsInfo.h b/src/script_opt/CPP/InitsInfo.h index c2bba9fbd6..d2b097a304 100644 --- a/src/script_opt/CPP/InitsInfo.h +++ b/src/script_opt/CPP/InitsInfo.h @@ -133,10 +133,10 @@ public: // Sets the associated C++ type. virtual void SetCPPType(std::string ct) { CPP_type = std::move(ct); } - // Whether this initializer is in terms of compound objects. Used + // Whether this initializer is in terms of compound vectors. Used // for avoiding compiler warnings about singleton initializations in // braces. - virtual bool IsCompound() const { return false; } + virtual bool UsesCompoundVectors() const { return false; } // Returns the type associated with the table used for initialization // (i.e., this is the type of the global returned by InitializersName()). @@ -146,9 +146,11 @@ public: void AddInstance(std::shared_ptr g); // Emit code to populate the table used to initialize this collection. - void GenerateInitializers(CPPCompile* c); + virtual void GenerateInitializers(CPPCompile* c); protected: + virtual void GenerateCohorts(CPPCompile* c); + // Computes offset_set - see below. void BuildOffsetSet(CPPCompile* c); @@ -214,7 +216,7 @@ public: BuildInitType(); } - bool IsCompound() const override { return true; } + bool UsesCompoundVectors() const override { return true; } private: void BuildInitType() { inits_type = std::string("CPP_CustomInits<") + CPPType() + ">"; } @@ -236,7 +238,7 @@ public: inits_type = std::string("CPP_BasicConsts<") + CPP_type + ", " + c_type + ", " + tag + "Val>"; } - bool IsCompound() const override { return false; } + bool UsesCompoundVectors() const override { return false; } void BuildCohortElement(CPPCompile* c, std::string init_type, std::vector& ivs) override; }; @@ -254,7 +256,12 @@ public: inits_type = std::string("CPP_IndexedInits<") + CPPType() + ">"; } - bool IsCompound() const override { return true; } + // This isn't true (anymore) because we separately build up the compound + // vectors needed for the initialization. + bool UsesCompoundVectors() const override { return false; } + + void GenerateInitializers(CPPCompile* c) override; + void GenerateCohorts(CPPCompile* c) override; void BuildCohortElement(CPPCompile* c, std::string init_type, std::vector& ivs) override; }; diff --git a/src/script_opt/CPP/RuntimeInits.cc b/src/script_opt/CPP/RuntimeInits.cc index 80c2337f63..99837d08e4 100644 --- a/src/script_opt/CPP/RuntimeInits.cc +++ b/src/script_opt/CPP/RuntimeInits.cc @@ -465,12 +465,12 @@ void CPP_GlobalInit::Generate(InitsManager* im, std::vector& /* inits_vec global->SetAttrs(im->Attributes(attrs)); } -void generate_indices_set(int* inits, std::vector>& indices_set) { +size_t generate_indices_set(int* inits, std::vector>& indices_set) { // First figure out how many groups of indices there are, so we // can pre-allocate the outer vector. auto i_ptr = inits; int num_inits = 0; - while ( *i_ptr >= 0 ) { + while ( *i_ptr != END_OF_VEC_VEC && *i_ptr != END_OF_VEC_VEC_VEC ) { ++num_inits; int n = *i_ptr; i_ptr += n + 1; // skip over vector elements @@ -479,7 +479,7 @@ void generate_indices_set(int* inits, std::vector>& indices_set indices_set.reserve(num_inits); i_ptr = inits; - while ( *i_ptr >= 0 ) { + while ( *i_ptr != END_OF_VEC_VEC ) { int n = *i_ptr; ++i_ptr; std::vector indices; @@ -490,6 +490,20 @@ void generate_indices_set(int* inits, std::vector>& indices_set indices_set.emplace_back(std::move(indices)); } + + return i_ptr - inits + 1; +} + +std::vector>> generate_indices_set(int* inits) { + std::vector>> indices_set; + + while ( *inits != END_OF_VEC_VEC_VEC ) { + std::vector> cohort_inits; + inits += generate_indices_set(inits, cohort_inits); + indices_set.push_back(std::move(cohort_inits)); + } + + return indices_set; } } // namespace zeek::detail diff --git a/src/script_opt/CPP/RuntimeInits.h b/src/script_opt/CPP/RuntimeInits.h index fe718e8226..268d2ca250 100644 --- a/src/script_opt/CPP/RuntimeInits.h +++ b/src/script_opt/CPP/RuntimeInits.h @@ -19,6 +19,28 @@ using FuncValPtr = IntrusivePtr; class InitsManager; +// Helper function that takes a (large) array of int's and from them +// constructs the corresponding vector-of-vector-of-indices. Each +// vector-of-indices is represented first by an int specifying its +// size, and then that many int's for its values. We recognize the +// end of the array upon encountering a "size" entry of END_OF_VEC_VEC. +// +// Returns how many elements were processed out of "inits", including its +// terminator. +extern size_t generate_indices_set(int* inits, std::vector>& indices_set); + +// The same but for one more level of vector construction. The source array +// has sub-arrays terminated with END_OF_VEC_VEC per the above, and the whole +// shebang is terminated with END_OF_VEC_VEC_VEC. +// +// Returns the vector construction. +extern std::vector>> generate_indices_set(int* inits); + +// These need to be distinct from any values that can appear, which means +// they should be negative, and not -1, which is used as a "N/A" value. +#define END_OF_VEC_VEC -100 +#define END_OF_VEC_VEC_VEC -200 + // An abstract helper class used to access elements of an initialization vector. // We need the abstraction because InitsManager below needs to be able to refer // to any of a range of templated classes. @@ -29,7 +51,12 @@ public: }; // Convenient way to refer to an offset associated with a particular Zeek type. -using CPP_ValElem = std::pair; +// A "struct" rather than a std::pair because C++ compilers are terribly slow +// at initializing large numbers of the latter. +struct CPP_ValElem { + TypeTag tag; + int offset; +}; // This class groups together all of the vectors needed for run-time // initialization. We gather them together into a single object so as @@ -57,7 +84,7 @@ public: // index. ValPtr ConstVals(int offset) const { auto& cv = const_vals[offset]; - return Consts(cv.first, cv.second); + return Consts(cv.tag, cv.offset); } // Retrieves the Zeek constant value for a particular Zeek type. @@ -157,9 +184,6 @@ protected: // Pre-initialize all elements requiring it. virtual void DoPreInits(InitsManager* im, const std::vector& offsets_vec) {} - // Generate a single element. - virtual void GenerateElement(InitsManager* im, T2& init, int offset) {} - // The initialization vector in its entirety. std::vector& inits_vec; @@ -221,16 +245,16 @@ using ValElemVecVec = std::vector; template class CPP_IndexedInits : public CPP_AbstractInits { public: - CPP_IndexedInits(std::vector& _inits_vec, int _offsets_set, std::vector _inits) - : CPP_AbstractInits(_inits_vec, _offsets_set, std::move(_inits)) {} + CPP_IndexedInits(std::vector& _inits_vec, int _offsets_set, int* raw_inits) + : CPP_AbstractInits(_inits_vec, _offsets_set, generate_indices_set(raw_inits)) {} protected: void InitializeCohortWithOffsets(InitsManager* im, int cohort, const std::vector& cohort_offsets) override; - // Note, in the following we pass in the inits_vec, even though - // the method will have direct access to it, because we want to - // use overloading to dispatch to custom generation for different - // types of values. + // Note, in the following we pass in the inits_vec ("ivec"), even though + // the method will have direct access to it, because we want to use + // overloading to dispatch to custom generation for different types of + // values. void Generate(InitsManager* im, std::vector& ivec, int offset, ValElemVec& init_vals); void Generate(InitsManager* im, std::vector& ivec, int offset, ValElemVec& init_vals); void Generate(InitsManager* im, std::vector& ivec, int offset, ValElemVec& init_vals); @@ -254,8 +278,8 @@ protected: // on subclasses of TypePtr. class CPP_TypeInits : public CPP_IndexedInits { public: - CPP_TypeInits(std::vector& _inits_vec, int _offsets_set, std::vector> _inits) - : CPP_IndexedInits(_inits_vec, _offsets_set, _inits) {} + CPP_TypeInits(std::vector& _inits_vec, int _offsets_set, int* raw_inits) + : CPP_IndexedInits(_inits_vec, _offsets_set, raw_inits) {} protected: void DoPreInits(InitsManager* im, const std::vector& offsets_vec) override; @@ -504,11 +528,4 @@ struct CPP_RegisterBody { std::vector events; }; -// Helper function that takes a (large) array of int's and from them -// constructs the corresponding vector-of-vector-of-indices. Each -// vector-of-indices is represented first by an int specifying its -// size, and then that many int's for its values. We recognize the -// end of the array upon encountering a "size" entry of -1. -extern void generate_indices_set(int* inits, std::vector>& indices_set); - } // namespace zeek::detail From 207b82ae4bb1461ec9e9f92840793273027973e3 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:45:33 -0700 Subject: [PATCH 08/10] robustness improvements for -O gen-C++ generation of lambdas / "when"s --- src/script_opt/CPP/Compile.h | 12 +++++++++++- src/script_opt/CPP/DeclFunc.cc | 23 +++++++++++++---------- src/script_opt/CPP/Exprs.cc | 16 +++++++++------- src/script_opt/CPP/Func.h | 1 + src/script_opt/CPP/GenFunc.cc | 25 ++++++++++++++++--------- src/script_opt/CPP/Stmts.cc | 24 ++++++++++++++++-------- 6 files changed, 66 insertions(+), 35 deletions(-) diff --git a/src/script_opt/CPP/Compile.h b/src/script_opt/CPP/Compile.h index 975956d79d..060dae6fa7 100644 --- a/src/script_opt/CPP/Compile.h +++ b/src/script_opt/CPP/Compile.h @@ -385,6 +385,10 @@ private: std::string LocalName(const ID* l) const; std::string LocalName(const IDPtr& l) const { return LocalName(l.get()); } + // The same, but for a capture. + std::string CaptureName(const ID* l) const; + std::string CaptureName(const IDPtr& l) const { return CaptureName(l.get()); } + // Returns a canonicalized name, with various non-alphanumeric // characters stripped or transformed, and guaranteed not to // conflict with C++ keywords. @@ -585,8 +589,11 @@ private: // Maps function names to events relevant to them. std::unordered_map> body_events; + // Full type of the function we're currently compiling. + FuncTypePtr func_type; + // Return type of the function we're currently compiling. - TypePtr ret_type = nullptr; + TypePtr ret_type; // Internal name of the function we're currently compiling. std::string body_name; @@ -697,6 +704,8 @@ private: void GenValueSwitchStmt(const Expr* e, const case_list* cases); void GenWhenStmt(const WhenStmt* w); + void GenWhenStmt(const WhenInfo* wi, const std::string& when_lambda, const Location* loc, + std::vector local_aggrs); void GenForStmt(const ForStmt* f); void GenForOverTable(const ExprPtr& tbl, const IDPtr& value_var, const IDPList* loop_vars); void GenForOverVector(const ExprPtr& tbl, const IDPtr& value_var, const IDPList* loop_vars); @@ -771,6 +780,7 @@ private: std::string GenSizeExpr(const Expr* e, GenType gt); std::string GenScheduleExpr(const Expr* e); std::string GenLambdaExpr(const Expr* e); + std::string GenLambdaExpr(const Expr* e, std::string capture_args); std::string GenIsExpr(const Expr* e, GenType gt); std::string GenArithCoerceExpr(const Expr* e, GenType gt); diff --git a/src/script_opt/CPP/DeclFunc.cc b/src/script_opt/CPP/DeclFunc.cc index 9ff0eb4134..f76c8acf46 100644 --- a/src/script_opt/CPP/DeclFunc.cc +++ b/src/script_opt/CPP/DeclFunc.cc @@ -31,7 +31,7 @@ void CPPCompile::DeclareLambda(const LambdaExpr* l, const ProfileFunc* pf) { auto& ids = l->OuterIDs(); for ( auto id : ids ) - lambda_names[id] = LocalName(id); + lambda_names[id] = CaptureName(id); CreateFunction(l_id->GetType(), pf, lname, body, 0, l, FUNC_FLAVOR_FUNCTION); } @@ -40,7 +40,12 @@ void CPPCompile::CreateFunction(const FuncTypePtr& ft, const ProfileFunc* pf, co int priority, const LambdaExpr* l, FunctionFlavor flavor) { const auto& yt = ft->Yield(); in_hook = flavor == FUNC_FLAVOR_HOOK; - const IDPList* lambda_ids = l ? &l->OuterIDs() : nullptr; + + IDPList effective_lambda_ids; + if ( l ) + effective_lambda_ids = l->OuterIDs(); + + const IDPList* lambda_ids = l ? &effective_lambda_ids : nullptr; string args = BindArgs(ft, lambda_ids); @@ -313,7 +318,7 @@ void CPPCompile::GatherParamTypes(vector& p_types, const FuncTypePtr& ft auto tn = FullTypeName(t); // Allow the captures to be modified. - p_types.emplace_back(string(tn) + "& "); + p_types.emplace_back(string(tn) + "&"); } } @@ -328,18 +333,16 @@ void CPPCompile::GatherParamNames(vector& p_names, const FuncTypePtr& ft if ( param_id ) { if ( t->Tag() == TYPE_ANY && param_id->GetType()->Tag() != TYPE_ANY ) - // We'll need to translate the parameter - // from its current representation to - // type "any". + // We'll need to translate the parameter from its current + // representation to type "any". p_names.emplace_back(string("any_param__CPP_") + Fmt(i)); else p_names.emplace_back(LocalName(param_id)); } else - // Parameters that are unused don't wind up in the - // ProfileFunc. Rather than dig their name out of - // the function's declaration, we explicitly name - // them to reflect that they're unused. + // Parameters that are unused don't wind up in the ProfileFunc. + // Rather than dig their name out of the function's declaration, + // we explicitly name them to reflect that they're unused. p_names.emplace_back(string("unused_param__CPP_") + Fmt(i)); } diff --git a/src/script_opt/CPP/Exprs.cc b/src/script_opt/CPP/Exprs.cc index 9628801e48..c653d56e47 100644 --- a/src/script_opt/CPP/Exprs.cc +++ b/src/script_opt/CPP/Exprs.cc @@ -639,13 +639,15 @@ string CPPCompile::GenScheduleExpr(const Expr* e) { } string CPPCompile::GenLambdaExpr(const Expr* e) { + auto l = static_cast(e); + auto& body = l->Ingredients()->Body(); + return GenLambdaExpr(e, GenLambdaClone(l, false)); +} + +string CPPCompile::GenLambdaExpr(const Expr* e, string capture_args) { auto l = static_cast(e); auto name = Canonicalize(l->Name().c_str()) + "_lb_cl"; - auto cl_args = string("\"") + name + "\""; - - if ( l->OuterIDs().size() > 0 ) - cl_args = cl_args + GenLambdaClone(l, false); - + auto cl_args = string("\"") + name + "\"" + std::move(capture_args); auto body = string("make_intrusive<") + name + ">(" + cl_args + ")"; auto func = string("make_intrusive(\"") + l->Name() + "\", cast_intrusive(" + GenTypeName(l->GetType()) + "), " + body + ")"; @@ -1175,7 +1177,7 @@ string CPPCompile::GenLambdaClone(const LambdaExpr* l, bool all_deep) { for ( const auto& id : ids ) { const auto& id_t = id->GetType(); - auto arg = LocalName(id); + auto arg = CaptureName(id); if ( captures && ! IsNativeType(id_t) ) { for ( const auto& c : *captures ) @@ -1183,7 +1185,7 @@ string CPPCompile::GenLambdaClone(const LambdaExpr* l, bool all_deep) { arg = string("cast_intrusive<") + TypeName(id_t) + ">(" + arg + "->Clone())"; } - cl_args = cl_args + ", " + arg; + cl_args += ", " + arg; } return cl_args; diff --git a/src/script_opt/CPP/Func.h b/src/script_opt/CPP/Func.h index 4642c2fdaa..9a12cb2921 100644 --- a/src/script_opt/CPP/Func.h +++ b/src/script_opt/CPP/Func.h @@ -77,6 +77,7 @@ protected: // Methods related to sending lambdas via Broker. std::optional SerializeCaptures() const override; void SetCaptures(Frame* f) override; + void SetCapturesVec(std::unique_ptr> _captures_vec) { captures_vec = std::move(_captures_vec); } FuncPtr DoClone() override; diff --git a/src/script_opt/CPP/GenFunc.cc b/src/script_opt/CPP/GenFunc.cc index 63ec0a4448..fc3dad0c48 100644 --- a/src/script_opt/CPP/GenFunc.cc +++ b/src/script_opt/CPP/GenFunc.cc @@ -38,12 +38,18 @@ void CPPCompile::GenInvokeBody(const string& call, const TypePtr& t) { void CPPCompile::DefineBody(const FuncTypePtr& ft, const ProfileFunc* pf, const string& fname, const StmtPtr& body, const IDPList* lambda_ids, FunctionFlavor flavor) { + IDPList l_ids; + if ( lambda_ids ) + l_ids = *lambda_ids; + locals.clear(); params.clear(); body_name = fname; + func_type = ft; ret_type = ft->Yield(); + in_hook = flavor == FUNC_FLAVOR_HOOK; auto ret_type_str = in_hook ? "bool" : FullTypeName(ret_type); @@ -52,7 +58,7 @@ void CPPCompile::DefineBody(const FuncTypePtr& ft, const ProfileFunc* pf, const NL(); - Emit("%s %s(%s)", ret_type_str, fname, ParamDecl(ft, lambda_ids, pf)); + Emit("%s %s(%s)", ret_type_str, fname, ParamDecl(ft, &l_ids, pf)); StartBlock(); @@ -64,7 +70,7 @@ void CPPCompile::DefineBody(const FuncTypePtr& ft, const ProfileFunc* pf, const InitializeEvents(pf); // Create the local variables. - DeclareLocals(pf, lambda_ids); + DeclareLocals(pf, &l_ids); GenStmt(body); @@ -135,11 +141,12 @@ void CPPCompile::InitializeEvents(const ProfileFunc* pf) { } void CPPCompile::DeclareLocals(const ProfileFunc* pf, const IDPList* lambda_ids) { - // It's handy to have a set of the lambda captures rather than a list. - IDSet lambda_set; + // We track captures by their names rather than their ID*'s because the + // latter can be inconsistent when inlining. + set capture_names; if ( lambda_ids ) for ( auto li : *lambda_ids ) - lambda_set.insert(li); + capture_names.insert(CaptureName(li)); const auto& ls = pf->Locals(); @@ -149,11 +156,11 @@ void CPPCompile::DeclareLocals(const ProfileFunc* pf, const IDPList* lambda_ids) for ( const auto& l : ls ) { auto ln = LocalName(l); + auto cn = CaptureName(l); - if ( lambda_set.count(l) > 0 ) - // No need to declare these, they're passed in as - // parameters. - ln = lambda_names[l]; + if ( capture_names.count(cn) > 0 ) + // No need to declare these, they're passed in as parameters. + ln = cn; else if ( params.count(l) == 0 ) { // Not a parameter, so must be a local. Emit("%s %s;", FullTypeName(l->GetType()), ln); diff --git a/src/script_opt/CPP/Stmts.cc b/src/script_opt/CPP/Stmts.cc index fe8d66fbe0..43e196f756 100644 --- a/src/script_opt/CPP/Stmts.cc +++ b/src/script_opt/CPP/Stmts.cc @@ -305,15 +305,22 @@ void CPPCompile::GenValueSwitchStmt(const Expr* e, const case_list* cases) { void CPPCompile::GenWhenStmt(const WhenStmt* w) { auto wi = w->Info(); - auto wl = wi->Lambda(); - if ( ! wl ) - reporter->FatalError("cannot compile deprecated \"when\" statement"); + vector local_aggrs; + for ( auto& l : wi->WhenExprLocals() ) + if ( IsAggr(l->GetType()) ) + local_aggrs.push_back(IDNameStr(l.get())); + + auto when_lambda = GenExpr(wi->Lambda(), GEN_NATIVE); + GenWhenStmt(wi.get(), when_lambda, w->GetLocationInfo(), std::move(local_aggrs)); +} + +void CPPCompile::GenWhenStmt(const WhenInfo* wi, const std::string& when_lambda, const Location* loc, + vector local_aggrs) { auto is_return = wi->IsReturn() ? "true" : "false"; auto timeout = wi->TimeoutExpr(); auto timeout_val = timeout ? GenExpr(timeout, GEN_NATIVE) : "-1.0"; - auto loc = w->GetLocationInfo(); Emit("{ // begin a new scope for internal variables"); @@ -331,17 +338,18 @@ void CPPCompile::GenWhenStmt(const WhenStmt* w) { NL(); Emit("std::vector CPP__local_aggrs;"); - for ( auto& l : wi->WhenExprLocals() ) - if ( IsAggr(l->GetType()) ) - Emit("CPP__local_aggrs.emplace_back(%s);", IDNameStr(l.get())); + for ( auto& la : local_aggrs ) + Emit("CPP__local_aggrs.emplace_back(%s);", la); - Emit("CPP__wi->Instantiate(%s);", GenExpr(wi->Lambda(), GEN_NATIVE)); + Emit("CPP__wi->Instantiate(%s);", when_lambda); // We need a new frame for the trigger to unambiguously associate // with, in case we're called multiple times with our existing frame. Emit("auto new_frame = make_intrusive(0, nullptr, nullptr);"); Emit("auto curr_t = f__CPP->GetTrigger();"); Emit("auto curr_assoc = f__CPP->GetTriggerAssoc();"); + if ( ! ret_type || ret_type->Tag() == TYPE_VOID ) + Emit("// Note, the following works even if curr_t is nil."); Emit("new_frame->SetTrigger({NewRef{}, curr_t});"); Emit("new_frame->SetTriggerAssoc(curr_assoc);"); From 5e3533428107e16e566f6ab668e7005a7de8387e Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 13 Aug 2024 14:46:08 -0700 Subject: [PATCH 09/10] marked some recently added BTests as not suitable for -O gen-C++ testing --- .../frameworks/cluster/cluster_started_restart_manager.zeek | 1 + .../frameworks/cluster/cluster_started_restart_worker.zeek | 1 + 2 files changed, 2 insertions(+) diff --git a/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_manager.zeek b/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_manager.zeek index 1000fa5cd7..fcc9f7e3e9 100644 --- a/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_manager.zeek +++ b/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_manager.zeek @@ -1,4 +1,5 @@ # @TEST-DOC: Verify cluster_started() is not rebroadcasted if the manager restarts. +# @TEST-REQUIRES: test "${ZEEK_USE_CPP}" != "1" # @TEST-PORT: SUPERVISOR_PORT # @TEST-PORT: MANAGER_PORT # @TEST-PORT: PROXY_PORT diff --git a/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_worker.zeek b/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_worker.zeek index 504f8d3d09..457d34db11 100644 --- a/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_worker.zeek +++ b/testing/btest/scripts/policy/frameworks/cluster/cluster_started_restart_worker.zeek @@ -1,4 +1,5 @@ # @TEST-DOC: Verify cluster_started() is not rebroadcasted if a worker restarts. +# @TEST-REQUIRES: test "${ZEEK_USE_CPP}" != "1" # @TEST-PORT: SUPERVISOR_PORT # @TEST-PORT: MANAGER_PORT # @TEST-PORT: PROXY_PORT From 5e36709905f3e61866b33b9e23bfbd985c89f31e Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 15 Aug 2024 10:08:47 +0200 Subject: [PATCH 10/10] Func: Add SetCapturesVec() Add an API to directly set captures_vec for use by C++ compilation. The current code keys off or asserts on ZAM stmts, making it difficult to leverage captures_vec in other contexts. --- src/Func.h | 15 ++++++++++++--- src/script_opt/CPP/Func.h | 1 - 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Func.h b/src/Func.h index f6808484d5..d22a8cc716 100644 --- a/src/Func.h +++ b/src/Func.h @@ -211,6 +211,15 @@ public: return *captures_vec; } + /** + * Set the set of ZVal's used for captures. + * + * Used for script optimization purposes. + * + * @param cv The value used for captures_vec. + */ + void SetCapturesVec(std::unique_ptr> cv) { captures_vec = std::move(cv); } + // Same definition as in Frame.h. using OffsetMap = std::unordered_map; @@ -291,9 +300,6 @@ protected: */ virtual void SetCaptures(Frame* f); - // Captures when using ZVal block instead of a Frame. - std::unique_ptr> captures_vec; - private: size_t frame_size = 0; @@ -307,6 +313,9 @@ private: OffsetMap* captures_offset_mapping = nullptr; + // Captures when using ZVal block instead of a Frame. + std::unique_ptr> captures_vec; + // The most recently added/updated body ... StmtPtr current_body; diff --git a/src/script_opt/CPP/Func.h b/src/script_opt/CPP/Func.h index 9a12cb2921..4642c2fdaa 100644 --- a/src/script_opt/CPP/Func.h +++ b/src/script_opt/CPP/Func.h @@ -77,7 +77,6 @@ protected: // Methods related to sending lambdas via Broker. std::optional SerializeCaptures() const override; void SetCaptures(Frame* f) override; - void SetCapturesVec(std::unique_ptr> _captures_vec) { captures_vec = std::move(_captures_vec); } FuncPtr DoClone() override;