Skip to content

GH-139922: Tail calling for MSVC (VS 2026) #143068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Fidget-Spinner merged 11 commits into from
Dec 22, 2025
Merged

GH-139922: Tail calling for MSVC (VS 2026) #143068

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
374874c
rebased + _Py_VectorCallInstrumentation_StackRefSteal
chris-eibl Dec 21, 2025
e0cd0f5
Merge commit '374874c3e6d8c4745a52b3bc22044db607b2c9e7' into msvc-tai…
chris-eibl Dec 21, 2025
7e5555a
try to fix MSVC tail call CI
chris-eibl Dec 22, 2025
a75042a
Alternative to restrict and conditional Py_NO_INLINE for PyMapping_Ge…
chris-eibl Dec 22, 2025
fdfd8a0
make _PyMapping_GetOptionalItem2 private and comment scopes
chris-eibl Dec 22, 2025
058889e
Merge with main because aarch64 jit builds started failing
chris-eibl Dec 22, 2025
c8d051b
Apply suggestions from code review
chris-eibl Dec 22, 2025
ab70524
Update .github/workflows/tail-call.yml
chris-eibl Dec 22, 2025
5c1c240
enable "Emulated Windows Clang (release)"
chris-eibl Dec 22, 2025
a04824f
Merge remote-tracking branch 'upstream/main' into msvc-tail-call-new
chris-eibl Dec 22, 2025
da1adaf
revert to cmd but keep Hugo's other changes
chris-eibl Dec 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Alternative to restrict and conditional Py_NO_INLINE for PyMapping_Ge… 
…tOptionalItem
  • Loading branch information
@chris-eibl
chris-eibl committed Dec 22, 2025
commit a75042a73e7173ea9652b1c01ead78239b7e0d52
1 change: 1 addition & 0 deletions Include/abstract.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ PyAPI_FUNC(PyObject *) PyMapping_GetItemString(PyObject *o,
*/
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030d0000
PyAPI_FUNC(int) PyMapping_GetOptionalItem(PyObject *, PyObject *, PyObject **);
PyAPI_FUNC(PyObject*) PyMapping_GetOptionalItem2(PyObject *, PyObject *, int *);
Comment thread
chris-eibl marked this conversation as resolved.
Outdated
PyAPI_FUNC(int) PyMapping_GetOptionalItemString(PyObject *, const char *, PyObject **);
#endif

Expand Down
16 changes: 9 additions & 7 deletions Objects/abstract.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,14 +205,8 @@ PyObject_GetItem(PyObject *o, PyObject *key)
return type_error("'%.200s' object is not subscriptable", o);
}

// MSVC fails during a tail call release build with loads of
// error C4737: Unable to perform required tail call.
// without using Py_NO_INLINE here, but PGO works fine.
#if defined(_MSC_VER) && !defined(__clang__) && _Py_TAIL_CALL_INTERP && !defined(_Py_USING_PGO)

@chris-eibl chris-eibl Dec 22, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It needed lots of trial and error to come up with this unintuive minimal change needed to convince MSVC that the local variable passed by reference into result here does not escape the tail call.

Using it as an return value in PyMapping_GetOptionalItem2 is more explicit, but might have other drawbacks?

Py_NO_INLINE
#endif
int
PyMapping_GetOptionalItem(PyObject *obj, PyObject *key, PyObject **restrict result)
PyMapping_GetOptionalItem(PyObject *obj, PyObject *key, PyObject **result)
{
if (PyDict_CheckExact(obj)) {
return PyDict_GetItemRef(obj, key, result);
Expand All @@ -230,6 +224,14 @@ PyMapping_GetOptionalItem(PyObject *obj, PyObject *key, PyObject **restrict resu
return 0;
}

PyObject*
PyMapping_GetOptionalItem2(PyObject *obj, PyObject *key, int *err)
{
PyObject* result;
*err = PyMapping_GetOptionalItem(obj, key, &result);
return result;
}

int
PyObject_SetItem(PyObject *o, PyObject *key, PyObject *value)
{
Expand Down
21 changes: 11 additions & 10 deletions Python/bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1507,8 +1507,8 @@ dummy_func(
}

inst(LOAD_BUILD_CLASS, ( -- bc)) {
PyObject *bc_o;
int err = PyMapping_GetOptionalItem(BUILTINS(), &_Py_ID(__build_class__), &bc_o);
int err;
PyObject *bc_o = PyMapping_GetOptionalItem2(BUILTINS(), &_Py_ID(__build_class__), &err);
ERROR_IF(err < 0);
if (bc_o == NULL) {
_PyErr_SetString(tstate, PyExc_NameError,
Expand Down Expand Up @@ -1711,8 +1711,9 @@ dummy_func(

inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) {
PyObject *name = GETITEM(FRAME_CO_NAMES, oparg);
PyObject *v_o;
int err = PyMapping_GetOptionalItem(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &v_o);
int err;
PyObject *v_o = PyMapping_GetOptionalItem2(PyStackRef_AsPyObjectBorrow(mod_or_class_dict), name, &err);

PyStackRef_CLOSE(mod_or_class_dict);
ERROR_IF(err < 0);
if (v_o == NULL) {
Expand All @@ -1735,11 +1736,11 @@ dummy_func(
else {
/* Slow-path if globals or builtins is not a dict */
/* namespace 1: globals */
int err = PyMapping_GetOptionalItem(GLOBALS(), name, &v_o);
v_o = PyMapping_GetOptionalItem2(GLOBALS(), name, &err);
ERROR_IF(err < 0);
if (v_o == NULL) {
/* namespace 2: builtins */
int err = PyMapping_GetOptionalItem(BUILTINS(), name, &v_o);
v_o = PyMapping_GetOptionalItem2(BUILTINS(), name, &err);
ERROR_IF(err < 0);
if (v_o == NULL) {
_PyEval_FormatExcCheckArg(
Expand Down Expand Up @@ -1898,14 +1899,14 @@ dummy_func(
}

inst(LOAD_FROM_DICT_OR_DEREF, (class_dict_st -- value)) {
PyObject *value_o;
PyObject *name;
PyObject *class_dict = PyStackRef_AsPyObjectBorrow(class_dict_st);

assert(class_dict);
assert(oparg >= 0 && oparg < _PyFrame_GetCode(frame)->co_nlocalsplus);
name = PyTuple_GET_ITEM(_PyFrame_GetCode(frame)->co_localsplusnames, oparg);
int err = PyMapping_GetOptionalItem(class_dict, name, &value_o);
int err;
PyObject* value_o = PyMapping_GetOptionalItem2(class_dict, name, &err);
if (err < 0) {
ERROR_NO_POP();
}
Expand Down Expand Up @@ -2074,14 +2075,14 @@ dummy_func(
}

inst(SETUP_ANNOTATIONS, (--)) {
PyObject *ann_dict;
if (LOCALS() == NULL) {
_PyErr_Format(tstate, PyExc_SystemError,
"no locals found when setting up annotations");
ERROR_IF(true);
}
/* check if __annotations__ in locals()... */
int err = PyMapping_GetOptionalItem(LOCALS(), &_Py_ID(__annotations__), &ann_dict);
int err;
PyObject* ann_dict = PyMapping_GetOptionalItem2(LOCALS(), &_Py_ID(__annotations__), &err);
ERROR_IF(err < 0);
if (ann_dict == NULL) {
ann_dict = PyDict_New();
Expand Down
12 changes: 6 additions & 6 deletions Python/executor_cases.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions Python/generated_cases.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading