Skip to content

Commit ced1af9

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 6413901 commit ced1af9

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
@@ -3283,6 +3283,186 @@ def testfunc(n):
32833283
self.assertIn("_CALL_BUILTIN_FAST_WITH_KEYWORDS", uops)
32843284
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_FAST_WITH_KEYWORDS", uops)
32853285

3286+
def test_call_builtin_o_extra_flags(self):
3287+
# Extra method flags must not prevent callable guard elimination.
3288+
_testcapi = import_helper.import_module("_testcapi")
3289+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3290+
3291+
namespace = {
3292+
"METH_CLASS_O": _testcapi.MethClass.meth_o,
3293+
"METH_STATIC_O": _testcapi.MethStatic.meth_o,
3294+
"METH_COEXIST_O": {}.__contains__,
3295+
}
3296+
3297+
@reset_code
3298+
def testfunc(n):
3299+
for _ in range(n):
3300+
class_result = METH_CLASS_O(1)
3301+
static_result = METH_STATIC_O(1)
3302+
coexist_result = METH_COEXIST_O(1)
3303+
return class_result, static_result, coexist_result
3304+
3305+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3306+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3307+
self.assertEqual(res, ((_testcapi.MethClass, 1), (None, 1), False))
3308+
self.assertIsNotNone(ex)
3309+
uops = get_opnames(ex)
3310+
self.assertEqual(uops.count("_CALL_BUILTIN_O"), 3)
3311+
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_O", uops)
3312+
3313+
def test_call_builtin_fast_extra_flags(self):
3314+
_testcapi = import_helper.import_module("_testcapi")
3315+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3316+
3317+
obj = _testcapi.MethInstance()
3318+
namespace = {
3319+
"METH_CLASS_FASTCALL": _testcapi.MethClass.meth_fastcall,
3320+
"METH_STATIC_FASTCALL": _testcapi.MethStatic.meth_fastcall,
3321+
"METH_COEXIST_FASTCALL": obj.meth_fastcall_coexist,
3322+
}
3323+
3324+
@reset_code
3325+
def testfunc(n):
3326+
for _ in range(n):
3327+
class_result = METH_CLASS_FASTCALL(1, 2)
3328+
static_result = METH_STATIC_FASTCALL(1, 2)
3329+
coexist_result = METH_COEXIST_FASTCALL(1, 2)
3330+
return class_result, static_result, coexist_result
3331+
3332+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3333+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3334+
self.assertEqual(res, (
3335+
(_testcapi.MethClass, (1, 2)),
3336+
(None, (1, 2)),
3337+
(obj, (1, 2)),
3338+
))
3339+
self.assertIsNotNone(ex)
3340+
uops = get_opnames(ex)
3341+
self.assertEqual(uops.count("_CALL_BUILTIN_FAST"), 3)
3342+
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_FAST", uops)
3343+
3344+
def test_call_builtin_fast_with_keywords_extra_flags(self):
3345+
_testcapi = import_helper.import_module("_testcapi")
3346+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3347+
3348+
obj = _testcapi.MethInstance()
3349+
namespace = {
3350+
"METH_CLASS_FASTCALL_KEYWORDS": (
3351+
_testcapi.MethClass.meth_fastcall_keywords),
3352+
"METH_STATIC_FASTCALL_KEYWORDS": (
3353+
_testcapi.MethStatic.meth_fastcall_keywords),
3354+
"METH_COEXIST_FASTCALL_KEYWORDS": (
3355+
obj.meth_fastcall_keywords_coexist),
3356+
}
3357+
3358+
@reset_code
3359+
def testfunc(n):
3360+
# Use positional arguments to exercise CALL, not CALL_KW.
3361+
for _ in range(n):
3362+
class_result = METH_CLASS_FASTCALL_KEYWORDS(1, 2)
3363+
static_result = METH_STATIC_FASTCALL_KEYWORDS(1, 2)
3364+
coexist_result = METH_COEXIST_FASTCALL_KEYWORDS(1, 2)
3365+
return class_result, static_result, coexist_result
3366+
3367+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3368+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3369+
self.assertEqual(res, (
3370+
(_testcapi.MethClass, (1, 2), {}),
3371+
(None, (1, 2), {}),
3372+
(obj, (1, 2), {}),
3373+
))
3374+
self.assertIsNotNone(ex)
3375+
uops = get_opnames(ex)
3376+
self.assertEqual(uops.count("_CALL_BUILTIN_FAST_WITH_KEYWORDS"), 3)
3377+
self.assertNotIn("_GUARD_CALLABLE_BUILTIN_FAST_WITH_KEYWORDS", uops)
3378+
3379+
def test_call_method_descriptor_o_extra_flags(self):
3380+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3381+
3382+
@reset_code
3383+
def testfunc(n):
3384+
d = {1: None}
3385+
for _ in range(n):
3386+
result = d.__contains__(1)
3387+
return result
3388+
3389+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3390+
self.assertIs(res, True)
3391+
self.assertIsNotNone(ex)
3392+
uops = get_opnames(ex)
3393+
self.assertEqual(uops.count("_CALL_METHOD_DESCRIPTOR_O_INLINE"), 1)
3394+
self.assertNotIn("_GUARD_CALLABLE_METHOD_DESCRIPTOR_O", uops)
3395+
3396+
def test_call_method_descriptor_noargs_extra_flags(self):
3397+
_testcapi = import_helper.import_module("_testcapi")
3398+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3399+
3400+
namespace = {
3401+
"METH_COEXIST_NOARGS_OBJECT": _testcapi.DocStringNoSignatureTest(),
3402+
}
3403+
3404+
@reset_code
3405+
def testfunc(n):
3406+
for _ in range(n):
3407+
result = METH_COEXIST_NOARGS_OBJECT.meth_noargs_coexist()
3408+
return result
3409+
3410+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3411+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3412+
self.assertIsNone(res)
3413+
self.assertIsNotNone(ex)
3414+
uops = get_opnames(ex)
3415+
self.assertEqual(
3416+
uops.count("_CALL_METHOD_DESCRIPTOR_NOARGS_INLINE"), 1)
3417+
self.assertNotIn("_GUARD_CALLABLE_METHOD_DESCRIPTOR_NOARGS", uops)
3418+
3419+
def test_call_method_descriptor_fast_extra_flags(self):
3420+
_testcapi = import_helper.import_module("_testcapi")
3421+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3422+
3423+
obj = _testcapi.MethInstance()
3424+
namespace = {"METH_COEXIST_FAST_OBJECT": obj}
3425+
3426+
@reset_code
3427+
def testfunc(n):
3428+
for _ in range(n):
3429+
result = METH_COEXIST_FAST_OBJECT.meth_fastcall_coexist(1, 2)
3430+
return result
3431+
3432+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3433+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3434+
self.assertEqual(res, (obj, (1, 2)))
3435+
self.assertIsNotNone(ex)
3436+
uops = get_opnames(ex)
3437+
self.assertEqual(uops.count("_CALL_METHOD_DESCRIPTOR_FAST_INLINE"), 1)
3438+
self.assertNotIn("_GUARD_CALLABLE_METHOD_DESCRIPTOR_FAST", uops)
3439+
3440+
def test_call_method_descriptor_fast_with_keywords_extra_flags(self):
3441+
_testcapi = import_helper.import_module("_testcapi")
3442+
self.addCleanup(_testinternalcapi.clear_executor_deletion_list)
3443+
3444+
obj = _testcapi.MethInstance()
3445+
namespace = {"METH_COEXIST_FAST_OBJECT": obj}
3446+
3447+
@reset_code
3448+
def testfunc(n):
3449+
# Use positional arguments to exercise CALL, not CALL_KW.
3450+
for _ in range(n):
3451+
result = (
3452+
METH_COEXIST_FAST_OBJECT.meth_fastcall_keywords_coexist(
3453+
1, 2))
3454+
return result
3455+
3456+
testfunc = types.FunctionType(testfunc.__code__, namespace)
3457+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
3458+
self.assertEqual(res, (obj, (1, 2), {}))
3459+
self.assertIsNotNone(ex)
3460+
uops = get_opnames(ex)
3461+
self.assertEqual(
3462+
uops.count("_CALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS_INLINE"), 1)
3463+
self.assertNotIn(
3464+
"_GUARD_CALLABLE_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS", uops)
3465+
32863466
def test_call_method_descriptor_o(self):
32873467
def testfunc(n):
32883468
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
@@ -3479,6 +3479,11 @@ static PyMethodDef meth_instance_methods[] = {
34793479
{"meth_noargs", meth_noargs, METH_NOARGS},
34803480
{"meth_fastcall", _PyCFunction_CAST(meth_fastcall), METH_FASTCALL},
34813481
{"meth_fastcall_keywords", _PyCFunction_CAST(meth_fastcall_keywords), METH_FASTCALL|METH_KEYWORDS},
3482+
{"meth_fastcall_coexist", _PyCFunction_CAST(meth_fastcall),
3483+
METH_FASTCALL|METH_COEXIST},
3484+
{"meth_fastcall_keywords_coexist",
3485+
_PyCFunction_CAST(meth_fastcall_keywords),
3486+
METH_FASTCALL|METH_KEYWORDS|METH_COEXIST},
34823487
{NULL, NULL} /* sentinel */
34833488
};
34843489

‎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)