diff --git a/src/runtime/objmodel.cpp b/src/runtime/objmodel.cpp index 8c4396a8b..12687da96 100644 --- a/src/runtime/objmodel.cpp +++ b/src/runtime/objmodel.cpp @@ -3674,31 +3674,6 @@ Box* callattrInternal(Box* obj, BoxedString* attr, LookupScope scope, CallattrRe int npassed_args = argspec.totalPassed(); - if (rewrite_args && !rewrite_args->args_guarded) { - // TODO duplication with runtime_call - // TODO should know which args don't need to be guarded, ex if we're guaranteed that they - // already fit, either since the type inferencer could determine that, - // or because they only need to fit into an UNKNOWN slot. - - if (npassed_args >= 1) - rewrite_args->arg1->addAttrGuard(offsetof(Box, cls), (intptr_t)arg1->cls); - if (npassed_args >= 2) - rewrite_args->arg2->addAttrGuard(offsetof(Box, cls), (intptr_t)arg2->cls); - if (npassed_args >= 3) - rewrite_args->arg3->addAttrGuard(offsetof(Box, cls), (intptr_t)arg3->cls); - - if (npassed_args > 3) { - for (int i = 3; i < npassed_args; i++) { - // TODO if there are a lot of args (>16), might be better to increment a pointer - // rather index them directly? - RewriterVar* v = rewrite_args->args->getAttr((i - 3) * sizeof(Box*), Location::any()); - v->addAttrGuard(offsetof(Box, cls), (intptr_t)args[i - 3]->cls); - } - } - - rewrite_args->args_guarded = true; - } - // right now I don't think this is ever called with INST_ONLY? assert(scope != INST_ONLY); @@ -5083,6 +5058,36 @@ const char* PyEval_GetFuncDesc(PyObject* func) noexcept { } } +static void addArgGuards(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Box* arg1, Box* arg2, Box* arg3, + Box** args) { + // TODO should know which args don't need to be guarded, ex if we're guaranteed that they + // already fit, either since the type inferencer could determine that, + // or because they only need to fit into an UNKNOWN slot. + + int kwargs_index = -1; + if (argspec.has_kwargs) + kwargs_index = argspec.kwargsIndex(); + + for (int i = 0; i < argspec.totalPassed(); i++) { + Box* v = getArg(i, arg1, arg2, arg3, args); + + if (i == kwargs_index) { + if (v == NULL) { + // I don't think this case should ever get hit currently -- the only places + // we offer rewriting are places that don't have the ability to pass a NULL + // kwargs. + getArg(i, rewrite_args)->addGuard(0); + } else { + getArg(i, rewrite_args)->addAttrGuard(offsetof(Box, cls), (intptr_t)v->cls); + } + } else { + assert(v); + getArg(i, rewrite_args)->addAttrGuard(offsetof(Box, cls), (intptr_t)v->cls); + } + } + rewrite_args->args_guarded = true; +} + template Box* runtimeCallInternal(Box* obj, CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Box* arg1, Box* arg2, Box* arg3, @@ -5173,65 +5178,46 @@ Box* runtimeCallInternal(Box* obj, CallRewriteArgs* rewrite_args, ArgPassSpec ar return rtn; } - if (rewrite_args) { - if (!rewrite_args->args_guarded) { - // TODO should know which args don't need to be guarded, ex if we're guaranteed that they - // already fit, either since the type inferencer could determine that, - // or because they only need to fit into an UNKNOWN slot. - - int kwargs_index = -1; - if (argspec.has_kwargs) - kwargs_index = argspec.kwargsIndex(); - - for (int i = 0; i < npassed_args; i++) { - Box* v = getArg(i, arg1, arg2, arg3, args); - - if (i == kwargs_index) { - if (v == NULL) { - // I don't think this case should ever get hit currently -- the only places - // we offer rewriting are places that don't have the ability to pass a NULL - // kwargs. - getArg(i, rewrite_args)->addGuard(0); - } else { - getArg(i, rewrite_args)->addAttrGuard(offsetof(Box, cls), (intptr_t)v->cls); - } - } else { - assert(v); - getArg(i, rewrite_args)->addAttrGuard(offsetof(Box, cls), (intptr_t)v->cls); - } - } - rewrite_args->args_guarded = true; - } - } if (obj->cls == function_cls || obj->cls == builtin_function_or_method_cls) { BoxedFunctionBase* f = static_cast(obj); - - if (rewrite_args && !rewrite_args->func_guarded) { - rewrite_args->obj->addGuard((intptr_t)f); - rewrite_args->func_guarded = true; - rewrite_args->rewriter->addDependenceOn(f->dependent_ics); - } + auto md = f->md; // Some functions are sufficiently important that we want them to be able to patchpoint themselves; // they can do this by setting the "internal_callable" field: - auto callable = f->md->internal_callable.get(); - + auto callable = md->internal_callable.get(); if (S == CAPI) - assert((bool(f->md->internal_callable.get(CXX)) == bool(callable)) + assert((bool(md->internal_callable.get(CXX)) == bool(callable)) && "too many opportunities for mistakes unless both CXX and CAPI versions are implemented"); else - assert((bool(f->md->internal_callable.get(CAPI)) == bool(callable)) + assert((bool(md->internal_callable.get(CAPI)) == bool(callable)) && "too many opportunities for mistake unless both CXX and CAPI versions are implementeds"); if (callable == NULL) { callable = callFunc; } + if (rewrite_args && !rewrite_args->args_guarded) { + bool does_not_need_guards = callable == &callFunc && md->always_use_version; + if (!does_not_need_guards) + addArgGuards(rewrite_args, argspec, arg1, arg2, arg3, args); + } + + if (rewrite_args && !rewrite_args->func_guarded) { + rewrite_args->obj->addGuard((intptr_t)f); + rewrite_args->func_guarded = true; + // callFunc will add the invalidator + if (callable != &callFunc) + rewrite_args->rewriter->addDependenceOn(f->dependent_ics); + } + KEEP_ALIVE(f); Box* res = callable(f, rewrite_args, argspec, arg1, arg2, arg3, args, keyword_names); return res; } else if (obj->cls == instancemethod_cls) { + if (rewrite_args && !rewrite_args->args_guarded) + addArgGuards(rewrite_args, argspec, arg1, arg2, arg3, args); + BoxedInstanceMethod* im = static_cast(obj); RewriterVar* r_im_func = NULL; diff --git a/test/tests/func_ics.py b/test/tests/func_ics.py new file mode 100644 index 000000000..7cf4c4d66 --- /dev/null +++ b/test/tests/func_ics.py @@ -0,0 +1,10 @@ +# statcheck: stats['ic_attempts'] <= 2000 +# statcheck: stats['ic_attempts_skipped_megamorphic'] == 0 +ins = (1, 1.0, 1l, "", u"") +def f(): + for x in ins: + isinstance(x, int) + [].append(x) + +for i in xrange(30000): + f()