Repository navigation
gh-144549: Fix tail calling interpreter on Windows for FT - #144550
Fidget-Spinner merged 10 commits into
Conversation
Sorry, something went wrong.
savannahostrowski
left a comment
There was a problem hiding this comment.
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.
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
Yep - agreed. Opened #144552. I'll work on this as a follow up. |
Sorry, something went wrong.
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.