Skip to content

gh-157833: Fix specialized C calls with additional method flags - #157834

Open
overcat wants to merge 3 commits into
python:mainfrom
overcat:fix-call-builtin-meth-flags
Open

overcat wants to merge 3 commits into
python:mainfrom
overcat:fix-call-builtin-meth-flags

Conversation

@overcat

@overcat overcat commented Sep 20, 2026

Copy link
Copy Markdown

Fixes #157833

Ignore METH_CLASS, METH_STATIC and METH_COEXIST in the specialized C call guards and corresponding Tier 2 optimizer checks. This prevents calls accepted by the specializer from failing their flag guards on every call. METH_METHOD remains checked because it changes the C calling convention.

Add Tier 1 and Tier 2 regression tests for all seven affected instructions, regenerate the affected files.

@python-cla-bot

python-cla-bot Bot commented Sep 20, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@cocolato cocolato left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can add a mask macro like this:

#define _Py_METH_CALL_FLAGS \
    (METH_VARARGS | METH_FASTCALL | METH_NOARGS | METH_O | METH_KEYWORDS | METH_METHOD)

Comment thread Python/bytecodes.c Outdated
Comment on lines +4874 to +4878
int flags = PyCFunction_GET_FLAGS(callable_o);
// METH_CLASS, METH_STATIC and METH_COEXIST do not change the C
// calling convention, and the specializer ignores them.
flags &= ~(METH_CLASS | METH_STATIC | METH_COEXIST);
EXIT_IF(flags != METH_O);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int flags = PyCFunction_GET_FLAGS(callable_o);
// METH_CLASS, METH_STATIC and METH_COEXIST do not change the C
// calling convention, and the specializer ignores them.
flags &= ~(METH_CLASS | METH_STATIC | METH_COEXIST);
EXIT_IF(flags != METH_O);
EXIT_IF((PyCFunction_GET_FLAGS(callable_o) & _Py_METH_CALL_FLAGS) != METH_O);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @cocolato, thanks for the suggestion! Updated to use _Py_METH_CALL_FLAGS.
5bc4896

Comment thread Python/optimizer_bytecodes.c Outdated
Comment on lines +1714 to +1716
int flags = PyCFunction_GET_FLAGS(callable_o);
flags &= ~(METH_CLASS | METH_STATIC | METH_COEXIST);
if (total_args == 1 && flags == METH_O) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Suggested change
int flags = PyCFunction_GET_FLAGS(callable_o);
flags &= ~(METH_CLASS | METH_STATIC | METH_COEXIST);
if (total_args == 1 && flags == METH_O) {
if ((PyCFunction_GET_FLAGS(callable_o) & _Py_METH_CALL_FLAGS) == METH_FASTCALL) {

@overcat
overcat requested a review from cocolato September 20, 2026 11:09
Comment thread Lib/test/test_capi/test_opt.py Outdated
Comment on lines +21 to +24
try:
import _testcapi
except ImportError:
_testcapi = None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move import_helper.import_module("_testcapi") into the six tests that need it, along with their fixture setup? The helper would skip only the current test if the module is unavailable, so we could drop this try/except and the skipIf decorators.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the input! Addressed in 4484374

Comment thread Lib/test/test_capi/test_opt.py Outdated
Comment on lines +40 to +43
# Keep these callables in globals so the optimizer can resolve their values.
# They have METH_CLASS, METH_STATIC, or METH_COEXIST flags.
if _testcapi is not None:
METH_CLASS_O = _testcapi.MethClass.meth_o

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can define the fixtures in each test and bind them as that function's globals before calling _run_with_optimizer:

testfunc = types.FunctionType(testfunc.__code__, namespace)

@overcat
overcat requested a review from cocolato September 21, 2026 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C call specialization misses with METH_CLASS, METH_STATIC or METH_COEXIST

2 participants