Skip to content

GH-132732: Use pure op machinery to optimize various instructions with _POP_TOP and _POP_TWO #137577

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Aug 9, 2025

@savannahostrowski savannahostrowski changed the title GH-132732: Use pure op machinery to optimize various _POP_TOP and _POP_TWO relevant instructions GH-132732: Use pure op machinery to optimize various instructions with _POP_TOP and _POP_TWO Aug 9, 2025
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

This is super cool! Just a quick run-through for now:

@bedevere-app
Copy link

bedevere-app bot commented Aug 9, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

Very cool, just one nit.

@@ -412,6 +413,7 @@ dummy_func(void) {
}

op(_UNARY_INVERT, (value -- res)) {
REPLACE_OPCODE_IF_EVALUATES_PURE(value);
Copy link
Member

Choose a reason for hiding this comment

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

I think this isn't pure for bool. ~True raises a warning.

Copy link
Member

Choose a reason for hiding this comment

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

Oof, nice catch.

Copy link
Member Author

@savannahostrowski savannahostrowski Aug 9, 2025

Choose a reason for hiding this comment

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

Does it make sense to guard against booleans? Or are we saying that we can't do that/it's not worth it?

Also, can one of you explain why ~True isn't pure and how you discovered this/where the warning is raised?

Copy link
Member

Choose a reason for hiding this comment

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

We could guard against booleans I think.

Anything that raises a warning isn't pure as that means the optimizer might cause a warning to be emitted during optimizing code, and we don't want that.

To test it out, try typing ~True into the current main's CPython repl. It's a recent deprecation so it wont show up on your system cpython probably, but a warning is emitted.

I found this out because I vaguely recalled reading about it somewhere https://discuss.python.org/t/bool-deprecation/62232

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