diff --git a/src/analysis/function_analysis.cpp b/src/analysis/function_analysis.cpp index 7ed5479c1..6e579fd56 100644 --- a/src/analysis/function_analysis.cpp +++ b/src/analysis/function_analysis.cpp @@ -549,22 +549,15 @@ std::unique_ptr computeRequiredPhis(const ParamNames& args, CFG* cf initial_map[vreg] = DefinednessAnalysis::Undefined; } - auto maybe_add = [&](AST_Name* n) { + for (AST_Name* n : args.allArgsAsName()) { ScopeInfo::VarScopeType vst = n->lookup_type; assert(vst != ScopeInfo::VarScopeType::UNKNOWN); assert(vst != ScopeInfo::VarScopeType::GLOBAL); // global-and-local error if (vst == ScopeInfo::VarScopeType::NAME) - return; + continue; assert(n->vreg >= 0); initial_map[n->vreg] = DefinednessAnalysis::Defined; - }; - - for (auto e : args.arg_names) - maybe_add(e); - if (args.vararg_name) - maybe_add(args.vararg_name); - if (args.kwarg_name) - maybe_add(args.kwarg_name); + } assert(initial_map.numVregs() == vreg_info.getTotalNumOfVRegs()); diff --git a/src/analysis/type_analysis.cpp b/src/analysis/type_analysis.cpp index 185847c5f..d390b18fd 100644 --- a/src/analysis/type_analysis.cpp +++ b/src/analysis/type_analysis.cpp @@ -859,30 +859,15 @@ TypeAnalysis* doTypeAnalysis(CFG* cfg, const ParamNames& arg_names, const std::v TypeMap initial_types(cfg->getVRegInfo().getTotalNumOfVRegs()); int i = 0; - - auto maybe_add = [&](AST_Name* n) { + for (AST_Name* n : arg_names.allArgsAsName()) { ScopeInfo::VarScopeType vst = n->lookup_type; assert(vst != ScopeInfo::VarScopeType::UNKNOWN); assert(vst != ScopeInfo::VarScopeType::GLOBAL); // global-and-local error - if (vst == ScopeInfo::VarScopeType::NAME) - return; - initial_types[n->vreg] = unboxedType(arg_types[i]); + if (vst != ScopeInfo::VarScopeType::NAME) + initial_types[n->vreg] = unboxedType(arg_types[i]); + ++i; }; - for (; i < arg_names.args.size(); i++) { - maybe_add(arg_names.arg_names[i]); - } - - if (arg_names.vararg.size()) { - maybe_add(arg_names.vararg_name); - i++; - } - - if (arg_names.kwarg.size()) { - maybe_add(arg_names.kwarg_name); - i++; - } - assert(i == arg_types.size()); return PropagatingTypeAnalysis::doAnalysis(speculation, scope_info, std::move(initial_types), diff --git a/src/asm_writing/icinfo.cpp b/src/asm_writing/icinfo.cpp index d34d2bfac..975364cb5 100644 --- a/src/asm_writing/icinfo.cpp +++ b/src/asm_writing/icinfo.cpp @@ -252,15 +252,19 @@ void ICSlotRewrite::addDependenceOn(ICInvalidator& invalidator) { int ICInfo::calculateSuggestedSize() { // if we never rewrote this IC just return the whole IC size for now if (!times_rewritten) - return slots[0].size; + return slots.begin()->size; int additional_space_per_slot = 50; // if there are less rewrites than slots we can give a very accurate estimate if (times_rewritten < slots.size()) { // add up the sizes of all used slots int size = 0; - for (int i = 0; i < times_rewritten; ++i) { - size += slots[i].size + additional_space_per_slot; + int i = 0; + for (auto&& slot : slots) { + if (i >= times_rewritten) + break; + size += slot.size + additional_space_per_slot; + ++i; } return size; } @@ -286,18 +290,23 @@ ICSlotInfo* ICInfo::pickEntryForRewrite(const char* debug_name) { int num_slots = slots.size(); int fallback_to_in_use_slot = -1; + llvm::SmallVector slots_vec; + for (auto&& slot : slots) { + slots_vec.push_back(&slot); + } + // we prefer to use a unused slot and if non is available we will fallback to a slot which is in use (but no one is // inside) for (int _i = 0; _i < num_slots; _i++) { int i = (_i + next_slot_to_try) % num_slots; - ICSlotInfo& sinfo = slots[i]; - assert(sinfo.num_inside >= 0); + ICSlotInfo* sinfo = slots_vec[i]; + assert(sinfo->num_inside >= 0); - if (sinfo.num_inside || sinfo.size == 0) + if (sinfo->num_inside || sinfo->size == 0) continue; - if (sinfo.used) { + if (sinfo->used) { if (fallback_to_in_use_slot == -1) fallback_to_in_use_slot = i; continue; @@ -308,7 +317,7 @@ ICSlotInfo* ICInfo::pickEntryForRewrite(const char* debug_name) { } next_slot_to_try = i; - return &sinfo; + return sinfo; } if (fallback_to_in_use_slot != -1) { @@ -317,7 +326,7 @@ ICSlotInfo* ICInfo::pickEntryForRewrite(const char* debug_name) { } next_slot_to_try = fallback_to_in_use_slot; - return &slots[fallback_to_in_use_slot]; + return slots_vec[fallback_to_in_use_slot]; } if (VERBOSITY() >= 4) @@ -357,7 +366,6 @@ ICInfo::~ICInfo() { ics_by_return_addr.erase(slowpath_rtn_addr); if (node) ics_by_ast_node.erase(node); - deregisterGCTrackedICInfo(this); for (auto& slot : slots) { // Calling a full clear() might be overkill here, but probably better safe than sorry: @@ -413,8 +421,6 @@ std::unique_ptr registerCompiledPatchpoint(uint8_t* start_addr, uint8_t* assert(!ics_by_return_addr.count(slowpath_rtn_addr)); ics_by_return_addr[slowpath_rtn_addr] = icinfo; - registerGCTrackedICInfo(icinfo); - return std::unique_ptr(icinfo); } @@ -447,11 +453,13 @@ void ICInfo::invalidate(ICSlotInfo* icentry) { llvm::sys::Memory::InvalidateInstructionCache(start, icentry->size); - for (int i = 0; i < slots.size(); ++i) { - if (&slots[i] == icentry) { + int i = 0; + for (auto&& slot : slots) { + if (&slot == icentry) { next_slot_to_try = i; break; } + ++i; } icentry->used = false; diff --git a/src/asm_writing/icinfo.h b/src/asm_writing/icinfo.h index 93d5e6732..832afe798 100644 --- a/src/asm_writing/icinfo.h +++ b/src/asm_writing/icinfo.h @@ -15,12 +15,13 @@ #ifndef PYSTON_ASMWRITING_ICINFO_H #define PYSTON_ASMWRITING_ICINFO_H -#include +#include #include #include #include #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/TinyPtrVector.h" #include "llvm/IR/CallingConv.h" #include "asm_writing/assembler.h" @@ -62,15 +63,16 @@ struct ICSlotInfo { ICInfo* ic; uint8_t* start_addr; + + std::vector gc_references; + std::vector decref_infos; + llvm::TinyPtrVector invalidators; // ICInvalidators that reference this slotinfo + int num_inside; // the number of stack frames that are currently inside this slot will also get increased during a // rewrite int size; bool used; // if this slot is empty or got invalidated - std::vector gc_references; - std::vector decref_infos; - std::vector invalidators; // ICInvalidators that reference this slotinfo - void clear(bool should_invalidate = true); }; @@ -79,7 +81,7 @@ typedef BitSet<16> LiveOutSet; class ICSlotRewrite; class ICInfo { private: - std::deque slots; + std::list slots; // For now, just use a round-robin eviction policy. // This is probably a bunch worse than LRU, but it's also // probably a bunch better than the "always evict slot #0" policy @@ -196,11 +198,6 @@ class ICSlotRewrite { friend class ICInfo; }; -inline void registerGCTrackedICInfo(ICInfo* ic) { -} -inline void deregisterGCTrackedICInfo(ICInfo* ic) { -} - class ICSetupInfo; struct CompiledFunction; std::unique_ptr diff --git a/src/codegen/ast_interpreter.cpp b/src/codegen/ast_interpreter.cpp index 5e189c334..7ac9effc2 100644 --- a/src/codegen/ast_interpreter.cpp +++ b/src/codegen/ast_interpreter.cpp @@ -74,7 +74,7 @@ class ASTInterpreter { static Box* executeInner(ASTInterpreter& interpreter, CFGBlock* start_block, AST_stmt* start_at); private: - Value createFunction(AST* node, AST_arguments* args, const std::vector& body); + Value createFunction(AST* node, AST_arguments* args); Value doBinOp(AST_expr* node, Value left, Value right, int op, BinExpType exp_type); void doStore(AST_expr* node, STOLEN(Value) value); void doStore(AST_Name* name, STOLEN(Value) value); @@ -272,26 +272,21 @@ void ASTInterpreter::initArguments(BoxedClosure* _closure, BoxedGenerator* _gene const ParamNames& param_names = getMD()->param_names; - // make sure the AST_Name nodes are set - assert(param_names.args.size() == param_names.arg_names.size()); - assert(param_names.vararg.empty() == (param_names.vararg_name == NULL)); - assert(param_names.kwarg.empty() == (param_names.kwarg_name == NULL)); - int i = 0; - for (auto& name : param_names.arg_names) { + for (auto& name : param_names.argsAsName()) { doStore(name, Value(incref(getArg(i++, arg1, arg2, arg3, args)), 0)); } - if (param_names.vararg_name) - doStore(param_names.vararg_name, Value(incref(getArg(i++, arg1, arg2, arg3, args)), 0)); + if (param_names.has_vararg_name) + doStore(param_names.varArgAsName(), Value(incref(getArg(i++, arg1, arg2, arg3, args)), 0)); - if (param_names.kwarg_name) { + if (param_names.has_kwarg_name) { Box* val = getArg(i++, arg1, arg2, arg3, args); if (!val) val = createDict(); else Py_INCREF(val); - doStore(param_names.kwarg_name, Value(val, 0)); + doStore(param_names.kwArgAsName(), Value(val, 0)); } assert(i == param_names.totalParameters()); } @@ -314,13 +309,9 @@ void ASTInterpreter::startJITing(CFGBlock* block, int exit_offset, llvm::DenseSe // small optimization: we know that the passed arguments in the entry block are non zero if (block == block->cfg->getStartingBlock() && block->predecessors.empty()) { auto param_names = getMD()->param_names; - for (auto&& arg : param_names.arg_names) { + for (auto&& arg : param_names.allArgsAsName()) { known_non_null_vregs.insert(arg->vreg); } - if (param_names.vararg_name) - known_non_null_vregs.insert(param_names.vararg_name->vreg); - if (param_names.kwarg_name) - known_non_null_vregs.insert(param_names.kwarg_name->vreg); } jit = code_block->newFragment(block, exit_offset, std::move(known_non_null_vregs)); } @@ -1131,8 +1122,8 @@ Value ASTInterpreter::visit_return(AST_Return* node) { return s; } -Value ASTInterpreter::createFunction(AST* node, AST_arguments* args, const std::vector& body) { - FunctionMetadata* md = wrapFunction(node, args, body, source_info); +Value ASTInterpreter::createFunction(AST* node, AST_arguments* args) { + FunctionMetadata* md = wrapFunction(node, args, source_info); std::vector defaults; llvm::SmallVector defaults_vars; @@ -1232,7 +1223,7 @@ Value ASTInterpreter::visit_makeFunction(AST_MakeFunction* mkfn) { for (AST_expr* d : node->decorator_list) decorators.push_back(visit_expr(d)); - Value func = createFunction(node, args, node->body); + Value func = createFunction(node, args); for (int i = decorators.size() - 1; i >= 0; i--) { func.o = runtimeCall(autoDecref(decorators[i].o), ArgPassSpec(1), autoDecref(func.o), 0, 0, 0, 0); @@ -1271,7 +1262,7 @@ Value ASTInterpreter::visit_makeClass(AST_MakeClass* mkclass) { closure = created_closure; assert(closure); } - FunctionMetadata* md = wrapFunction(node, nullptr, node->body, source_info); + FunctionMetadata* md = wrapFunction(node, nullptr, source_info); Box* passed_globals = NULL; if (!getMD()->source->scoping->areGlobalsFromModule()) @@ -1553,7 +1544,7 @@ Value ASTInterpreter::visit_call(AST_Call* node) { AUTO_DECREF(func.o); - std::vector args; + llvm::SmallVector args; llvm::SmallVector args_vars; args.reserve(node->args.size()); args_vars.reserve(node->args.size()); @@ -1586,7 +1577,7 @@ Value ASTInterpreter::visit_call(AST_Call* node) { args_vars.push_back(v); } - AUTO_DECREF_ARRAY(&args[0], args.size()); + AUTO_DECREF_ARRAY(args.data(), args.size()); ArgPassSpec argspec(node->args.size(), node->keywords.size(), node->starargs, node->kwargs); diff --git a/src/codegen/baseline_jit.cpp b/src/codegen/baseline_jit.cpp index 1d0ac228b..7fd9a26f0 100644 --- a/src/codegen/baseline_jit.cpp +++ b/src/codegen/baseline_jit.cpp @@ -882,14 +882,6 @@ std::pair> JitFragmentWriter::finishCompilation() { code_block.fragmentFinished(assembler->bytesWritten(), num_bytes_overlapping, next_fragment_start, std::move(ic_infos), *ic_info); -#if MOVING_GC - // If JitFragmentWriter is destroyed, we don't necessarily want the ICInfo to be destroyed also, - // because it may contain a list of references to pointers in generated code that still exists - // and we need to keep those around. - // TODO: When should these ICInfo be freed? - registerGCTrackedICInfo(ic_info.release()); -#endif - return std::make_pair(exit_info.num_bytes, std::move(known_non_null_vregs)); } diff --git a/src/codegen/baseline_jit.h b/src/codegen/baseline_jit.h index 8abaa88ca..59d34bd28 100644 --- a/src/codegen/baseline_jit.h +++ b/src/codegen/baseline_jit.h @@ -148,7 +148,7 @@ class JitFragmentWriter; class JitCodeBlock { public: static constexpr int scratch_size = 256; - static constexpr int memory_size = 32768; // must fit the EH frame + generated code + static constexpr int memory_size = 6 * 4096; // must fit the EH frame + generated code static constexpr int num_stack_args = 2; // scratch size + space for passing additional args on the stack without having to adjust the SP when calling diff --git a/src/codegen/codegen.cpp b/src/codegen/codegen.cpp index ca350f9ea..07b182066 100644 --- a/src/codegen/codegen.cpp +++ b/src/codegen/codegen.cpp @@ -40,22 +40,22 @@ namespace pyston { FunctionMetadata::FunctionMetadata(int num_args, bool takes_varargs, bool takes_kwargs, std::unique_ptr source) : code_obj(NULL), - num_args(num_args), - takes_varargs(takes_varargs), - takes_kwargs(takes_kwargs), source(std::move(source)), param_names(this->source->ast, this->source->getInternedStrings()), + takes_varargs(takes_varargs), + takes_kwargs(takes_kwargs), + num_args(num_args), times_interpreted(0), internal_callable(NULL, NULL) { } FunctionMetadata::FunctionMetadata(int num_args, bool takes_varargs, bool takes_kwargs, const ParamNames& param_names) : code_obj(NULL), - num_args(num_args), - takes_varargs(takes_varargs), - takes_kwargs(takes_kwargs), source(nullptr), param_names(param_names), + takes_varargs(takes_varargs), + takes_kwargs(takes_kwargs), + num_args(num_args), times_interpreted(0), internal_callable(NULL, NULL) { } @@ -87,12 +87,12 @@ void FunctionMetadata::addVersion(CompiledFunction* compiled) { assert(compiled->spec->arg_types.size() == numReceivedArgs()); versions.push_back(compiled); } else { - osr_versions[compiled->entry_descriptor] = compiled; + osr_versions.emplace_front(compiled->entry_descriptor, compiled); } } SourceInfo::SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, FutureFlags future_flags, AST* ast, BoxedString* fn) - : parent_module(m), scoping(scoping), scope_info(NULL), future_flags(future_flags), ast(ast), cfg(NULL) { + : parent_module(m), scoping(scoping), scope_info(NULL), ast(ast), cfg(NULL), future_flags(future_flags) { assert(fn); // TODO: this is a very bad way of handling this: @@ -272,7 +272,6 @@ llvm::JITEventListener* makeRegistryListener() { return new RegistryEventListener(); } - FunctionSpecialization::FunctionSpecialization(ConcreteCompilerType* rtn_type) : rtn_type(rtn_type) { accepts_all_inputs = true; boxed_return_value = (rtn_type->llvmType() == UNKNOWN->llvmType()); diff --git a/src/codegen/irgen.cpp b/src/codegen/irgen.cpp index e3bae2e9d..444d9092d 100644 --- a/src/codegen/irgen.cpp +++ b/src/codegen/irgen.cpp @@ -1024,7 +1024,7 @@ static std::string getUniqueFunctionName(std::string nameprefix, EffortLevel eff } std::pair doCompile(FunctionMetadata* md, SourceInfo* source, - ParamNames* param_names, + const ParamNames* param_names, const OSREntryDescriptor* entry_descriptor, EffortLevel effort, ExceptionStyle exception_style, FunctionSpecialization* spec, llvm::StringRef nameprefix) { diff --git a/src/codegen/irgen.h b/src/codegen/irgen.h index 61e8aabdd..f04241415 100644 --- a/src/codegen/irgen.h +++ b/src/codegen/irgen.h @@ -138,9 +138,11 @@ extern const std::string PASSED_GENERATOR_NAME; InternedString getIsDefinedName(InternedString name, InternedStringPool& interned_strings); bool isIsDefinedName(llvm::StringRef name); -std::pair -doCompile(FunctionMetadata* md, SourceInfo* source, ParamNames* param_names, const OSREntryDescriptor* entry_descriptor, - EffortLevel effort, ExceptionStyle exception_style, FunctionSpecialization* spec, llvm::StringRef nameprefix); +std::pair doCompile(FunctionMetadata* md, SourceInfo* source, + const ParamNames* param_names, + const OSREntryDescriptor* entry_descriptor, EffortLevel effort, + ExceptionStyle exception_style, FunctionSpecialization* spec, + llvm::StringRef nameprefix); // A common pattern is to branch based off whether a variable is defined but only if it is // potentially-undefined. If it is potentially-undefined, we have to generate control-flow diff --git a/src/codegen/irgen/hooks.cpp b/src/codegen/irgen/hooks.cpp index 275d25ef1..2211203cf 100644 --- a/src/codegen/irgen/hooks.cpp +++ b/src/codegen/irgen/hooks.cpp @@ -56,11 +56,9 @@ namespace pyston { // TODO terrible place for these! ParamNames::ParamNames(AST* ast, InternedStringPool& pool) - : takes_param_names(true), vararg_name(NULL), kwarg_name(NULL) { + : all_args_contains_names(1), takes_param_names(1), has_vararg_name(0), has_kwarg_name(0) { if (ast->type == AST_TYPE::Module || ast->type == AST_TYPE::ClassDef || ast->type == AST_TYPE::Expression || ast->type == AST_TYPE::Suite) { - kwarg = ""; - vararg = ""; } else if (ast->type == AST_TYPE::FunctionDef || ast->type == AST_TYPE::Lambda) { AST_arguments* arguments = ast->type == AST_TYPE::FunctionDef ? ast_cast(ast)->args : ast_cast(ast)->args; @@ -68,32 +66,57 @@ ParamNames::ParamNames(AST* ast, InternedStringPool& pool) AST_expr* arg = arguments->args[i]; if (arg->type == AST_TYPE::Name) { AST_Name* name = ast_cast(arg); - arg_names.push_back(name); - args.push_back(name->id.s()); + all_args.emplace_back(name); } else { InternedString dot_arg_name = pool.get("." + std::to_string(i)); - arg_names.push_back(new AST_Name(dot_arg_name, AST_TYPE::Param, arg->lineno, arg->col_offset)); - args.push_back(dot_arg_name.s()); + all_args.emplace_back(new AST_Name(dot_arg_name, AST_TYPE::Param, arg->lineno, arg->col_offset)); } } - vararg_name = arguments->vararg; - if (vararg_name) - vararg = vararg_name->id.s(); + auto vararg_name = arguments->vararg; + if (vararg_name) { + has_vararg_name = 1; + all_args.emplace_back(vararg_name); + } - kwarg_name = arguments->kwarg; - if (kwarg_name) - kwarg = kwarg_name->id.s(); + auto kwarg_name = arguments->kwarg; + if (kwarg_name) { + has_kwarg_name = 1; + all_args.emplace_back(kwarg_name); + } } else { RELEASE_ASSERT(0, "%d", ast->type); } } -ParamNames::ParamNames(const std::vector& args, llvm::StringRef vararg, llvm::StringRef kwarg) - : takes_param_names(true), vararg_name(NULL), kwarg_name(NULL) { - this->args = args; - this->vararg = vararg; - this->kwarg = kwarg; +ParamNames::ParamNames(const std::vector& args, const char* vararg, const char* kwarg) + : all_args_contains_names(0), + takes_param_names(1), + has_vararg_name(vararg && *vararg), + has_kwarg_name(kwarg && *kwarg) { + all_args.reserve(args.size() + has_vararg_name + has_kwarg_name); + for (auto&& arg : args) { + all_args.emplace_back(arg); + } + if (has_vararg_name) + all_args.emplace_back(vararg); + if (has_kwarg_name) + all_args.emplace_back(kwarg); +} + +std::vector ParamNames::allArgsAsStr() const { + std::vector ret; + ret.reserve(all_args.size()); + if (all_args_contains_names) { + for (auto&& arg : all_args) { + ret.push_back(arg.name->id.c_str()); + } + } else { + for (auto&& arg : all_args) { + ret.push_back(arg.str); + } + } + return ret; } InternedString SourceInfo::mangleName(InternedString id) { @@ -233,7 +256,7 @@ CompiledFunction* compileFunction(FunctionMetadata* f, FunctionSpecialization* s BoxedString* name = source->getName(); - ASSERT(f->versions.size() < 20, "%s %ld", name->c_str(), f->versions.size()); + ASSERT(f->versions.size() < 20, "%s %u", name->c_str(), f->versions.size()); ExceptionStyle exception_style; if (force_exception_style) @@ -669,14 +692,14 @@ void CompiledFunction::speculationFailed() { } if (!found) { - for (auto it = md->osr_versions.begin(); it != md->osr_versions.end(); ++it) { - if (it->second == this) { - md->osr_versions.erase(it); + md->osr_versions.remove_if([&](const std::pair& e) { + if (e.second == this) { this->dependent_callsites.invalidateAll(); found = true; - break; + return true; } - } + return false; + }); } if (!found) { @@ -701,20 +724,8 @@ CompiledFunction::CompiledFunction(FunctionMetadata* md, FunctionSpecialization* times_speculation_failed(0), location_map(nullptr) { assert((spec != NULL) + (entry_descriptor != NULL) == 1); - -#if MOVING_GC - assert(all_compiled_functions.count(this) == 0); - all_compiled_functions.insert(this); -#endif } -#if MOVING_GC -CompiledFunction::~CompiledFunction() { - assert(all_compiled_functions.count(this) == 1); - all_compiled_functions.erase(this); -} -#endif - ConcreteCompilerType* CompiledFunction::getReturnType() { assert(((bool)spec) ^ ((bool)entry_descriptor)); if (spec) @@ -752,7 +763,7 @@ static CompiledFunction* _doReopt(CompiledFunction* cf, EffortLevel new_effort) } } - printf("Couldn't find a version; %ld exist:\n", versions.size()); + printf("Couldn't find a version; %u exist:\n", versions.size()); for (auto cf : versions) { printf("%p\n", cf); } @@ -770,17 +781,17 @@ CompiledFunction* compilePartialFuncInternal(OSRExit* exit) { FunctionMetadata* md = exit->entry->md; assert(md); - CompiledFunction*& new_cf = md->osr_versions[exit->entry]; - if (new_cf == NULL) { - EffortLevel new_effort = EffortLevel::MAXIMAL; - CompiledFunction* compiled - = compileFunction(md, NULL, new_effort, exit->entry, true, exit->entry->exception_style); - assert(compiled == new_cf); - - stat_osr_compiles.log(); + for (auto&& osr_functions : md->osr_versions) { + if (osr_functions.first == exit->entry) + return osr_functions.second; } - return new_cf; + EffortLevel new_effort = EffortLevel::MAXIMAL; + CompiledFunction* compiled = compileFunction(md, NULL, new_effort, exit->entry, true, exit->entry->exception_style); + stat_osr_compiles.log(); + assert(std::find(md->osr_versions.begin(), md->osr_versions.end(), std::make_pair(exit->entry, compiled)) + != md->osr_versions.end()); + return compiled; } void* compilePartialFunc(OSRExit* exit) { diff --git a/src/codegen/irgen/irgenerator.cpp b/src/codegen/irgen/irgenerator.cpp index 6e12cc688..8cd7e24be 100644 --- a/src/codegen/irgen/irgenerator.cpp +++ b/src/codegen/irgen/irgenerator.cpp @@ -45,7 +45,7 @@ extern "C" void dumpLLVM(void* _v) { } IRGenState::IRGenState(FunctionMetadata* md, CompiledFunction* cf, llvm::Function* func, SourceInfo* source_info, - std::unique_ptr phis, ParamNames* param_names, GCBuilder* gc, + std::unique_ptr phis, const ParamNames* param_names, GCBuilder* gc, llvm::MDNode* func_dbg_info, RefcountTracker* refcount_tracker) : md(md), cf(cf), @@ -1617,7 +1617,7 @@ class IRGeneratorImpl : public IRGenerator { decorators.push_back(evalExpr(d, unw_info)); } - FunctionMetadata* md = wrapFunction(node, nullptr, node->body, irstate->getSourceInfo()); + FunctionMetadata* md = wrapFunction(node, nullptr, irstate->getSourceInfo()); // TODO duplication with _createFunction: llvm::Value* this_closure = NULL; @@ -1658,9 +1658,8 @@ class IRGeneratorImpl : public IRGenerator { return cls; } - CompilerVariable* _createFunction(AST* node, const UnwindInfo& unw_info, AST_arguments* args, - const std::vector& body) { - FunctionMetadata* md = wrapFunction(node, args, body, irstate->getSourceInfo()); + CompilerVariable* _createFunction(AST* node, const UnwindInfo& unw_info, AST_arguments* args) { + FunctionMetadata* md = wrapFunction(node, args, irstate->getSourceInfo()); std::vector defaults; for (auto d : args->defaults) { @@ -1706,7 +1705,7 @@ class IRGeneratorImpl : public IRGenerator { decorators.push_back(evalExpr(d, unw_info)); } - CompilerVariable* func = _createFunction(node, unw_info, node->args, node->body); + CompilerVariable* func = _createFunction(node, unw_info, node->args); for (int i = decorators.size() - 1; i >= 0; i--) { func = decorators[i]->call(emitter, getOpInfoForNode(node, unw_info), ArgPassSpec(1), { func }, NULL); @@ -2983,16 +2982,17 @@ class IRGeneratorImpl : public IRGenerator { assert(python_parameters.size() == param_names.totalParameters()); int i = 0; - for (; i < param_names.args.size(); i++) { - loadArgument(param_names.arg_names[i], arg_types[i], python_parameters[i], UnwindInfo::cantUnwind()); + for (auto&& arg : param_names.argsAsName()) { + loadArgument(arg, arg_types[i], python_parameters[i], UnwindInfo::cantUnwind()); + ++i; } - if (param_names.vararg.size()) { - loadArgument(param_names.vararg_name, arg_types[i], python_parameters[i], UnwindInfo::cantUnwind()); + if (param_names.has_vararg_name) { + loadArgument(param_names.varArgAsName(), arg_types[i], python_parameters[i], UnwindInfo::cantUnwind()); i++; } - if (param_names.kwarg.size()) { + if (param_names.has_kwarg_name) { llvm::Value* passed_dict = python_parameters[i]; emitter.setNullable(passed_dict, true); @@ -3018,7 +3018,7 @@ class IRGeneratorImpl : public IRGenerator { emitter.refConsumed(passed_dict, null_check); emitter.refConsumed(created_dict, isnull_terminator); - loadArgument(param_names.kwarg_name, arg_types[i], phi, UnwindInfo::cantUnwind()); + loadArgument(param_names.kwArgAsName(), arg_types[i], phi, UnwindInfo::cantUnwind()); i++; } @@ -3248,10 +3248,10 @@ IRGenerator* createIRGenerator(IRGenState* irstate, std::unordered_map& body, SourceInfo* source) { +FunctionMetadata* wrapFunction(AST* node, AST_arguments* args, SourceInfo* source) { // Different compilations of the parent scope of a functiondef should lead // to the same FunctionMetadata* being used: - static std::unordered_map made; + static llvm::DenseMap made; FunctionMetadata*& md = made[node]; if (md == NULL) { diff --git a/src/codegen/irgen/irgenerator.h b/src/codegen/irgen/irgenerator.h index 0a85bbaed..46c1784c1 100644 --- a/src/codegen/irgen/irgenerator.h +++ b/src/codegen/irgen/irgenerator.h @@ -66,7 +66,7 @@ class IRGenState { llvm::Function* func; SourceInfo* source_info; std::unique_ptr phis; - ParamNames* param_names; + const ParamNames* param_names; GCBuilder* gc; llvm::MDNode* func_dbg_info; RefcountTracker* refcount_tracker; @@ -84,8 +84,8 @@ class IRGenState { public: IRGenState(FunctionMetadata* md, CompiledFunction* cf, llvm::Function* func, SourceInfo* source_info, - std::unique_ptr phis, ParamNames* param_names, GCBuilder* gc, llvm::MDNode* func_dbg_info, - RefcountTracker* refcount_tracker); + std::unique_ptr phis, const ParamNames* param_names, GCBuilder* gc, + llvm::MDNode* func_dbg_info, RefcountTracker* refcount_tracker); ~IRGenState(); CFG* getCFG() { return getSourceInfo()->cfg; } @@ -125,7 +125,7 @@ class IRGenState { RefcountTracker* getRefcounts() { return refcount_tracker; } - ParamNames* getParamNames() { return param_names; } + const ParamNames* getParamNames() { return param_names; } llvm::Value* getPassedClosure(); llvm::Value* getCreatedClosure(); @@ -205,7 +205,7 @@ IREmitter* createIREmitter(IRGenState* irstate, llvm::BasicBlock*& curblock, IRG IRGenerator* createIRGenerator(IRGenState* irstate, std::unordered_map& entry_blocks, CFGBlock* myblock, TypeAnalysis* types); -FunctionMetadata* wrapFunction(AST* node, AST_arguments* args, const std::vector& body, SourceInfo* source); +FunctionMetadata* wrapFunction(AST* node, AST_arguments* args, SourceInfo* source); std::vector* getKeywordNameStorage(AST_Call* node); } diff --git a/src/core/cfg.cpp b/src/core/cfg.cpp index e177717d7..4ea9d0524 100644 --- a/src/core/cfg.cpp +++ b/src/core/cfg.cpp @@ -2829,14 +2829,9 @@ void VRegInfo::assignVRegs(CFG* cfg, const ParamNames& param_names, ScopeInfo* s #endif if (b == cfg->getStartingBlock()) { - for (auto* name : param_names.arg_names) { + for (auto* name : param_names.allArgsAsName()) { name->accept(&visitor); } - if (param_names.vararg_name) - param_names.vararg_name->accept(&visitor); - - if (param_names.kwarg_name) - param_names.kwarg_name->accept(&visitor); } for (AST_stmt* stmt : b->body) { diff --git a/src/core/types.h b/src/core/types.h index cf381856f..954e2c709 100644 --- a/src/core/types.h +++ b/src/core/types.h @@ -19,6 +19,7 @@ // over having them spread randomly in different files, this should probably be split again // but in a way that makes more sense. +#include #include #include #include @@ -26,6 +27,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/iterator_range.h" #include "llvm/ADT/SmallPtrSet.h" +#include "llvm/ADT/TinyPtrVector.h" #include "Python.h" #include "core/common.h" @@ -217,31 +219,61 @@ static_assert(sizeof(ArgPassSpec) <= sizeof(void*), "ArgPassSpec doesn't fit in static_assert(sizeof(ArgPassSpec) == sizeof(uint32_t), "ArgPassSpec::asInt needs to be updated"); struct ParamNames { - bool takes_param_names; - std::vector args; - llvm::StringRef vararg, kwarg; + // the arguments are either an array of char* or AST_Name* depending on if all_args_contains_names is set or not + union NameOrStr { + NameOrStr(const char* str) : str(str) {} + NameOrStr(AST_Name* name) : name(name) {} - // This members are only set if the InternedStringPool& constructor is used (aka. source is available)! - // They are used as an optimization while interpreting because the AST_Names nodes cache important stuff - // (InternedString, lookup_type) which would otherwise have to get recomputed all the time. - std::vector arg_names; - AST_Name* vararg_name, *kwarg_name; + const char* str; + AST_Name* name; + }; + std::vector all_args; + + const unsigned char all_args_contains_names : 1; + const unsigned char takes_param_names : 1; + unsigned char has_vararg_name : 1; + unsigned char has_kwarg_name : 1; explicit ParamNames(AST* ast, InternedStringPool& pool); - ParamNames(const std::vector& args, llvm::StringRef vararg, llvm::StringRef kwarg); + ParamNames(const std::vector& args, const char* vararg, const char* kwarg); static ParamNames empty() { return ParamNames(); } - int totalParameters() const { - return args.size() + (vararg.str().size() == 0 ? 0 : 1) + (kwarg.str().size() == 0 ? 0 : 1); - } + int numNormalArgs() const { return all_args.size() - has_vararg_name - has_kwarg_name; } + int totalParameters() const { return all_args.size(); } int kwargsIndex() const { - assert(kwarg.str().size()); - return args.size() + (vararg.str().size() == 0 ? 0 : 1); + assert(has_kwarg_name); + return all_args.size() - 1; + } + + llvm::ArrayRef argsAsName() const { + assert(all_args_contains_names); + return llvm::makeArrayRef((AST_Name * const*)all_args.data(), numNormalArgs()); + } + + llvm::ArrayRef allArgsAsName() const { + assert(all_args_contains_names); + return llvm::makeArrayRef((AST_Name * const*)all_args.data(), all_args.size()); + } + + std::vector allArgsAsStr() const; + + AST_Name* varArgAsName() const { + assert(all_args_contains_names); + if (has_vararg_name) + return all_args[all_args.size() - 1 - has_kwarg_name].name; + return NULL; + } + + AST_Name* kwArgAsName() const { + assert(all_args_contains_names); + if (has_kwarg_name) + return all_args.back().name; + return NULL; } private: - ParamNames() : takes_param_names(false), vararg_name(NULL), kwarg_name(NULL) {} + ParamNames() : all_args_contains_names(0), takes_param_names(0), has_vararg_name(0), has_kwarg_name(0) {} }; // Similar to ArgPassSpec, this struct is how functions specify what their parameter signature is. @@ -406,14 +438,15 @@ class LivenessAnalysis; class SourceInfo { private: BoxedString* fn; // equivalent of code.co_filename + std::unique_ptr liveness_info; public: BoxedModule* parent_module; ScopingAnalysis* scoping; ScopeInfo* scope_info; - FutureFlags future_flags; AST* ast; CFG* cfg; + FutureFlags future_flags; bool is_generator; InternedStringPool& getInternedStrings(); @@ -433,12 +466,9 @@ class SourceInfo { SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, FutureFlags future_flags, AST* ast, BoxedString* fn); ~SourceInfo(); - -private: - std::unique_ptr liveness_info; }; -typedef std::vector FunctionList; +typedef llvm::TinyPtrVector FunctionList; struct CallRewriteArgs; // A BoxedCode is our implementation of the Python "code" object (such as function.func_code). @@ -460,17 +490,17 @@ class FunctionMetadata { BoxedCode* code_obj; public: - int num_args; - bool takes_varargs, takes_kwargs; - std::unique_ptr source; // source can be NULL for functions defined in the C/C++ runtime - ParamNames param_names; + + const ParamNames param_names; + const bool takes_varargs, takes_kwargs; + const int num_args; FunctionList versions; // any compiled versions along with their type parameters; in order from most preferred to least ExceptionSwitchable always_use_version; // if this version is set, always use it (for unboxed cases) - std::unordered_map osr_versions; + std::forward_list> osr_versions; // Profiling counter: int propagated_cxx_exceptions = 0; @@ -517,9 +547,9 @@ class FunctionMetadata { static FunctionMetadata* create(void* f, ConcreteCompilerType* rtn_type, int nargs, bool takes_varargs, bool takes_kwargs, const ParamNames& param_names = ParamNames::empty(), ExceptionStyle exception_style = CXX) { - assert(!param_names.takes_param_names || nargs == param_names.args.size()); - assert(takes_varargs || param_names.vararg.str() == ""); - assert(takes_kwargs || param_names.kwarg.str() == ""); + assert(!param_names.takes_param_names || nargs == param_names.numNormalArgs()); + assert(takes_varargs || !param_names.has_vararg_name); + assert(takes_kwargs || !param_names.has_kwarg_name); FunctionMetadata* fmd = new FunctionMetadata(nargs, takes_varargs, takes_kwargs, param_names); fmd->addVersion(f, rtn_type, exception_style); diff --git a/src/runtime/code.cpp b/src/runtime/code.cpp index ea68c9b6d..940e94600 100644 --- a/src/runtime/code.cpp +++ b/src/runtime/code.cpp @@ -78,12 +78,8 @@ Box* BoxedCode::varnames(Box* b, void*) noexcept { return incref(EmptyTuple); std::vector elts; - for (auto sr : param_names.args) + for (auto sr : param_names.allArgsAsStr()) elts.push_back(boxString(sr)); - if (param_names.vararg.size()) - elts.push_back(boxString(param_names.vararg)); - if (param_names.kwarg.size()) - elts.push_back(boxString(param_names.kwarg)); auto rtn = BoxedTuple::create(elts.size(), &elts[0]); for (auto e : elts) Py_DECREF(e); @@ -95,9 +91,9 @@ Box* BoxedCode::flags(Box* b, void*) noexcept { BoxedCode* code = static_cast(b); int flags = 0; - if (code->f->param_names.vararg.size()) + if (code->f->param_names.has_vararg_name) flags |= CO_VARARGS; - if (code->f->param_names.kwarg.size()) + if (code->f->param_names.has_kwarg_name) flags |= CO_VARKEYWORDS; if (code->f->isGenerator()) flags |= CO_GENERATOR; diff --git a/src/runtime/objmodel.cpp b/src/runtime/objmodel.cpp index 6db142097..148ededd2 100644 --- a/src/runtime/objmodel.cpp +++ b/src/runtime/objmodel.cpp @@ -4066,8 +4066,13 @@ static int placeKeyword(const ParamNames* param_names, llvm::SmallVectorargs.size(); j++) { - if (param_names->args[j] == kw_name->s() && kw_name->size() > 0) { + for (int j = 0; j < param_names->numNormalArgs(); j++) { + llvm::StringRef s; + if (param_names->all_args_contains_names) + s = param_names->all_args[j].name->id.s(); + else + s = param_names->all_args[j].str; + if (s == kw_name->s() && kw_name->size() > 0) { if (params_filled[j]) { raiseExcHelper(TypeError, "%.200s() got multiple values for keyword argument '%s'", func_name_cb(), kw_name->c_str()); diff --git a/src/runtime/util.cpp b/src/runtime/util.cpp index 809b77cec..5bd361a3b 100644 --- a/src/runtime/util.cpp +++ b/src/runtime/util.cpp @@ -260,7 +260,7 @@ extern "C" void dumpEx(void* p, int levels) { printf("A builtin function\n"); } - printf("Has %ld function versions\n", md->versions.size()); + printf("Has %u function versions\n", md->versions.size()); for (CompiledFunction* cf : md->versions) { bool got_name; if (cf->exception_style == CXX)