Skip to content

GH-135379: Specialize int operations for compact ints only #135668

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 4 commits into
base: main
Choose a base branch
from

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Jun 18, 2025

This changes the way we handle ints during specialization.
Instead of handling all ints we only specialize for compact ints.
Since compact ints represent the large majority of ints we get a faster common path, at the cost of a few more de-optimizations.
Using compact ints only, and de-optimizing in the extremely rare no-memory case, we can simplify the performance critical paths for integer operations.

This is helpful for top-of-stack caching as the newer simpler instructions to not escape or need to handle errors, so avoids the need to spill to memory.

This also gives us the potential to use BINARY_OP_EXTEND for multi-digit ints in the future.

@Fidget-Spinner
Copy link
Member

Can you please add tests for the new synbol to optimizer_symbols.c?

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.

Need tests and one comment. Otherwise LGTM.

return;
case JIT_SYM_KNOWN_VALUE_TAG:
if (!PyLong_CheckCompact(sym->value.value)) {
Py_CLEAR(sym->value.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 don't think you need to clear things here? It's automatically cleared on finalization of the whole optimizer I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to do it here (we do the same in _Py_uop_sym_set_type) as we are changing the tag, so the finalization won't know there was an object there.

Copy link
Member

Choose a reason for hiding this comment

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

AH ok that's very tricky! Good observation.

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.

2 participants