Skip to content

gh-144549: Fix tail calling interpreter on Windows for FT#144550

Merged
Fidget-Spinner merged 10 commits intopython:mainfrom
Fidget-Spinner:ft_tc_windows
Feb 6, 2026
Merged

gh-144549: Fix tail calling interpreter on Windows for FT#144550
Fidget-Spinner merged 10 commits intopython:mainfrom
Fidget-Spinner:ft_tc_windows

Conversation

@Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Feb 6, 2026

I just moved the body of _LOAD_ATTR into an external function.

The problem was that we had a local variable that has its address taken and "escaped" in the tail calling function. By moving this to an external function, the tail calling function no longer has escaping variables.

Also benefits #141794 due to smaller JIT stencil sizes.

Copy link
Member

@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

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

LGTM - I do have more meta comments about existing patterns in the workflow file though:

  • I'm wondering if x86_64-pc-windows-msvc/msvc-free-threading would be more consistent with the triple/compiler pattern.
  • It might be a bit more robust to use a matrix variable here like free-threading: true? If we add more FT variants later, this could get a bit hairy.

Not a problem to solve in this PR since both of these are leveraging existing patterns but maybe some futureproofing we should consider.

@Fidget-Spinner
Copy link
Member Author

LGTM - I do have more meta comments about existing patterns in the workflow file though:

  • I'm wondering if x86_64-pc-windows-msvc/msvc-free-threading would be more consistent with the triple/compiler pattern.
  • It might be a bit more robust to use a matrix variable here like free-threading: true? If we add more FT variants later, this could get a bit hairy.

Not a problem to solve in this PR since both of these are leveraging existing patterns but maybe some futureproofing we should consider.

Thanks for the review. Yeah I agree we need to refactor the CI files. Unfortunately, I'm too bad at this to refactor the file. Even editing the current one to make it work was already a struggle for me. Let's do this in a follow-up PR or let a new contributor take it.

@Fidget-Spinner Fidget-Spinner merged commit 895e837 into python:main Feb 6, 2026
76 checks passed
@savannahostrowski
Copy link
Member

Yep - agreed. Opened #144552. I'll work on this as a follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants