FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-130415: Use `POP_TWO_LOAD_CONST_INLINE_BORROW` by savannahostrowski · Pull Request #134241 · python/cpython · GitHub

Repository navigation

GH-130415: Use POP_TWO_LOAD_CONST_INLINE_BORROW - #134241

Closed
savannahostrowski wants to merge 5 commits into
python:mainfrom
savannahostrowski:use_pop_2
Closed

savannahostrowski wants to merge 5 commits into
python:mainfrom
savannahostrowski:use_pop_2

Conversation

savannahostrowski commented May 19, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member

Thanks for the PR. I'd like to advocate for this PR #132733 which will simplify this PR quite a bit. All you'd have to do after that PR lands is to add pure to the current bytecode definition. Then if you want you can improve the cases generator to automatically insert the pops.

Copy link
Copy Markdown
Member Author

Ah, I didn't know that work was happening. I'll add some tests and then add the annotations when your PR lands.

Copy link
Copy Markdown
Member

Thanks! The tests are definitely still gonna be very useful :).

Copy link
Copy Markdown
Member

@savannahostrowski sorry for blocking this PR! The base PR has landed.

You just have to replace the body in the optimizer_bytecodes.c with REPLACE_OPCODE_IF_EVALUATES_PURE(left, right) now.

Copy link
Copy Markdown
Member

It’ll also involve modifying the cases generator to insert the new instruction when there is a single output and it’s immortal, too.

Copy link
Copy Markdown
Member

We should also swap out _COMPARE_OP_INT to use the new style in this PR while we're at it I think.

Copy link
Copy Markdown
Member Author

Closing this in favour of a different approach using the new machinery Ken Jin introduced for pure ops.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL