gh-139922: Tail calling for MSVC (VS 2026) #139962

Fidget-Spinner · 2025-10-11T16:45:56Z

I know it isn't good practice to bundle multiple changes into the same PR, but this PR does two things which are required to get tail calling working on MSVC and which I think are beneficial.

Move the _PyStackRef off a per-ceval allocated buffer into a per-thread one. This actually reduces the C stack consumption of ceval.c. I had planned this for some time (see Move stackref buffer to thread state to reduce interp stack usage #138115), but we also need this to let MSVC know the variables are fine to outlive their current scope. This may be a tiny bit slower versus allocating it on the stack, but it removes the performance cliff of having any function calling more than 10 arguments suddenly getting a lot slower due to having to allocate memory. Overall, I think this is a better design choice for the two reasons of less C stack consumption and allowing longer calls.
Add __restrict to correct places and scoping where needed to tell MSVC that a local variable does not outlive the scope. We only use this for MSVC for now as other compilers have different qualifier rules. However, it's definitely possible to extend this to clang and gcc to let them benefit in the future.

There is no noticeable perf drop on a nogil benchmark for the normal interpreter https://github.com/facebookexperimental/free-threading-benchmarking/tree/main/results/bm-20251011-3.15.0a0-0786133-NOGIL

Issue: Tail calling interpreter for MSVC #139922

Co-Authored-By: Brandt Bucher <[email protected]>

Co-Authored-By: Hulon Jenkins <[email protected]>

Co-Authored-By: Chris Eibl <[email protected]>

…ython into msvc-tailcall-take-two

Include/pyport.h

…ython into msvc-tailcall-take-two

zooba

Very neat, looks good to me, but don't let me be the final reviewer on this.

zooba · 2025-10-13T13:49:02Z

Include/internal/pycore_tstate.h

 #endif

+/* How much scratch space to give stackref to PyObject* conversion. */
+#define MAX_STACKREF_SCRATCH 1024


How arbitrary is this amount? What is the usual amount of space required for conversions?

We used to arbitrarily always consume 10 PyObject/PyStackRef per call. Meaning even if you had a single item, the whole 10 slot space would be used.

zooba · 2025-10-13T13:52:52Z

Python/bytecodes.c

        inst(LOAD_BUILD_CLASS, ( -- bc)) {
-            PyObject *bc_o;
-            int err = PyMapping_GetOptionalItem(BUILTINS(), &_Py_ID(__build_class__), &bc_o);
+            PyObject *Py_MSVC_RESTRICT bc_o;


I wonder if there's a more semantically meaningful name than Py_MSVC_RESTRICT here? "Unaliased" or "no escape"?

And possibly define it as a macro like #define UNALIASED(x) x Py_MSVC_RESTRICT to get UNALIASED(PyObject *) bc_o?

Just looking to reduce the sense of "oh I have to go look up this MSVC thing to know what's going on here" when reading the code. There may be other ways.

zooba · 2025-10-13T13:53:20Z

Python/bytecodes.c

+            int *Py_MSVC_RESTRICT method_found_ptr = &method_found;
+            attr_o = _PySuper_Lookup(cls, self, name,
+                                   Py_TYPE(self)->tp_getattro == PyObject_GenericGetAttr ? method_found_ptr : NULL);


Indent? Like in other places

zooba · 2025-10-13T13:53:43Z

Python/bytecodes.c

-            PyObject *stack[] = {class, self};
-            PyObject *super = PyObject_Vectorcall(global_super, stack, oparg & 2, NULL);
+            PyObject *super;
+            Py_BEGIN_LOCALS_MUST_NOT_ESCAPE();


We usually don't require parentheses on macros like this, especially when they define a scope.

zooba · 2025-10-13T13:55:55Z

PCbuild/pythoncore.vcxproj

    <ClCompile Include="..\Python\brc.c" />
-    <ClCompile Include="..\Python\ceval.c" />
+    <ClCompile Include="..\Python\ceval.c">
+      <AdditionalOptions Condition="'$(UseTailCallInterp)' == 'true' and $(PlatformToolset) != 'ClangCL'">/std:clatest %(AdditionalOptions)</AdditionalOptions>


What happens if we pass this option to clang-cl? Does it break?

Any possibility of passing a specific /std:c rather than "latest"?

So I've worked with Chris on this and apparently it builds with clang-cl still the last time I checked with him? @chris-eibl

What happens if we pass this option to clang-cl? Does it break?

Yupp, clang-cl doesn't like /std:clatest, hence I omitted it, because it doesn't need it.
Seems the clang-cl versions we are using (>=19)

support by default at least C99 (which is would be needed for restrict)

and support must tail, etc, in the "clang specific notation"

But I assume MSVC needs /std:clatest for [[msvc::musttail]] - @Fidget-Spinner correct?
And at least MSVC definitely needs at least /std:c11 for restrict (https://learn.microsoft.com/en-us/cpp/build/reference/std-specify-language-standard-version?view=msvc-170).

Any possibility of passing a specific /std:c rather than "latest"?

Yeah, we could explicitely enforce /std:c11for clang-cl, but we didn't need it in the past and AFAICT this PR didn't change anything that clang-cl sees, due to carefully crafted combinations of _MSC_VER and __clang__ like

#if defined(_MSC_VER) && !defined(__clang__)

and apparently it builds with clang-cl still the last time I checked with him?

Yes, it still does. Having done a lot of builds recently and still playing around.
And tail calling CI is green as well: https://github.com/python/cpython/actions/runs/18435075900/job/52527447202

I'm under the impression we need clatest for msvc musttail as well.

I've just verified again:

MSVC needs /std:clatest for [[msvc::musttail]]

using /std:c17 fails with error C2143: syntax error: missing ';' before '['

clang-cl doesn't accept /std:clatest

So atm we're stuck with this construct until MSVC introduces a new "fixed" option (or maybe accepts [[msvc::musttail]] in /std:c17).

I agree with Steve #135927 (comment)

For the record, I don't like building with Clatest and if we're considering making that change upstream, we should pick a version.

and we should revisit this once we know a fixed version option for it.

Co-Authored-By: Chris Eibl <[email protected]>

…inner/cpython into msvc-tailcall-take-two

zooba · 2025-10-13T21:25:51Z

Include/internal/pycore_ceval.h

+#  define Py_NO_INLINE_MSVC_TAILCALL
+#endif
+
+// Tells the compiler that this variable cannot be alised.


Grammar nit, but "cannot be aliased" is ambiguous (because it's not clear whether we are choosing not to alias it, or telling the compiler that it cannot choose to alias it).

I think we're promising that it won't be aliased? So "Tells the compiler to trust that we haven't aliased this variable" better explains what it's doing. Or just "... this variable is not going to be aliased".

Alternatively, "Tells the compiler not to alias the variable" (but I don't think that even makes sense, let alone being correct here).

Python/ceval.c

zooba · 2025-10-13T21:29:07Z

Python/executor_cases.c.h

+            PyObject *attr_o;
+            Py_BEGIN_LOCALS_MUST_NOT_ESCAPE;


Another crazy idea, based on seeing how this is used (feel free to ignore unless you really like it):

Py_BEGIN_RESTRICTED_PYOBJECT_SCOPE(attr_o); ... attr_o = ... ... Py_END_RESTRICTED_PYOBJECT_SCOPE;

It seems like it only gets used in the same pattern, and this makes it really clear what it's for (at the expense of overly limiting how it could be used).

But it's all generated code, so not a big deal.

markshannon · 2025-10-14T14:49:27Z

I know it isn't good practice to bundle multiple changes into the same PR

Indeed not. Can we move all the restrict annotations to their own PR? Are they MSVC specific? If the restrict annotation is correct, we should add them for all platforms. If they are not correct we shouldn't add them anywhere.

markshannon · 2025-10-14T14:51:58Z

Python/bytecodes.c

            // we make no attempt to optimize here; specializations should
            // handle any case whose performance we care about
+            PyObject *super;
+            Py_BEGIN_LOCALS_MUST_NOT_ESCAPE;


Suggested change

Py_BEGIN_LOCALS_MUST_NOT_ESCAPE;

{ // 'stack' array must not escape

Fidget-Spinner · 2025-10-14T14:53:58Z

I know it isn't good practice to bundle multiple changes into the same PR

Indeed not. Can we move all the restrict annotations to their own PR? Are they MSVC specific? If the restrict annotation is correct, we should add them for all platforms. If they are not correct we shouldn't add them anywhere.

Fair. This PR will be just fixing the stackref buffer then.

markshannon · 2025-10-14T14:59:48Z

Include/internal/pycore_ceval.h

 #define SPECIAL_MAX   3

+// Special counterparts of ceval functions for performance reasons
+PyAPI_FUNC(int) _PyEval_Mapping_GetOptionalItem(PyObject *obj, PyObject *key, PyObject **result);


Is the only difference the restrict annotation?
We could just add the annotation to PyMapping_GetOptionalItem the result pointer can never alias the other parameters.

Yes. We can't because PyMapping_GetOptionalItem is part of the public API, and we can't change the signature of it.

We aren't changing the signature, just telling the compiler than one of its parameters can't alias the others.
I'm surprised we need to do this anyway. I would have thought that strict aliasing already tells MSVC that they can't alias.

My understanding is, that strict aliasing can't help here, since in

PyMapping_GetOptionalItem(PyObject *obj, PyObject *key, PyObject **result)

all parameters are of type PyObject. And by using restrict we tell the compiler that "we assure you, that none of these PyObjects point to the same memory", and if we brake that contract, UB kicks in.

markshannon · 2025-10-14T15:28:35Z

What is necessary for tail calling?

IIUC, the restict annotations and the tighter scoping around arrays in the interpreter are necessary, but moving the temporary arrays from the C stack to the threadstate are not. Is that correct?

Fidget-Spinner · 2025-10-14T15:42:59Z

IIUC, the restict annotations and the tighter scoping around arrays in the interpreter are necessary, but moving the temporary arrays from the C stack to the threadstate are not. Is that correct?

No this is needed too. The problem is that the temporary arrays from the C stack are local variables. These variables are passed around in ways that are unpredictable to MSVC (such as passing to an arbitrary vectorcall function pointer). So we have to move this to thread state to get tail calling.

There are other side benefits and I've wanted to do this for awhile, so I thought might as well put this in this PR. Do you still want to split up the restrict stuff knowing that we need both to get TC on MSVC?

markshannon · 2025-10-15T09:19:36Z

This PR should contain everything that is necessary for tailcalling. If that includes moving the temporary arrays then move them.

But, why is it necessary to move the arrays? If this array https://github.com/python/cpython/pull/139962/files#diff-729a985b0cb8b431cb291f1edb561bbbfea22e3f8c262451cd83328a0936a342R3522 is made OK by adding parentheses, then why not the temporary arrays? In fact, why do we need to insert the parentheses in bytecodes.c at all. Why not do it in the code generator as we discussed a while ago?

Fidget-Spinner · 2025-10-15T09:52:05Z

I'm not sure what MSVC is doing under the hood here, but the last time I tried, just adding scoping around the temporary arrays doesn't work.

It seems to be more lines of code and more maintainence burden to add functionality to the cases generator to detect what is the smallest possible scope to wrap a escaping local variable around. We dont have that many cases in bytecodes.c so I think we should just do it manually. Note that we cant just add curly braces, we need to pass variables around too in certain cases.

Fidget-Spinner · 2025-10-22T20:38:44Z

@markshannon gentle ping to see the latest comment please, thanks!

Co-Authored-By: Chris Eibl <[email protected]>

Fidget-Spinner and others added 7 commits August 21, 2025 22:20

Brandt's changes

82d1259

Co-Authored-By: Brandt Bucher <[email protected]>

Hulon's and mine changes

3248658

Co-Authored-By: Hulon Jenkins <[email protected]>

push stackrefs into tstate

9ac430d

Fix last few remaining problems

085c1d7

fix handling of recursion

0b12f2e

Changes by Chris to support clang-cl

acf48f5

Co-Authored-By: Chris Eibl <[email protected]>

Make the restrict MSVC only for now

35e96c1

Fidget-Spinner requested review from a team, ZeroIntensity, ericsnowcurrently, markshannon and methane as code owners October 11, 2025 16:45

bedevere-app bot added the awaiting core review label Oct 11, 2025

bedevere-app bot mentioned this pull request Oct 11, 2025

Tail calling interpreter for MSVC #139922

Open

Fidget-Spinner and others added 7 commits October 11, 2025 17:47

Merge remote-tracking branch 'upstream/main' into msvc-tailcall-take-two

d7737e9

Change restricts to MSVC only

86f19cf

📜🤖 Added by blurb_it.

40013cc

Reduce diff

48db59e

Merge branch 'msvc-tailcall-take-two' of github.com:Fidget-Spinner/cp…

bc9d23c

…ython into msvc-tailcall-take-two

Work around specialization

19e02c2

Fix VS 2026

66ec774

ZeroIntensity reviewed Oct 11, 2025

View reviewed changes

Include/pyport.h Outdated Show resolved Hide resolved

Fidget-Spinner added 3 commits October 12, 2025 02:27

fix a typo

50f8ff7

Move to macros to internal header

5d908b4

Merge branch 'msvc-tailcall-take-two' of github.com:Fidget-Spinner/cp…

0786133

…ython into msvc-tailcall-take-two

zooba reviewed Oct 13, 2025

View reviewed changes

Fidget-Spinner and others added 3 commits October 14, 2025 04:49

Reduce number of restricts

e699d40

Co-Authored-By: Chris Eibl <[email protected]>

Merge branch 'msvc-tailcall-take-two' of https://github.com/Fidget-Sp…

66d6c39

…inner/cpython into msvc-tailcall-take-two

Reduce restrict use even more, reduce usage

5584fec

zooba reviewed Oct 13, 2025

View reviewed changes

Python/ceval.c Show resolved Hide resolved

zooba reviewed Oct 13, 2025

View reviewed changes

markshannon reviewed Oct 14, 2025

View reviewed changes

Fidget-Spinner mentioned this pull request Oct 18, 2025

gh-139109: A new tracing JIT compiler frontend for CPython #140310

Open

Apply Chris' changes

7eeeaa8

Co-Authored-By: Chris Eibl <[email protected]>

chris-eibl mentioned this pull request Oct 24, 2025

We should fail to compile if _Py_TAIL_CALL_INTERP is set but preserve_none and musttail do not exist. #140513

Open

	Py_BEGIN_LOCALS_MUST_NOT_ESCAPE;
	{ // 'stack' array must not escape

Uh oh!

gh-139922: Tail calling for MSVC (VS 2026) #139962

Are you sure you want to change the base?

gh-139922: Tail calling for MSVC (VS 2026) #139962

Uh oh!

Conversation

Fidget-Spinner commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

zooba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-eibl Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zooba Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markshannon commented Oct 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fidget-Spinner commented Oct 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markshannon Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markshannon commented Oct 14, 2025

Uh oh!

Fidget-Spinner commented Oct 14, 2025

Uh oh!

markshannon commented Oct 15, 2025

Uh oh!

Fidget-Spinner commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fidget-Spinner commented Oct 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fidget-Spinner commented Oct 11, 2025 •

edited

Loading

chris-eibl Oct 13, 2025 •

edited

Loading

zooba Oct 13, 2025 •

edited

Loading

markshannon Oct 14, 2025 •

edited

Loading

Fidget-Spinner commented Oct 15, 2025 •

edited

Loading