Skip to content

Commit e11625d

Browse files
overcatmiss-islington
authored andcommitted
gh-157833: Fix specialized C calls with additional method flags (GH-157834)
(cherry picked from commit 069c74a) Co-authored-by: Jun Luo <2368403+overcat@users.noreply.github.com>
1 parent 2337dd6 commit e11625d

15 files changed

Lines changed: 383 additions & 60 deletions

File tree

‎Include/internal/pycore_call.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ extern "C" {
1212
#include "pycore_pystate.h" // _PyThreadState_GET()
1313
#include "pycore_stats.h"
1414

15+
/* Flags that determine the C calling convention. */
16+
#define _Py_METH_CALL_FLAGS \
17+
(METH_VARARGS | METH_FASTCALL | METH_NOARGS | METH_O | \
18+
METH_KEYWORDS | METH_METHOD)
19+
1520
/* Suggested size (number of positional arguments) for arrays of PyObject*
1621
allocated on a C stack to avoid allocating memory on the heap memory. Such
1722
array is used to pass positional arguments to call functions of the

‎Lib/test/test_capi/test_opt.py‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3315,6 +3315,186 @@ def testfunc(n):
33153315
self.assertIn("_CALL_BUILTIN_FAST_WITH_KEYWORDS", uops)
33163316
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_FAST_WITH_KEYWORDS", uops)
33173317

3318+
def test_call_builtin_o_extra_flags(self):
3319+
# Extra method flags must not prevent callable guard elimination.
3320+
_testcapi = import_helper.import_module("_testcapi")
3321+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3322+
3323+
namespace = {
3324+
"METH_CLASS_O": _testcapi.MethClass.meth_o,
3325+
"METH_STATIC_O": _testcapi.MethStatic.meth_o,
3326+
"METH_COEXIST_O": {}.__contains__,
3327+
}
3328+
3329+
@reset_code
3330+
def testfunc(n):
3331+
for _ in range(n):
3332+
class_result = METH_CLASS_O(1)
3333+
static_result = METH_STATIC_O(1)
3334+
coexist_result = METH_COEXIST_O(1)
3335+
return class_result, static_result, coexist_result
3336+
3337+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3338+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3339+
self.assertEqual(res, ((_testcapi.MethClass, 1), (None, 1), False))
3340+
self.assertIsNotNone(ex)
3341+
uops = get_opnames(ex)
3342+
self.assertEqual(uops.count("_CALL_BUILTIN_O"), 3)
3343+
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_O", uops)
3344+
3345+
def test_call_builtin_fast_extra_flags(self):
3346+
_testcapi = import_helper.import_module("_testcapi")
3347+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3348+
3349+
obj = _testcapi.MethInstance()
3350+
namespace = {
3351+
"METH_CLASS_FASTCALL": _testcapi.MethClass.meth_fastcall,
3352+
"METH_STATIC_FASTCALL": _testcapi.MethStatic.meth_fastcall,
3353+
"METH_COEXIST_FASTCALL": obj.meth_fastcall_coexist,
3354+
}
3355+
3356+
@reset_code
3357+
def testfunc(n):
3358+
for _ in range(n):
3359+
class_result = METH_CLASS_FASTCALL(1, 2)
3360+
static_result = METH_STATIC_FASTCALL(1, 2)
3361+
coexist_result = METH_COEXIST_FASTCALL(1, 2)
3362+
return class_result, static_result, coexist_result
3363+
3364+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3365+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3366+
self.assertEqual(res, (
3367+
(_testcapi.MethClass, (1, 2)),
3368+
(None, (1, 2)),
3369+
(obj, (1, 2)),
3370+
))
3371+
self.assertIsNotNone(ex)
3372+
uops = get_opnames(ex)
3373+
self.assertEqual(uops.count("_CALL_BUILTIN_FAST"), 3)
3374+
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_FAST", uops)
3375+
3376+
def test_call_builtin_fast_with_keywords_extra_flags(self):
3377+
_testcapi = import_helper.import_module("_testcapi")
3378+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3379+
3380+
obj = _testcapi.MethInstance()
3381+
namespace = {
3382+
"METH_CLASS_FASTCALL_KEYWORDS": (
3383+
_testcapi.MethClass.meth_fastcall_keywords),
3384+
"METH_STATIC_FASTCALL_KEYWORDS": (
3385+
_testcapi.MethStatic.meth_fastcall_keywords),
3386+
"METH_COEXIST_FASTCALL_KEYWORDS": (
3387+
obj.meth_fastcall_keywords_coexist),
3388+
}
3389+
3390+
@reset_code
3391+
def testfunc(n):
3392+
# Use positional arguments to exercise CALL, not CALL_KW.
3393+
for _ in range(n):
3394+
class_result = METH_CLASS_FASTCALL_KEYWORDS(1, 2)
3395+
static_result = METH_STATIC_FASTCALL_KEYWORDS(1, 2)
3396+
coexist_result = METH_COEXIST_FASTCALL_KEYWORDS(1, 2)
3397+
return class_result, static_result, coexist_result
3398+
3399+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3400+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3401+
self.assertEqual(res, (
3402+
(_testcapi.MethClass, (1, 2), {}),
3403+
(None, (1, 2), {}),
3404+
(obj, (1, 2), {}),
3405+
))
3406+
self.assertIsNotNone(ex)
3407+
uops = get_opnames(ex)
3408+
self.assertEqual(uops.count("_CALL_BUILTIN_FAST_WITH_KEYWORDS"), 3)
3409+
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_FAST_WITH_KEYWORDS", uops)
3410+
3411+
def test_call_method_descriptor_o_extra_flags(self):
3412+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3413+
3414+
@reset_code
3415+
def testfunc(n):
3416+
d = {1: None}
3417+
for _ in range(n):
3418+
result = d.__contains__(1)
3419+
return result
3420+
3421+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3422+
self.assertIs(res, True)
3423+
self.assertIsNotNone(ex)
3424+
uops = get_opnames(ex)
3425+
self.assertEqual(uops.count("_CALL_METHOD_DESCRIPTOR_O_INLINE"), 1)
3426+
self.assertNotIn("_GUARD_CALLABLE_METHOD_DESCRIPTOR_O", uops)
3427+
3428+
def test_call_method_descriptor_noargs_extra_flags(self):
3429+
_testcapi = import_helper.import_module("_testcapi")
3430+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3431+
3432+
namespace = {
3433+
"METH_COEXIST_NOARGS_OBJECT": _testcapi.DocStringNoSignatureTest(),
3434+
}
3435+
3436+
@reset_code
3437+
def testfunc(n):
3438+
for _ in range(n):
3439+
result = METH_COEXIST_NOARGS_OBJECT.meth_noargs_coexist()
3440+
return result
3441+
3442+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3443+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3444+
self.assertIsNone(res)
3445+
self.assertIsNotNone(ex)
3446+
uops = get_opnames(ex)
3447+
self.assertEqual(
3448+
uops.count("_CALL_METHOD_DESCRIPTOR_NOARGS_INLINE"), 1)
3449+
self.assertNotIn("_GUARD_CALLABLE_METHOD_DESCRIPTOR_NOARGS", uops)
3450+
3451+
def test_call_method_descriptor_fast_extra_flags(self):
3452+
_testcapi = import_helper.import_module("_testcapi")
3453+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3454+
3455+
obj = _testcapi.MethInstance()
3456+
namespace = {"METH_COEXIST_FAST_OBJECT": obj}
3457+
3458+
@reset_code
3459+
def testfunc(n):
3460+
for _ in range(n):
3461+
result = METH_COEXIST_FAST_OBJECT.meth_fastcall_coexist(1, 2)
3462+
return result
3463+
3464+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3465+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3466+
self.assertEqual(res, (obj, (1, 2)))
3467+
self.assertIsNotNone(ex)
3468+
uops = get_opnames(ex)
3469+
self.assertEqual(uops.count("_CALL_METHOD_DESCRIPTOR_FAST_INLINE"), 1)
3470+
self.assertNotIn("_GUARD_CALLABLE_METHOD_DESCRIPTOR_FAST", uops)
3471+
3472+
def test_call_method_descriptor_fast_with_keywords_extra_flags(self):
3473+
_testcapi = import_helper.import_module("_testcapi")
3474+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3475+
3476+
obj = _testcapi.MethInstance()
3477+
namespace = {"METH_COEXIST_FAST_OBJECT": obj}
3478+
3479+
@reset_code
3480+
def testfunc(n):
3481+
# Use positional arguments to exercise CALL, not CALL_KW.
3482+
for _ in range(n):
3483+
result = (
3484+
METH_COEXIST_FAST_OBJECT.meth_fastcall_keywords_coexist(
3485+
1, 2))
3486+
return result
3487+
3488+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3489+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3490+
self.assertEqual(res, (obj, (1, 2), {}))
3491+
self.assertIsNotNone(ex)
3492+
uops = get_opnames(ex)
3493+
self.assertEqual(
3494+
uops.count("_CALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS_INLINE"), 1)
3495+
self.assertNotIn(
3496+
"_GUARD_CALLABLE_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS", uops)
3497+
33183498
def test_call_method_descriptor_o(self):
33193499
def testfunc(n):
33203500
x = 0

‎Lib/test/test_opcache.py‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2171,6 +2171,89 @@ class MyList(list): pass
21712171
self.assert_no_opcode(my_list_append, "CALL_LIST_APPEND")
21722172
self.assert_no_opcode(my_list_append, "CALL")
21732173

2174+
@cpython_only
2175+
@requires_specialization
2176+
def test_call_c_function_extra_flags(self):
2177+
# METH_CLASS, METH_STATIC and METH_COEXIST do not change the C
2178+
# calling convention, so the specialized instructions must not
2179+
# miss because of them.
2180+
_testcapi = import_module("_testcapi")
2181+
2182+
def call_1(func, arg):
2183+
return func(arg)
2184+
2185+
def call_2(func, arg1, arg2):
2186+
return func(arg1, arg2)
2187+
2188+
def call_3(func, arg1, arg2, arg3):
2189+
return func(arg1, arg2, arg3)
2190+
2191+
def call_method(obj, arg):
2192+
return obj.__contains__(arg)
2193+
2194+
def call_method_noargs(obj):
2195+
return obj.meth_noargs_coexist()
2196+
2197+
def call_method_fast(obj, arg1, arg2):
2198+
return obj.meth_fastcall_coexist(arg1, arg2)
2199+
2200+
def call_method_fast_with_keywords(obj, arg1, arg2):
2201+
return obj.meth_fastcall_keywords_coexist(arg1, arg2)
2202+
2203+
def call_site(f):
2204+
[call] = [instr for instr in dis.get_instructions(f, adaptive=True)
2205+
if instr.baseopname == "CALL"]
2206+
cache = {name: data for name, _, data in call.cache_info}
2207+
return call.opname, cache["counter"]
2208+
2209+
def label(obj):
2210+
return getattr(obj, "__qualname__", type(obj).__name__)
2211+
2212+
coexist = _testcapi.MethInstance()
2213+
cases = [
2214+
# dict.__contains__ has METH_O | METH_COEXIST
2215+
(call_1, {}.__contains__, ("key",), "CALL_BUILTIN_O"),
2216+
(call_method, {}, ("key",), "CALL_METHOD_DESCRIPTOR_O"),
2217+
# meth_noargs_coexist has METH_NOARGS | METH_COEXIST
2218+
(call_method_noargs, _testcapi.DocStringNoSignatureTest(), (),
2219+
"CALL_METHOD_DESCRIPTOR_NOARGS"),
2220+
# METH_FASTCALL, with or without METH_KEYWORDS, and METH_COEXIST
2221+
(call_method_fast, coexist, (1, 2),
2222+
"CALL_METHOD_DESCRIPTOR_FAST"),
2223+
(call_method_fast_with_keywords, coexist, (1, 2),
2224+
"CALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS"),
2225+
(call_3, _testcapi.MethInstance.meth_fastcall_coexist,
2226+
(coexist, 1, 2), "CALL_METHOD_DESCRIPTOR_FAST"),
2227+
(call_3, _testcapi.MethInstance.meth_fastcall_keywords_coexist,
2228+
(coexist, 1, 2), "CALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS"),
2229+
(call_2, coexist.meth_fastcall_coexist, (1, 2),
2230+
"CALL_BUILTIN_FAST"),
2231+
(call_2, coexist.meth_fastcall_keywords_coexist, (1, 2),
2232+
"CALL_BUILTIN_FAST_WITH_KEYWORDS"),
2233+
]
2234+
for owner in (_testcapi.MethClass, _testcapi.MethStatic):
2235+
cases += [
2236+
(call_1, owner.meth_o, (1,), "CALL_BUILTIN_O"),
2237+
(call_2, owner.meth_fastcall, (1, 2), "CALL_BUILTIN_FAST"),
2238+
(call_2, owner.meth_fastcall_keywords, (1, 2),
2239+
"CALL_BUILTIN_FAST_WITH_KEYWORDS"),
2240+
]
2241+
2242+
for f, func, args, opname in cases:
2243+
with self.subTest(call=f.__name__, func=label(func)):
2244+
reset_code(f)
2245+
expected = f(func, *args)
2246+
for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD):
2247+
f(func, *args)
2248+
self.assertEqual(call_site(f)[0], opname)
2249+
2250+
# A hit leaves the counter of the call site unchanged.
2251+
# A miss decrements it.
2252+
before = call_site(f)
2253+
for _ in range(10):
2254+
self.assertEqual(f(func, *args), expected)
2255+
self.assertEqual(call_site(f), before)
2256+
21742257
@cpython_only
21752258
@requires_specialization
21762259
def test_load_attr_module_with_getattr(self):
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix call specialization for C functions and methods with ``METH_CLASS``,
2+
``METH_STATIC``, or ``METH_COEXIST`` flags, improving their call performance.

‎Modules/_testcapimodule.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3506,6 +3506,11 @@ static PyMethodDef meth_instance_methods[] = {
35063506
{"meth_noargs", meth_noargs, METH_NOARGS},
35073507
{"meth_fastcall", _PyCFunction_CAST(meth_fastcall), METH_FASTCALL},
35083508
{"meth_fastcall_keywords", _PyCFunction_CAST(meth_fastcall_keywords), METH_FASTCALL|METH_KEYWORDS},
3509+
{"meth_fastcall_coexist", _PyCFunction_CAST(meth_fastcall),
3510+
METH_FASTCALL|METH_COEXIST},
3511+
{"meth_fastcall_keywords_coexist",
3512+
_PyCFunction_CAST(meth_fastcall_keywords),
3513+
METH_FASTCALL|METH_KEYWORDS|METH_COEXIST},
35093514
{NULL, NULL} /* sentinel */
35103515
};
35113516

‎Modules/_testinternalcapi/test_cases.c.h‎

Lines changed: 14 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Objects/descrobject.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -928,8 +928,7 @@ PyDescr_NewMethod(PyTypeObject *type, PyMethodDef *method)
928928
{
929929
/* Figure out correct vectorcall function to use */
930930
vectorcallfunc vectorcall;
931-
switch (method->ml_flags & (METH_VARARGS | METH_FASTCALL | METH_NOARGS |
932-
METH_O | METH_KEYWORDS | METH_METHOD))
931+
switch (method->ml_flags & _Py_METH_CALL_FLAGS)
933932
{
934933
case METH_VARARGS:
935934
vectorcall = method_vectorcall_VARARGS;

‎Objects/methodobject.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ PyCMethod_New(PyMethodDef *ml, PyObject *self, PyObject *module, PyTypeObject *c
4848
{
4949
/* Figure out correct vectorcall function to use */
5050
vectorcallfunc vectorcall;
51-
switch (ml->ml_flags & (METH_VARARGS | METH_FASTCALL | METH_NOARGS |
52-
METH_O | METH_KEYWORDS | METH_METHOD))
51+
switch (ml->ml_flags & _Py_METH_CALL_FLAGS)
5352
{
5453
case METH_VARARGS:
5554
case METH_VARARGS | METH_KEYWORDS:

0 commit comments

Comments
 (0)