Skip to content

[3.13] gh-119258: Backport optimizer frame fixes in GH-119365 #120699

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.

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
merged 1 commit into from
Jun 20, 2024

Conversation

Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Jun 18, 2024

Removes frame overlapping, instead copy over their arguments directly.

@Fidget-Spinner
Copy link
Member Author

Skip news as it's a experimental feature.

@Fidget-Spinner Fidget-Spinner changed the title gh-119258: Backport optimizer frame fixes in GH-119365 [3.13] gh-119258: Backport optimizer frame fixes in GH-119365 Jun 18, 2024
@brandtbucher
Copy link
Member

brandtbucher commented Jun 18, 2024

Does this mean we can correctly optimize across _PY_FRAME_GENERAL now instead of bailing?

@Fidget-Spinner
Copy link
Member Author

I think we were already doing that. This just fixes a bug with overlapping frames (it removes it altogether).

@Fidget-Spinner
Copy link
Member Author

Merging this as backport bugfixes don't normally need approval (or at least, I can approve it if it were done by the bot).

@Fidget-Spinner Fidget-Spinner merged commit 7c7aa5a into python:3.13 Jun 20, 2024
50 of 55 checks passed
@Fidget-Spinner Fidget-Spinner deleted the optim_frame_backport branch June 20, 2024 15:55
@brandtbucher
Copy link
Member

I think we were already doing that. This just fixes a bug with overlapping frames (it removes it altogether).

Maybe I'm misunderstanding. Is there anything preventing us from updating _PY_FRAME_GENERAL in the optimizer to push a new frame and continue optimizing, rather than bailing (like it does now)?

@Fidget-Spinner
Copy link
Member Author

IIRC we used to not be able to determine the size of the nee frame to push which is why we bailed. If that's solved I dont see why we cant toptimize through it.

@markshannon
Copy link
Member

markshannon commented Jun 21, 2024

_Py_uop_frame_new still assumes positional arguments only, but it would be simple enough to drop that assumption by initializing the arguments in the caller.

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.

3 participants