Skip to content

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

Merged
Fidget-Spinner merged 1 commit into
python:3.13from
Fidget-Spinner:optim_frame_backport
Jun 20, 2024
Merged

[3.13] gh-119258: Backport optimizer frame fixes in GH-119365#120699
Fidget-Spinner merged 1 commit into
python:3.13from
Fidget-Spinner:optim_frame_backport

Conversation

@Fidget-Spinner

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

Copy link
Copy Markdown
Member

Removes frame overlapping, instead copy over their arguments directly.

@Fidget-Spinner

Copy link
Copy Markdown
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

brandtbucher commented Jun 18, 2024

Copy link
Copy Markdown
Member

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

@Fidget-Spinner

Copy link
Copy Markdown
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
Copy Markdown
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
@Fidget-Spinner
Fidget-Spinner deleted the optim_frame_backport branch June 20, 2024 15:55
@brandtbucher

Copy link
Copy Markdown
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
Copy Markdown
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

markshannon commented Jun 21, 2024

Copy link
Copy Markdown
Member

_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